linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ipmi: Don't allow device module unload when in use
       [not found] <20191014134141.GA25427@t560>
@ 2019-10-14 15:46 ` minyard
  2019-10-16 12:18   ` tony camuso
                     ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: minyard @ 2019-10-14 15:46 UTC (permalink / raw)
  To: tony camuso
  Cc: openipmi-developer, Corey Minyard, linux-kernel, Corey Minyard

From: Corey Minyard <cminyard@mvista.com>

If something has the IPMI driver open, don't allow the device
module to be unloaded.  Before it would unload and the user would
get errors on use.

This change is made on user request, and it makes it consistent
with the I2C driver, which has the same behavior.

It does change things a little bit with respect to kernel users.
If the ACPI or IPMI watchdog (or any other kernel user) has
created a user, then the device module cannot be unloaded.  Before
it could be unloaded,

This does not affect hot-plug.  If the device goes away (it's on
something removable that is removed or is hot-removed via sysfs)
then it still behaves as it did before.

Reported-by: tony camuso <tcamuso@redhat.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
Tony, here is a suggested change for this.  Can you look it over and
see if it looks ok?

Thanks,

-corey

 drivers/char/ipmi/ipmi_msghandler.c | 23 ++++++++++++++++-------
 include/linux/ipmi_smi.h            | 12 ++++++++----
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 2aab80e19ae0..15680de18625 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -448,6 +448,8 @@ enum ipmi_stat_indexes {
 
 #define IPMI_IPMB_NUM_SEQ	64
 struct ipmi_smi {
+	struct module *owner;
+
 	/* What interface number are we? */
 	int intf_num;
 
@@ -1220,6 +1222,11 @@ int ipmi_create_user(unsigned int          if_num,
 	if (rv)
 		goto out_kfree;
 
+	if (!try_module_get(intf->owner)) {
+		rv = -ENODEV;
+		goto out_kfree;
+	}
+	
 	/* Note that each existing user holds a refcount to the interface. */
 	kref_get(&intf->refcount);
 
@@ -1349,6 +1356,7 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
 	}
 
 	kref_put(&intf->refcount, intf_free);
+	module_put(intf->owner);
 }
 
 int ipmi_destroy_user(struct ipmi_user *user)
@@ -2459,7 +2467,7 @@ static int __get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc)
  * been recently fetched, this will just use the cached data.  Otherwise
  * it will run a new fetch.
  *
- * Except for the first time this is called (in ipmi_register_smi()),
+ * Except for the first time this is called (in ipmi_add_smi()),
  * this will always return good data;
  */
 static int __bmc_get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc,
@@ -3377,10 +3385,11 @@ static void redo_bmc_reg(struct work_struct *work)
 	kref_put(&intf->refcount, intf_free);
 }
 
-int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
-		      void		       *send_info,
-		      struct device            *si_dev,
-		      unsigned char            slave_addr)
+int ipmi_add_smi(struct module         *owner,
+		 const struct ipmi_smi_handlers *handlers,
+		 void		       *send_info,
+		 struct device         *si_dev,
+		 unsigned char         slave_addr)
 {
 	int              i, j;
 	int              rv;
@@ -3406,7 +3415,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
 		return rv;
 	}
 
-
+	intf->owner = owner;
 	intf->bmc = &intf->tmp_bmc;
 	INIT_LIST_HEAD(&intf->bmc->intfs);
 	mutex_init(&intf->bmc->dyn_mutex);
@@ -3514,7 +3523,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
 
 	return rv;
 }
-EXPORT_SYMBOL(ipmi_register_smi);
+EXPORT_SYMBOL(ipmi_add_smi);
 
 static void deliver_smi_err_response(struct ipmi_smi *intf,
 				     struct ipmi_smi_msg *msg,
diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h
index 4dc66157d872..deec18b8944a 100644
--- a/include/linux/ipmi_smi.h
+++ b/include/linux/ipmi_smi.h
@@ -224,10 +224,14 @@ static inline int ipmi_demangle_device_id(uint8_t netfn, uint8_t cmd,
  * is called, and the lower layer must get the interface from that
  * call.
  */
-int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
-		      void                     *send_info,
-		      struct device            *dev,
-		      unsigned char            slave_addr);
+int ipmi_add_smi(struct module            *owner,
+		 const struct ipmi_smi_handlers *handlers,
+		 void                     *send_info,
+		 struct device            *dev,
+		 unsigned char            slave_addr);
+
+#define ipmi_register_smi(handlers, send_info, dev, slave_addr) \
+	ipmi_add_smi(THIS_MODULE, handlers, send_info, dev, slave_addr)
 
 /*
  * Remove a low-level interface from the IPMI driver.  This will
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] ipmi: Don't allow device module unload when in use
  2019-10-14 15:46 ` [PATCH] ipmi: Don't allow device module unload when in use minyard
@ 2019-10-16 12:18   ` tony camuso
  2019-10-16 13:13     ` tony camuso
  2019-10-16 19:25   ` Tony Camuso
  2019-10-22 14:29   ` Tony Camuso
  2 siblings, 1 reply; 8+ messages in thread
From: tony camuso @ 2019-10-16 12:18 UTC (permalink / raw)
  To: minyard; +Cc: openipmi-developer, linux-kernel, Corey Minyard

On 10/14/19 11:46 AM, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> If something has the IPMI driver open, don't allow the device
> module to be unloaded.  Before it would unload and the user would
> get errors on use.
> 
> This change is made on user request, and it makes it consistent
> with the I2C driver, which has the same behavior.
> 
> It does change things a little bit with respect to kernel users.
> If the ACPI or IPMI watchdog (or any other kernel user) has
> created a user, then the device module cannot be unloaded.  Before
> it could be unloaded,
> 
> This does not affect hot-plug.  If the device goes away (it's on
> something removable that is removed or is hot-removed via sysfs)
> then it still behaves as it did before.
> 
> Reported-by: tony camuso <tcamuso@redhat.com>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
> Tony, here is a suggested change for this.  Can you look it over and
> see if it looks ok?
> 
> Thanks,
> 
> -corey
> 
>   drivers/char/ipmi/ipmi_msghandler.c | 23 ++++++++++++++++-------
>   include/linux/ipmi_smi.h            | 12 ++++++++----
>   2 files changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> index 2aab80e19ae0..15680de18625 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -448,6 +448,8 @@ enum ipmi_stat_indexes {
>   
>   #define IPMI_IPMB_NUM_SEQ	64
>   struct ipmi_smi {
> +	struct module *owner;
> +
>   	/* What interface number are we? */
>   	int intf_num;
>   
> @@ -1220,6 +1222,11 @@ int ipmi_create_user(unsigned int          if_num,
>   	if (rv)
>   		goto out_kfree;
>   
> +	if (!try_module_get(intf->owner)) {
> +		rv = -ENODEV;
> +		goto out_kfree;
> +	}
> +	
>   	/* Note that each existing user holds a refcount to the interface. */
>   	kref_get(&intf->refcount);
>   
> @@ -1349,6 +1356,7 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
>   	}
>   
>   	kref_put(&intf->refcount, intf_free);
> +	module_put(intf->owner);
>   }
>   
>   int ipmi_destroy_user(struct ipmi_user *user)
> @@ -2459,7 +2467,7 @@ static int __get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc)
>    * been recently fetched, this will just use the cached data.  Otherwise
>    * it will run a new fetch.
>    *
> - * Except for the first time this is called (in ipmi_register_smi()),
> + * Except for the first time this is called (in ipmi_add_smi()),
>    * this will always return good data;
>    */
>   static int __bmc_get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc,
> @@ -3377,10 +3385,11 @@ static void redo_bmc_reg(struct work_struct *work)
>   	kref_put(&intf->refcount, intf_free);
>   }
>   
> -int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
> -		      void		       *send_info,
> -		      struct device            *si_dev,
> -		      unsigned char            slave_addr)
> +int ipmi_add_smi(struct module         *owner,
> +		 const struct ipmi_smi_handlers *handlers,
> +		 void		       *send_info,
> +		 struct device         *si_dev,
> +		 unsigned char         slave_addr)
>   {
>   	int              i, j;
>   	int              rv;
> @@ -3406,7 +3415,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
>   		return rv;
>   	}
>   
> -
> +	intf->owner = owner;
>   	intf->bmc = &intf->tmp_bmc;
>   	INIT_LIST_HEAD(&intf->bmc->intfs);
>   	mutex_init(&intf->bmc->dyn_mutex);
> @@ -3514,7 +3523,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
>   
>   	return rv;
>   }
> -EXPORT_SYMBOL(ipmi_register_smi);
> +EXPORT_SYMBOL(ipmi_add_smi);
>   
>   static void deliver_smi_err_response(struct ipmi_smi *intf,
>   				     struct ipmi_smi_msg *msg,
> diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h
> index 4dc66157d872..deec18b8944a 100644
> --- a/include/linux/ipmi_smi.h
> +++ b/include/linux/ipmi_smi.h
> @@ -224,10 +224,14 @@ static inline int ipmi_demangle_device_id(uint8_t netfn, uint8_t cmd,
>    * is called, and the lower layer must get the interface from that
>    * call.
>    */
> -int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
> -		      void                     *send_info,
> -		      struct device            *dev,
> -		      unsigned char            slave_addr);
> +int ipmi_add_smi(struct module            *owner,
> +		 const struct ipmi_smi_handlers *handlers,
> +		 void                     *send_info,
> +		 struct device            *dev,
> +		 unsigned char            slave_addr);
> +
> +#define ipmi_register_smi(handlers, send_info, dev, slave_addr) \
> +	ipmi_add_smi(THIS_MODULE, handlers, send_info, dev, slave_addr)
>   
>   /*
>    * Remove a low-level interface from the IPMI driver.  This will
> 

Thanks, Corey.

Regards,
Tony


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] ipmi: Don't allow device module unload when in use
  2019-10-16 12:18   ` tony camuso
@ 2019-10-16 13:13     ` tony camuso
  0 siblings, 0 replies; 8+ messages in thread
From: tony camuso @ 2019-10-16 13:13 UTC (permalink / raw)
  To: minyard; +Cc: openipmi-developer, linux-kernel, Corey Minyard

On 10/16/19 8:18 AM, tony camuso wrote:
> On 10/14/19 11:46 AM, minyard@acm.org wrote:
>> From: Corey Minyard <cminyard@mvista.com>
>>
>> If something has the IPMI driver open, don't allow the device
>> module to be unloaded.  Before it would unload and the user would
>> get errors on use.
>>
>> This change is made on user request, and it makes it consistent
>> with the I2C driver, which has the same behavior.
>>
>> It does change things a little bit with respect to kernel users.
>> If the ACPI or IPMI watchdog (or any other kernel user) has
>> created a user, then the device module cannot be unloaded.  Before
>> it could be unloaded,
>>
>> This does not affect hot-plug.  If the device goes away (it's on
>> something removable that is removed or is hot-removed via sysfs)
>> then it still behaves as it did before.
>>
>> Reported-by: tony camuso <tcamuso@redhat.com>
>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
>> ---
>> Tony, here is a suggested change for this.  Can you look it over and
>> see if it looks ok?
>>
>> Thanks,
>>
>> -corey
>>
>>   drivers/char/ipmi/ipmi_msghandler.c | 23 ++++++++++++++++-------
>>   include/linux/ipmi_smi.h            | 12 ++++++++----
>>   2 files changed, 24 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
>> index 2aab80e19ae0..15680de18625 100644
>> --- a/drivers/char/ipmi/ipmi_msghandler.c
>> +++ b/drivers/char/ipmi/ipmi_msghandler.c
>> @@ -448,6 +448,8 @@ enum ipmi_stat_indexes {
>>   #define IPMI_IPMB_NUM_SEQ    64
>>   struct ipmi_smi {
>> +    struct module *owner;
>> +
>>       /* What interface number are we? */
>>       int intf_num;
>> @@ -1220,6 +1222,11 @@ int ipmi_create_user(unsigned int          if_num,
>>       if (rv)
>>           goto out_kfree;
>> +    if (!try_module_get(intf->owner)) {
>> +        rv = -ENODEV;
>> +        goto out_kfree;
>> +    }
>> +
>>       /* Note that each existing user holds a refcount to the interface. */
>>       kref_get(&intf->refcount);
>> @@ -1349,6 +1356,7 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
>>       }
>>       kref_put(&intf->refcount, intf_free);
>> +    module_put(intf->owner);
>>   }
>>   int ipmi_destroy_user(struct ipmi_user *user)
>> @@ -2459,7 +2467,7 @@ static int __get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc)
>>    * been recently fetched, this will just use the cached data.  Otherwise
>>    * it will run a new fetch.
>>    *
>> - * Except for the first time this is called (in ipmi_register_smi()),
>> + * Except for the first time this is called (in ipmi_add_smi()),
>>    * this will always return good data;
>>    */
>>   static int __bmc_get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc,
>> @@ -3377,10 +3385,11 @@ static void redo_bmc_reg(struct work_struct *work)
>>       kref_put(&intf->refcount, intf_free);
>>   }
>> -int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
>> -              void               *send_info,
>> -              struct device            *si_dev,
>> -              unsigned char            slave_addr)
>> +int ipmi_add_smi(struct module         *owner,
>> +         const struct ipmi_smi_handlers *handlers,
>> +         void               *send_info,
>> +         struct device         *si_dev,
>> +         unsigned char         slave_addr)
>>   {
>>       int              i, j;
>>       int              rv;
>> @@ -3406,7 +3415,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
>>           return rv;
>>       }
>> -
>> +    intf->owner = owner;
>>       intf->bmc = &intf->tmp_bmc;
>>       INIT_LIST_HEAD(&intf->bmc->intfs);
>>       mutex_init(&intf->bmc->dyn_mutex);
>> @@ -3514,7 +3523,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
>>       return rv;
>>   }
>> -EXPORT_SYMBOL(ipmi_register_smi);
>> +EXPORT_SYMBOL(ipmi_add_smi);
>>   static void deliver_smi_err_response(struct ipmi_smi *intf,
>>                        struct ipmi_smi_msg *msg,
>> diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h
>> index 4dc66157d872..deec18b8944a 100644
>> --- a/include/linux/ipmi_smi.h
>> +++ b/include/linux/ipmi_smi.h
>> @@ -224,10 +224,14 @@ static inline int ipmi_demangle_device_id(uint8_t netfn, uint8_t cmd,
>>    * is called, and the lower layer must get the interface from that
>>    * call.
>>    */
>> -int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
>> -              void                     *send_info,
>> -              struct device            *dev,
>> -              unsigned char            slave_addr);
>> +int ipmi_add_smi(struct module            *owner,
>> +         const struct ipmi_smi_handlers *handlers,
>> +         void                     *send_info,
>> +         struct device            *dev,
>> +         unsigned char            slave_addr);
>> +
>> +#define ipmi_register_smi(handlers, send_info, dev, slave_addr) \
>> +    ipmi_add_smi(THIS_MODULE, handlers, send_info, dev, slave_addr)
>>   /*
>>    * Remove a low-level interface from the IPMI driver.  This will
>>
> 
> Thanks, Corey.
> 
> Regards,
> Tony
> 

And I meant to add that I will be testing this over the next couple days.


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] ipmi: Don't allow device module unload when in use
  2019-10-14 15:46 ` [PATCH] ipmi: Don't allow device module unload when in use minyard
  2019-10-16 12:18   ` tony camuso
@ 2019-10-16 19:25   ` Tony Camuso
  2019-10-16 19:33     ` Corey Minyard
  2019-10-22 14:29   ` Tony Camuso
  2 siblings, 1 reply; 8+ messages in thread
From: Tony Camuso @ 2019-10-16 19:25 UTC (permalink / raw)
  To: minyard; +Cc: openipmi-developer, linux-kernel, Corey Minyard

On 10/14/19 11:46 AM, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> If something has the IPMI driver open, don't allow the device
> module to be unloaded.  Before it would unload and the user would
> get errors on use.
> 
> This change is made on user request, and it makes it consistent
> with the I2C driver, which has the same behavior.
> 
> It does change things a little bit with respect to kernel users.
> If the ACPI or IPMI watchdog (or any other kernel user) has
> created a user, then the device module cannot be unloaded.  Before
> it could be unloaded,
> 
> This does not affect hot-plug.  If the device goes away (it's on
> something removable that is removed or is hot-removed via sysfs)
> then it still behaves as it did before.
> 
> Reported-by: tony camuso <tcamuso@redhat.com>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
> Tony, here is a suggested change for this.  Can you look it over and
> see if it looks ok?
> 
> Thanks,
> 
> -corey
> 
>   drivers/char/ipmi/ipmi_msghandler.c | 23 ++++++++++++++++-------
>   include/linux/ipmi_smi.h            | 12 ++++++++----
>   2 files changed, 24 insertions(+), 11 deletions(-)

Hi Corey.

You changed ipmi_register_ipmi to ipmi_add_ipmi in ipmi_msghandler, but you
did not change it where it is actually called.

# grep ipmi_register_smi drivers/char/ipmi/*.c
drivers/char/ipmi/ipmi_powernv.c:	rc = ipmi_register_smi(&ipmi_powernv_smi_handlers, ipmi, dev, 0);
drivers/char/ipmi/ipmi_si_intf.c:	rv = ipmi_register_smi(&handlers,
drivers/char/ipmi/ipmi_ssif.c:	rv = ipmi_register_smi(&ssif_info->handlers,

Is there a reason for changing the interface name? Is this something
that I could do instead of troubling you with it?

Regards,
Tony


> 
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> index 2aab80e19ae0..15680de18625 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -448,6 +448,8 @@ enum ipmi_stat_indexes {
>   
>   #define IPMI_IPMB_NUM_SEQ	64
>   struct ipmi_smi {
> +	struct module *owner;
> +
>   	/* What interface number are we? */
>   	int intf_num;
>   
> @@ -1220,6 +1222,11 @@ int ipmi_create_user(unsigned int          if_num,
>   	if (rv)
>   		goto out_kfree;
>   
> +	if (!try_module_get(intf->owner)) {
> +		rv = -ENODEV;
> +		goto out_kfree;
> +	}
> +	
>   	/* Note that each existing user holds a refcount to the interface. */
>   	kref_get(&intf->refcount);
>   
> @@ -1349,6 +1356,7 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
>   	}
>   
>   	kref_put(&intf->refcount, intf_free);
> +	module_put(intf->owner);
>   }
>   
>   int ipmi_destroy_user(struct ipmi_user *user)
> @@ -2459,7 +2467,7 @@ static int __get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc)
>    * been recently fetched, this will just use the cached data.  Otherwise
>    * it will run a new fetch.
>    *
> - * Except for the first time this is called (in ipmi_register_smi()),
> + * Except for the first time this is called (in ipmi_add_smi()),
>    * this will always return good data;
>    */
>   static int __bmc_get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc,
> @@ -3377,10 +3385,11 @@ static void redo_bmc_reg(struct work_struct *work)
>   	kref_put(&intf->refcount, intf_free);
>   }
>   
> -int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
> -		      void		       *send_info,
> -		      struct device            *si_dev,
> -		      unsigned char            slave_addr)
> +int ipmi_add_smi(struct module         *owner,
> +		 const struct ipmi_smi_handlers *handlers,
> +		 void		       *send_info,
> +		 struct device         *si_dev,
> +		 unsigned char         slave_addr)
>   {
>   	int              i, j;
>   	int              rv;
> @@ -3406,7 +3415,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
>   		return rv;
>   	}
>   
> -
> +	intf->owner = owner;
>   	intf->bmc = &intf->tmp_bmc;
>   	INIT_LIST_HEAD(&intf->bmc->intfs);
>   	mutex_init(&intf->bmc->dyn_mutex);
> @@ -3514,7 +3523,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
>   
>   	return rv;
>   }
> -EXPORT_SYMBOL(ipmi_register_smi);
> +EXPORT_SYMBOL(ipmi_add_smi);
>   
>   static void deliver_smi_err_response(struct ipmi_smi *intf,
>   				     struct ipmi_smi_msg *msg,
> diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h
> index 4dc66157d872..deec18b8944a 100644
> --- a/include/linux/ipmi_smi.h
> +++ b/include/linux/ipmi_smi.h
> @@ -224,10 +224,14 @@ static inline int ipmi_demangle_device_id(uint8_t netfn, uint8_t cmd,
>    * is called, and the lower layer must get the interface from that
>    * call.
>    */
> -int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
> -		      void                     *send_info,
> -		      struct device            *dev,
> -		      unsigned char            slave_addr);
> +int ipmi_add_smi(struct module            *owner,
> +		 const struct ipmi_smi_handlers *handlers,
> +		 void                     *send_info,
> +		 struct device            *dev,
> +		 unsigned char            slave_addr);
> +
> +#define ipmi_register_smi(handlers, send_info, dev, slave_addr) \
> +	ipmi_add_smi(THIS_MODULE, handlers, send_info, dev, slave_addr)
>   
>   /*
>    * Remove a low-level interface from the IPMI driver.  This will
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] ipmi: Don't allow device module unload when in use
  2019-10-16 19:25   ` Tony Camuso
@ 2019-10-16 19:33     ` Corey Minyard
  2019-10-16 19:54       ` Tony Camuso
  0 siblings, 1 reply; 8+ messages in thread
From: Corey Minyard @ 2019-10-16 19:33 UTC (permalink / raw)
  To: Tony Camuso; +Cc: openipmi-developer, linux-kernel, Corey Minyard

On Wed, Oct 16, 2019 at 03:25:56PM -0400, Tony Camuso wrote:
> On 10/14/19 11:46 AM, minyard@acm.org wrote:
> > From: Corey Minyard <cminyard@mvista.com>
> > 
> > If something has the IPMI driver open, don't allow the device
> > module to be unloaded.  Before it would unload and the user would
> > get errors on use.
> > 
> > This change is made on user request, and it makes it consistent
> > with the I2C driver, which has the same behavior.
> > 
> > It does change things a little bit with respect to kernel users.
> > If the ACPI or IPMI watchdog (or any other kernel user) has
> > created a user, then the device module cannot be unloaded.  Before
> > it could be unloaded,
> > 
> > This does not affect hot-plug.  If the device goes away (it's on
> > something removable that is removed or is hot-removed via sysfs)
> > then it still behaves as it did before.
> > 
> > Reported-by: tony camuso <tcamuso@redhat.com>
> > Signed-off-by: Corey Minyard <cminyard@mvista.com>
> > ---
> > Tony, here is a suggested change for this.  Can you look it over and
> > see if it looks ok?
> > 
> > Thanks,
> > 
> > -corey
> > 
> >   drivers/char/ipmi/ipmi_msghandler.c | 23 ++++++++++++++++-------
> >   include/linux/ipmi_smi.h            | 12 ++++++++----
> >   2 files changed, 24 insertions(+), 11 deletions(-)
> 
> Hi Corey.
> 
> You changed ipmi_register_ipmi to ipmi_add_ipmi in ipmi_msghandler, but you
> did not change it where it is actually called.
> 
> # grep ipmi_register_smi drivers/char/ipmi/*.c
> drivers/char/ipmi/ipmi_powernv.c:	rc = ipmi_register_smi(&ipmi_powernv_smi_handlers, ipmi, dev, 0);
> drivers/char/ipmi/ipmi_si_intf.c:	rv = ipmi_register_smi(&handlers,
> drivers/char/ipmi/ipmi_ssif.c:	rv = ipmi_register_smi(&ssif_info->handlers,
> 
> Is there a reason for changing the interface name? Is this something
> that I could do instead of troubling you with it?

I don't understand.  You don't say that anything went wrong, you just
referenced something I changed.

I changed the name so I could create a macro with that name to pass in
the module name.  Pretty standard to do in the kernel.  Is there a
compile or runtime issue?

-corey

> 
> Regards,
> Tony
> 
> 
> > 
> > diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> > index 2aab80e19ae0..15680de18625 100644
> > --- a/drivers/char/ipmi/ipmi_msghandler.c
> > +++ b/drivers/char/ipmi/ipmi_msghandler.c
> > @@ -448,6 +448,8 @@ enum ipmi_stat_indexes {
> >   #define IPMI_IPMB_NUM_SEQ	64
> >   struct ipmi_smi {
> > +	struct module *owner;
> > +
> >   	/* What interface number are we? */
> >   	int intf_num;
> > @@ -1220,6 +1222,11 @@ int ipmi_create_user(unsigned int          if_num,
> >   	if (rv)
> >   		goto out_kfree;
> > +	if (!try_module_get(intf->owner)) {
> > +		rv = -ENODEV;
> > +		goto out_kfree;
> > +	}
> > +	
> >   	/* Note that each existing user holds a refcount to the interface. */
> >   	kref_get(&intf->refcount);
> > @@ -1349,6 +1356,7 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
> >   	}
> >   	kref_put(&intf->refcount, intf_free);
> > +	module_put(intf->owner);
> >   }
> >   int ipmi_destroy_user(struct ipmi_user *user)
> > @@ -2459,7 +2467,7 @@ static int __get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc)
> >    * been recently fetched, this will just use the cached data.  Otherwise
> >    * it will run a new fetch.
> >    *
> > - * Except for the first time this is called (in ipmi_register_smi()),
> > + * Except for the first time this is called (in ipmi_add_smi()),
> >    * this will always return good data;
> >    */
> >   static int __bmc_get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc,
> > @@ -3377,10 +3385,11 @@ static void redo_bmc_reg(struct work_struct *work)
> >   	kref_put(&intf->refcount, intf_free);
> >   }
> > -int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
> > -		      void		       *send_info,
> > -		      struct device            *si_dev,
> > -		      unsigned char            slave_addr)
> > +int ipmi_add_smi(struct module         *owner,
> > +		 const struct ipmi_smi_handlers *handlers,
> > +		 void		       *send_info,
> > +		 struct device         *si_dev,
> > +		 unsigned char         slave_addr)
> >   {
> >   	int              i, j;
> >   	int              rv;
> > @@ -3406,7 +3415,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
> >   		return rv;
> >   	}
> > -
> > +	intf->owner = owner;
> >   	intf->bmc = &intf->tmp_bmc;
> >   	INIT_LIST_HEAD(&intf->bmc->intfs);
> >   	mutex_init(&intf->bmc->dyn_mutex);
> > @@ -3514,7 +3523,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
> >   	return rv;
> >   }
> > -EXPORT_SYMBOL(ipmi_register_smi);
> > +EXPORT_SYMBOL(ipmi_add_smi);
> >   static void deliver_smi_err_response(struct ipmi_smi *intf,
> >   				     struct ipmi_smi_msg *msg,
> > diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h
> > index 4dc66157d872..deec18b8944a 100644
> > --- a/include/linux/ipmi_smi.h
> > +++ b/include/linux/ipmi_smi.h
> > @@ -224,10 +224,14 @@ static inline int ipmi_demangle_device_id(uint8_t netfn, uint8_t cmd,
> >    * is called, and the lower layer must get the interface from that
> >    * call.
> >    */
> > -int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
> > -		      void                     *send_info,
> > -		      struct device            *dev,
> > -		      unsigned char            slave_addr);
> > +int ipmi_add_smi(struct module            *owner,
> > +		 const struct ipmi_smi_handlers *handlers,
> > +		 void                     *send_info,
> > +		 struct device            *dev,
> > +		 unsigned char            slave_addr);
> > +
> > +#define ipmi_register_smi(handlers, send_info, dev, slave_addr) \
> > +	ipmi_add_smi(THIS_MODULE, handlers, send_info, dev, slave_addr)
> >   /*
> >    * Remove a low-level interface from the IPMI driver.  This will
> > 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] ipmi: Don't allow device module unload when in use
  2019-10-16 19:33     ` Corey Minyard
@ 2019-10-16 19:54       ` Tony Camuso
  0 siblings, 0 replies; 8+ messages in thread
From: Tony Camuso @ 2019-10-16 19:54 UTC (permalink / raw)
  To: minyard; +Cc: openipmi-developer, linux-kernel, Corey Minyard

On 10/16/19 3:33 PM, Corey Minyard wrote:
> On Wed, Oct 16, 2019 at 03:25:56PM -0400, Tony Camuso wrote:
>> On 10/14/19 11:46 AM, minyard@acm.org wrote:
>>> From: Corey Minyard <cminyard@mvista.com>
>>>
>>> If something has the IPMI driver open, don't allow the device
>>> module to be unloaded.  Before it would unload and the user would
>>> get errors on use.
>>>
>>> This change is made on user request, and it makes it consistent
>>> with the I2C driver, which has the same behavior.
>>>
>>> It does change things a little bit with respect to kernel users.
>>> If the ACPI or IPMI watchdog (or any other kernel user) has
>>> created a user, then the device module cannot be unloaded.  Before
>>> it could be unloaded,
>>>
>>> This does not affect hot-plug.  If the device goes away (it's on
>>> something removable that is removed or is hot-removed via sysfs)
>>> then it still behaves as it did before.
>>>
>>> Reported-by: tony camuso <tcamuso@redhat.com>
>>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
>>> ---
>>> Tony, here is a suggested change for this.  Can you look it over and
>>> see if it looks ok?
>>>
>>> Thanks,
>>>
>>> -corey
>>>
>>>    drivers/char/ipmi/ipmi_msghandler.c | 23 ++++++++++++++++-------
>>>    include/linux/ipmi_smi.h            | 12 ++++++++----
>>>    2 files changed, 24 insertions(+), 11 deletions(-)
>>
>> Hi Corey.
>>
>> You changed ipmi_register_ipmi to ipmi_add_ipmi in ipmi_msghandler, but you
>> did not change it where it is actually called.
>>
>> # grep ipmi_register_smi drivers/char/ipmi/*.c
>> drivers/char/ipmi/ipmi_powernv.c:	rc = ipmi_register_smi(&ipmi_powernv_smi_handlers, ipmi, dev, 0);
>> drivers/char/ipmi/ipmi_si_intf.c:	rv = ipmi_register_smi(&handlers,
>> drivers/char/ipmi/ipmi_ssif.c:	rv = ipmi_register_smi(&ssif_info->handlers,
>>
>> Is there a reason for changing the interface name? Is this something
>> that I could do instead of troubling you with it?
> 
> I don't understand.  You don't say that anything went wrong, you just
> referenced something I changed.
> 
> I changed the name so I could create a macro with that name to pass in
> the module name.  Pretty standard to do in the kernel.  

Can't believe I missed that.

> Is there a
> compile or runtime issue?
> 
> -corey

All is well, so far. Haven't finished testing.

>>
>> Regards,
>> Tony
>>
>>
>>>
>>> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
>>> index 2aab80e19ae0..15680de18625 100644
>>> --- a/drivers/char/ipmi/ipmi_msghandler.c
>>> +++ b/drivers/char/ipmi/ipmi_msghandler.c
>>> @@ -448,6 +448,8 @@ enum ipmi_stat_indexes {
>>>    #define IPMI_IPMB_NUM_SEQ	64
>>>    struct ipmi_smi {
>>> +	struct module *owner;
>>> +
>>>    	/* What interface number are we? */
>>>    	int intf_num;
>>> @@ -1220,6 +1222,11 @@ int ipmi_create_user(unsigned int          if_num,
>>>    	if (rv)
>>>    		goto out_kfree;
>>> +	if (!try_module_get(intf->owner)) {
>>> +		rv = -ENODEV;
>>> +		goto out_kfree;
>>> +	}
>>> +	
>>>    	/* Note that each existing user holds a refcount to the interface. */
>>>    	kref_get(&intf->refcount);
>>> @@ -1349,6 +1356,7 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
>>>    	}
>>>    	kref_put(&intf->refcount, intf_free);
>>> +	module_put(intf->owner);
>>>    }
>>>    int ipmi_destroy_user(struct ipmi_user *user)
>>> @@ -2459,7 +2467,7 @@ static int __get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc)
>>>     * been recently fetched, this will just use the cached data.  Otherwise
>>>     * it will run a new fetch.
>>>     *
>>> - * Except for the first time this is called (in ipmi_register_smi()),
>>> + * Except for the first time this is called (in ipmi_add_smi()),
>>>     * this will always return good data;
>>>     */
>>>    static int __bmc_get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc,
>>> @@ -3377,10 +3385,11 @@ static void redo_bmc_reg(struct work_struct *work)
>>>    	kref_put(&intf->refcount, intf_free);
>>>    }
>>> -int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
>>> -		      void		       *send_info,
>>> -		      struct device            *si_dev,
>>> -		      unsigned char            slave_addr)
>>> +int ipmi_add_smi(struct module         *owner,
>>> +		 const struct ipmi_smi_handlers *handlers,
>>> +		 void		       *send_info,
>>> +		 struct device         *si_dev,
>>> +		 unsigned char         slave_addr)
>>>    {
>>>    	int              i, j;
>>>    	int              rv;
>>> @@ -3406,7 +3415,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
>>>    		return rv;
>>>    	}
>>> -
>>> +	intf->owner = owner;
>>>    	intf->bmc = &intf->tmp_bmc;
>>>    	INIT_LIST_HEAD(&intf->bmc->intfs);
>>>    	mutex_init(&intf->bmc->dyn_mutex);
>>> @@ -3514,7 +3523,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
>>>    	return rv;
>>>    }
>>> -EXPORT_SYMBOL(ipmi_register_smi);
>>> +EXPORT_SYMBOL(ipmi_add_smi);
>>>    static void deliver_smi_err_response(struct ipmi_smi *intf,
>>>    				     struct ipmi_smi_msg *msg,
>>> diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h
>>> index 4dc66157d872..deec18b8944a 100644
>>> --- a/include/linux/ipmi_smi.h
>>> +++ b/include/linux/ipmi_smi.h
>>> @@ -224,10 +224,14 @@ static inline int ipmi_demangle_device_id(uint8_t netfn, uint8_t cmd,
>>>     * is called, and the lower layer must get the interface from that
>>>     * call.
>>>     */
>>> -int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
>>> -		      void                     *send_info,
>>> -		      struct device            *dev,
>>> -		      unsigned char            slave_addr);
>>> +int ipmi_add_smi(struct module            *owner,
>>> +		 const struct ipmi_smi_handlers *handlers,
>>> +		 void                     *send_info,
>>> +		 struct device            *dev,
>>> +		 unsigned char            slave_addr);
>>> +
>>> +#define ipmi_register_smi(handlers, send_info, dev, slave_addr) \
>>> +	ipmi_add_smi(THIS_MODULE, handlers, send_info, dev, slave_addr)
>>>    /*
>>>     * Remove a low-level interface from the IPMI driver.  This will
>>>
>>


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] ipmi: Don't allow device module unload when in use
  2019-10-14 15:46 ` [PATCH] ipmi: Don't allow device module unload when in use minyard
  2019-10-16 12:18   ` tony camuso
  2019-10-16 19:25   ` Tony Camuso
@ 2019-10-22 14:29   ` Tony Camuso
  2019-10-22 15:56     ` Corey Minyard
  2 siblings, 1 reply; 8+ messages in thread
From: Tony Camuso @ 2019-10-22 14:29 UTC (permalink / raw)
  To: minyard; +Cc: openipmi-developer, linux-kernel, Corey Minyard

Corey,

Testing shows that this patch works as expected.

Regards,
Tony


On 10/14/19 11:46 AM, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> If something has the IPMI driver open, don't allow the device
> module to be unloaded.  Before it would unload and the user would
> get errors on use.
> 
> This change is made on user request, and it makes it consistent
> with the I2C driver, which has the same behavior.
> 
> It does change things a little bit with respect to kernel users.
> If the ACPI or IPMI watchdog (or any other kernel user) has
> created a user, then the device module cannot be unloaded.  Before
> it could be unloaded,
> 
> This does not affect hot-plug.  If the device goes away (it's on
> something removable that is removed or is hot-removed via sysfs)
> then it still behaves as it did before.
> 
> Reported-by: tony camuso <tcamuso@redhat.com>
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
> Tony, here is a suggested change for this.  Can you look it over and
> see if it looks ok?
> 
> Thanks,
> 
> -corey
> 
>   drivers/char/ipmi/ipmi_msghandler.c | 23 ++++++++++++++++-------
>   include/linux/ipmi_smi.h            | 12 ++++++++----
>   2 files changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> index 2aab80e19ae0..15680de18625 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -448,6 +448,8 @@ enum ipmi_stat_indexes {
>   
>   #define IPMI_IPMB_NUM_SEQ	64
>   struct ipmi_smi {
> +	struct module *owner;
> +
>   	/* What interface number are we? */
>   	int intf_num;
>   
> @@ -1220,6 +1222,11 @@ int ipmi_create_user(unsigned int          if_num,
>   	if (rv)
>   		goto out_kfree;
>   
> +	if (!try_module_get(intf->owner)) {
> +		rv = -ENODEV;
> +		goto out_kfree;
> +	}
> +	
>   	/* Note that each existing user holds a refcount to the interface. */
>   	kref_get(&intf->refcount);
>   
> @@ -1349,6 +1356,7 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
>   	}
>   
>   	kref_put(&intf->refcount, intf_free);
> +	module_put(intf->owner);
>   }
>   
>   int ipmi_destroy_user(struct ipmi_user *user)
> @@ -2459,7 +2467,7 @@ static int __get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc)
>    * been recently fetched, this will just use the cached data.  Otherwise
>    * it will run a new fetch.
>    *
> - * Except for the first time this is called (in ipmi_register_smi()),
> + * Except for the first time this is called (in ipmi_add_smi()),
>    * this will always return good data;
>    */
>   static int __bmc_get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc,
> @@ -3377,10 +3385,11 @@ static void redo_bmc_reg(struct work_struct *work)
>   	kref_put(&intf->refcount, intf_free);
>   }
>   
> -int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
> -		      void		       *send_info,
> -		      struct device            *si_dev,
> -		      unsigned char            slave_addr)
> +int ipmi_add_smi(struct module         *owner,
> +		 const struct ipmi_smi_handlers *handlers,
> +		 void		       *send_info,
> +		 struct device         *si_dev,
> +		 unsigned char         slave_addr)
>   {
>   	int              i, j;
>   	int              rv;
> @@ -3406,7 +3415,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
>   		return rv;
>   	}
>   
> -
> +	intf->owner = owner;
>   	intf->bmc = &intf->tmp_bmc;
>   	INIT_LIST_HEAD(&intf->bmc->intfs);
>   	mutex_init(&intf->bmc->dyn_mutex);
> @@ -3514,7 +3523,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
>   
>   	return rv;
>   }
> -EXPORT_SYMBOL(ipmi_register_smi);
> +EXPORT_SYMBOL(ipmi_add_smi);
>   
>   static void deliver_smi_err_response(struct ipmi_smi *intf,
>   				     struct ipmi_smi_msg *msg,
> diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h
> index 4dc66157d872..deec18b8944a 100644
> --- a/include/linux/ipmi_smi.h
> +++ b/include/linux/ipmi_smi.h
> @@ -224,10 +224,14 @@ static inline int ipmi_demangle_device_id(uint8_t netfn, uint8_t cmd,
>    * is called, and the lower layer must get the interface from that
>    * call.
>    */
> -int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
> -		      void                     *send_info,
> -		      struct device            *dev,
> -		      unsigned char            slave_addr);
> +int ipmi_add_smi(struct module            *owner,
> +		 const struct ipmi_smi_handlers *handlers,
> +		 void                     *send_info,
> +		 struct device            *dev,
> +		 unsigned char            slave_addr);
> +
> +#define ipmi_register_smi(handlers, send_info, dev, slave_addr) \
> +	ipmi_add_smi(THIS_MODULE, handlers, send_info, dev, slave_addr)
>   
>   /*
>    * Remove a low-level interface from the IPMI driver.  This will
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] ipmi: Don't allow device module unload when in use
  2019-10-22 14:29   ` Tony Camuso
@ 2019-10-22 15:56     ` Corey Minyard
  0 siblings, 0 replies; 8+ messages in thread
From: Corey Minyard @ 2019-10-22 15:56 UTC (permalink / raw)
  To: Tony Camuso; +Cc: openipmi-developer, linux-kernel, Corey Minyard

On Tue, Oct 22, 2019 at 10:29:12AM -0400, Tony Camuso wrote:
> Corey,
> 
> Testing shows that this patch works as expected.

Thanks, I'll add a Tested-by for you.  It's queued for the next merge
window.

-corey

> 
> Regards,
> Tony
> 
> 
> On 10/14/19 11:46 AM, minyard@acm.org wrote:
> > From: Corey Minyard <cminyard@mvista.com>
> > 
> > If something has the IPMI driver open, don't allow the device
> > module to be unloaded.  Before it would unload and the user would
> > get errors on use.
> > 
> > This change is made on user request, and it makes it consistent
> > with the I2C driver, which has the same behavior.
> > 
> > It does change things a little bit with respect to kernel users.
> > If the ACPI or IPMI watchdog (or any other kernel user) has
> > created a user, then the device module cannot be unloaded.  Before
> > it could be unloaded,
> > 
> > This does not affect hot-plug.  If the device goes away (it's on
> > something removable that is removed or is hot-removed via sysfs)
> > then it still behaves as it did before.
> > 
> > Reported-by: tony camuso <tcamuso@redhat.com>
> > Signed-off-by: Corey Minyard <cminyard@mvista.com>
> > ---
> > Tony, here is a suggested change for this.  Can you look it over and
> > see if it looks ok?
> > 
> > Thanks,
> > 
> > -corey
> > 
> >   drivers/char/ipmi/ipmi_msghandler.c | 23 ++++++++++++++++-------
> >   include/linux/ipmi_smi.h            | 12 ++++++++----
> >   2 files changed, 24 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> > index 2aab80e19ae0..15680de18625 100644
> > --- a/drivers/char/ipmi/ipmi_msghandler.c
> > +++ b/drivers/char/ipmi/ipmi_msghandler.c
> > @@ -448,6 +448,8 @@ enum ipmi_stat_indexes {
> >   #define IPMI_IPMB_NUM_SEQ	64
> >   struct ipmi_smi {
> > +	struct module *owner;
> > +
> >   	/* What interface number are we? */
> >   	int intf_num;
> > @@ -1220,6 +1222,11 @@ int ipmi_create_user(unsigned int          if_num,
> >   	if (rv)
> >   		goto out_kfree;
> > +	if (!try_module_get(intf->owner)) {
> > +		rv = -ENODEV;
> > +		goto out_kfree;
> > +	}
> > +	
> >   	/* Note that each existing user holds a refcount to the interface. */
> >   	kref_get(&intf->refcount);
> > @@ -1349,6 +1356,7 @@ static void _ipmi_destroy_user(struct ipmi_user *user)
> >   	}
> >   	kref_put(&intf->refcount, intf_free);
> > +	module_put(intf->owner);
> >   }
> >   int ipmi_destroy_user(struct ipmi_user *user)
> > @@ -2459,7 +2467,7 @@ static int __get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc)
> >    * been recently fetched, this will just use the cached data.  Otherwise
> >    * it will run a new fetch.
> >    *
> > - * Except for the first time this is called (in ipmi_register_smi()),
> > + * Except for the first time this is called (in ipmi_add_smi()),
> >    * this will always return good data;
> >    */
> >   static int __bmc_get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc,
> > @@ -3377,10 +3385,11 @@ static void redo_bmc_reg(struct work_struct *work)
> >   	kref_put(&intf->refcount, intf_free);
> >   }
> > -int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
> > -		      void		       *send_info,
> > -		      struct device            *si_dev,
> > -		      unsigned char            slave_addr)
> > +int ipmi_add_smi(struct module         *owner,
> > +		 const struct ipmi_smi_handlers *handlers,
> > +		 void		       *send_info,
> > +		 struct device         *si_dev,
> > +		 unsigned char         slave_addr)
> >   {
> >   	int              i, j;
> >   	int              rv;
> > @@ -3406,7 +3415,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
> >   		return rv;
> >   	}
> > -
> > +	intf->owner = owner;
> >   	intf->bmc = &intf->tmp_bmc;
> >   	INIT_LIST_HEAD(&intf->bmc->intfs);
> >   	mutex_init(&intf->bmc->dyn_mutex);
> > @@ -3514,7 +3523,7 @@ int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
> >   	return rv;
> >   }
> > -EXPORT_SYMBOL(ipmi_register_smi);
> > +EXPORT_SYMBOL(ipmi_add_smi);
> >   static void deliver_smi_err_response(struct ipmi_smi *intf,
> >   				     struct ipmi_smi_msg *msg,
> > diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h
> > index 4dc66157d872..deec18b8944a 100644
> > --- a/include/linux/ipmi_smi.h
> > +++ b/include/linux/ipmi_smi.h
> > @@ -224,10 +224,14 @@ static inline int ipmi_demangle_device_id(uint8_t netfn, uint8_t cmd,
> >    * is called, and the lower layer must get the interface from that
> >    * call.
> >    */
> > -int ipmi_register_smi(const struct ipmi_smi_handlers *handlers,
> > -		      void                     *send_info,
> > -		      struct device            *dev,
> > -		      unsigned char            slave_addr);
> > +int ipmi_add_smi(struct module            *owner,
> > +		 const struct ipmi_smi_handlers *handlers,
> > +		 void                     *send_info,
> > +		 struct device            *dev,
> > +		 unsigned char            slave_addr);
> > +
> > +#define ipmi_register_smi(handlers, send_info, dev, slave_addr) \
> > +	ipmi_add_smi(THIS_MODULE, handlers, send_info, dev, slave_addr)
> >   /*
> >    * Remove a low-level interface from the IPMI driver.  This will
> > 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-10-22 15:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20191014134141.GA25427@t560>
2019-10-14 15:46 ` [PATCH] ipmi: Don't allow device module unload when in use minyard
2019-10-16 12:18   ` tony camuso
2019-10-16 13:13     ` tony camuso
2019-10-16 19:25   ` Tony Camuso
2019-10-16 19:33     ` Corey Minyard
2019-10-16 19:54       ` Tony Camuso
2019-10-22 14:29   ` Tony Camuso
2019-10-22 15:56     ` Corey Minyard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).