[PATCH] ipmi_si: fix unexpected driver unregister warning

[PATCH] ipmi_si: fix unexpected driver unregister warning

[Kefeng Wang]

If ipmi_si_platform_init()->platform_driver_register() fails,
platform_driver_unregister() called unconditionally will trigger
following warning,

ipmi_platform: Unable to register driver: -12
------------[ cut here ]------------
Unexpected driver unregister!
WARNING: CPU: 1 PID: 7210 at drivers/base/driver.c:193 driver_unregister+0x60/0x70 drivers/base/driver.c:193

Fix it by adding platform_registered variable, only unregister platform
driver when it is already successfully registered.

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/char/ipmi/ipmi_si_platform.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c
index f2a91c4d8cab..0cd849675d99 100644
--- a/drivers/char/ipmi/ipmi_si_platform.c
+++ b/drivers/char/ipmi/ipmi_si_platform.c
@@ -19,6 +19,7 @@
 #include "ipmi_si.h"
 #include "ipmi_dmi.h"
 
+static bool platform_registered;
 static bool si_tryplatform = true;
 #ifdef CONFIG_ACPI
 static bool          si_tryacpi = true;
@@ -469,9 +470,12 @@ void ipmi_si_platform_init(void)
 	int rv = platform_driver_register(&ipmi_platform_driver);
 	if (rv)
 		pr_err("Unable to register driver: %d\n", rv);
+	else
+		platform_registered = true;
 }
 
 void ipmi_si_platform_shutdown(void)
 {
-	platform_driver_unregister(&ipmi_platform_driver);
+	if (platform_registered)
+		platform_driver_unregister(&ipmi_platform_driver);
 }
-- 
2.20.1

[PATCH] ipmi_si: use bool type for initialized variable

[Kefeng Wang]

Cover 'int' to 'bool' type for initialized variable.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/char/ipmi/ipmi_si_intf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index f124a2d2bb9f..da5b6723329a 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -71,7 +71,7 @@ enum si_intf_state {
 
 static const char * const si_to_str[] = { "invalid", "kcs", "smic", "bt" };
 
-static int initialized;
+static bool initialized;
 
 /*
  * Indexes into stats[] in smi_info below.
@@ -2124,7 +2124,7 @@ static int __init init_ipmi_si(void)
 	}
 
 skip_fallback_noirq:
-	initialized = 1;
+	initialized = true;
 	mutex_unlock(&smi_infos_lock);
 
 	if (type)
-- 
2.20.1

Re: [PATCH] ipmi_si: fix unexpected driver unregister warning

[Corey Minyard]

On Fri, May 17, 2019 at 06:12:44PM +0800, Kefeng Wang wrote:
> If ipmi_si_platform_init()->platform_driver_register() fails,
> platform_driver_unregister() called unconditionally will trigger
> following warning,
> 
> ipmi_platform: Unable to register driver: -12
> ------------[ cut here ]------------
> Unexpected driver unregister!
> WARNING: CPU: 1 PID: 7210 at drivers/base/driver.c:193 driver_unregister+0x60/0x70 drivers/base/driver.c:193
> 
> Fix it by adding platform_registered variable, only unregister platform
> driver when it is already successfully registered.

This is good, but have you found out why the driver was unable to
register in the first place?  That really shouldn't happen.

This patch is queued for 5.3.

-corey

> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  drivers/char/ipmi/ipmi_si_platform.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c
> index f2a91c4d8cab..0cd849675d99 100644
> --- a/drivers/char/ipmi/ipmi_si_platform.c
> +++ b/drivers/char/ipmi/ipmi_si_platform.c
> @@ -19,6 +19,7 @@
>  #include "ipmi_si.h"
>  #include "ipmi_dmi.h"
>  
> +static bool platform_registered;
>  static bool si_tryplatform = true;
>  #ifdef CONFIG_ACPI
>  static bool          si_tryacpi = true;
> @@ -469,9 +470,12 @@ void ipmi_si_platform_init(void)
>  	int rv = platform_driver_register(&ipmi_platform_driver);
>  	if (rv)
>  		pr_err("Unable to register driver: %d\n", rv);
> +	else
> +		platform_registered = true;
>  }
>  
>  void ipmi_si_platform_shutdown(void)
>  {
> -	platform_driver_unregister(&ipmi_platform_driver);
> +	if (platform_registered)
> +		platform_driver_unregister(&ipmi_platform_driver);
>  }
> -- 
> 2.20.1
> 

Re: [PATCH] ipmi_si: use bool type for initialized variable

[Corey Minyard]

On Fri, May 17, 2019 at 06:12:45PM +0800, Kefeng Wang wrote:
> Cover 'int' to 'bool' type for initialized variable.

A reasonable cleanup, queued for 5.3.

-corey

> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  drivers/char/ipmi/ipmi_si_intf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index f124a2d2bb9f..da5b6723329a 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -71,7 +71,7 @@ enum si_intf_state {
>  
>  static const char * const si_to_str[] = { "invalid", "kcs", "smic", "bt" };
>  
> -static int initialized;
> +static bool initialized;
>  
>  /*
>   * Indexes into stats[] in smi_info below.
> @@ -2124,7 +2124,7 @@ static int __init init_ipmi_si(void)
>  	}
>  
>  skip_fallback_noirq:
> -	initialized = 1;
> +	initialized = true;
>  	mutex_unlock(&smi_infos_lock);
>  
>  	if (type)
> -- 
> 2.20.1
> 

Re: [PATCH] ipmi_si: fix unexpected driver unregister warning

[Kefeng Wang]

On 2019/5/18 0:18, Corey Minyard wrote:
> On Fri, May 17, 2019 at 06:12:44PM +0800, Kefeng Wang wrote:
>> If ipmi_si_platform_init()->platform_driver_register() fails,
>> platform_driver_unregister() called unconditionally will trigger
>> following warning,
>>
>> ipmi_platform: Unable to register driver: -12
>> ------------[ cut here ]------------
>> Unexpected driver unregister!
>> WARNING: CPU: 1 PID: 7210 at drivers/base/driver.c:193 driver_unregister+0x60/0x70 drivers/base/driver.c:193
>>
>> Fix it by adding platform_registered variable, only unregister platform
>> driver when it is already successfully registered.
> This is good, but have you found out why the driver was unable to
> register in the first place?  That really shouldn't happen.

Yes, it is hard to happen in  real scene, the test robot found this issue

when FAULT_INJECTION enabled.

>
> This patch is queued for 5.3.
>
> -corey
>
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>  drivers/char/ipmi/ipmi_si_platform.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/char/ipmi/ipmi_si_platform.c b/drivers/char/ipmi/ipmi_si_platform.c
>> index f2a91c4d8cab..0cd849675d99 100644
>> --- a/drivers/char/ipmi/ipmi_si_platform.c
>> +++ b/drivers/char/ipmi/ipmi_si_platform.c
>> @@ -19,6 +19,7 @@
>>  #include "ipmi_si.h"
>>  #include "ipmi_dmi.h"
>>  
>> +static bool platform_registered;
>>  static bool si_tryplatform = true;
>>  #ifdef CONFIG_ACPI
>>  static bool          si_tryacpi = true;
>> @@ -469,9 +470,12 @@ void ipmi_si_platform_init(void)
>>  	int rv = platform_driver_register(&ipmi_platform_driver);
>>  	if (rv)
>>  		pr_err("Unable to register driver: %d\n", rv);
>> +	else
>> +		platform_registered = true;
>>  }
>>  
>>  void ipmi_si_platform_shutdown(void)
>>  {
>> -	platform_driver_unregister(&ipmi_platform_driver);
>> +	if (platform_registered)
>> +		platform_driver_unregister(&ipmi_platform_driver);
>>  }
>> -- 
>> 2.20.1
>>
> .
>