Subject: Re: [PATCH] fadump: allow duplicate assignment to /sys/kernel/fadump_registered when it's assigned the desired value already

From: Mahesh Jagannath Salgaonkar <[email-masked]>
Date: Wed, 10 Apr 2013 07:13:12 +0530

On 04/10/2013 06:16 AM, Wang Sheng-Hui wrote:
> When the fadump is enabled, we have /sys/kernel/fadump_enabled assigned 1.
> But sometimes we need to restart the fadump service by 'service kdump
> restart',
> in case the kdump script has added the fadump detect/support already.
> In current implementation, we cannot re-assign 1 to
> /sys/kernel/fadump_enabled

I assume you meant /sys/kernel/fadump_registered here. Ideally service
kdump restart should first echo 0 to /sys/kernel/fadump_registered to
un-register fadump before echo 1 to /sys/kernel/fadump_registered for
re-registration. I would fix the user space tool than changing the
kernel code.

> if 1 is already set and we have added more logic check in the user space
> script.
> 
> I think we can enable the duplicate assignment to ease the user space
> tools,
> as long as the value is the right 1 or 0.
> 
> Signed-off-by: Wang Sheng-Hui <[email-masked]>
> ---
>  arch/powerpc/kernel/fadump.c |    8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 06c8202..e1347e5 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -1140,18 +1140,14 @@ static ssize_t fadump_register_store(struct
> kobject *kobj,
> 
>      switch (buf[0]) {
>      case '0':
> -        if (fw_dump.dump_registered == 0) {
> -            ret = -EINVAL;
> +        if (fw_dump.dump_registered == 0)
>              goto unlock_out;
> -        }
>          /* Un-register Firmware-assisted dump */
>          fadump_unregister_dump(&fdm);
>          break;
>      case '1':
> -        if (fw_dump.dump_registered == 1) {
> -            ret = -EINVAL;
> +        if (fw_dump.dump_registered == 1)
>              goto unlock_out;
> -        }
>          /* Register Firmware-assisted dump */
>          register_fadump();
>          break;