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

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

[Wang Sheng-Hui]

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
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 <shhuiw@gmail.com>
---
  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;
-- 
1.7.10.4

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

[Mahesh Jagannath Salgaonkar]

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 <shhuiw@gmail.com>
> ---
>  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;