diff mbox series

[1/2] driver core: tell caller if the device/kboject is really released

Message ID 20210930052028.934747-2-ming.lei@redhat.com (mailing list archive)
State Superseded
Headers show
Series SCSI: fix race between releasing shost and unloading LLD module | expand

Commit Message

Ming Lei Sept. 30, 2021, 5:20 a.m. UTC
Return if the device/kobject is really released to caller.

One use case is scsi_device_put() and the scsi device's release handler
runs async work to clean up things. We have to piggyback the module_put()
into the async work for avoiding to touch unmapped module page.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/base/core.c     | 5 +++--
 include/linux/device.h  | 2 +-
 include/linux/kobject.h | 2 +-
 lib/kobject.c           | 5 +++--
 4 files changed, 8 insertions(+), 6 deletions(-)

Comments

Greg KH Sept. 30, 2021, 5:51 a.m. UTC | #1
On Thu, Sep 30, 2021 at 01:20:27PM +0800, Ming Lei wrote:
> Return if the device/kobject is really released to caller.
> 
> One use case is scsi_device_put() and the scsi device's release handler
> runs async work to clean up things. We have to piggyback the module_put()
> into the async work for avoiding to touch unmapped module page.
> 
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  drivers/base/core.c     | 5 +++--
>  include/linux/device.h  | 2 +-
>  include/linux/kobject.h | 2 +-
>  lib/kobject.c           | 5 +++--
>  4 files changed, 8 insertions(+), 6 deletions(-)

I really don't like this as you should not ever care if you are
releasing the last reference on an object or not.

Why are you needing this?

And if you really do need this, you MUST document how this works in the
apis you are changing here, so I can't take this as is sorry.

greg k-h
Ming Lei Sept. 30, 2021, 7:22 a.m. UTC | #2
On Thu, Sep 30, 2021 at 07:51:35AM +0200, Greg Kroah-Hartman wrote:
> On Thu, Sep 30, 2021 at 01:20:27PM +0800, Ming Lei wrote:
> > Return if the device/kobject is really released to caller.
> > 
> > One use case is scsi_device_put() and the scsi device's release handler
> > runs async work to clean up things. We have to piggyback the module_put()
> > into the async work for avoiding to touch unmapped module page.
> > 
> > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > ---
> >  drivers/base/core.c     | 5 +++--
> >  include/linux/device.h  | 2 +-
> >  include/linux/kobject.h | 2 +-
> >  lib/kobject.c           | 5 +++--
> >  4 files changed, 8 insertions(+), 6 deletions(-)
> 
> I really don't like this as you should not ever care if you are
> releasing the last reference on an object or not.
> 
> Why are you needing this?
> 
> And if you really do need this, you MUST document how this works in the
> apis you are changing here, so I can't take this as is sorry.

Please ignore this series, and I have one better approach to address the
issue, will send it out soon.


Thanks,
Ming
diff mbox series

Patch

diff --git a/drivers/base/core.c b/drivers/base/core.c
index e65dd803a453..cd1365a934b9 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3459,11 +3459,12 @@  EXPORT_SYMBOL_GPL(get_device);
  * put_device - decrement reference count.
  * @dev: device in question.
  */
-void put_device(struct device *dev)
+int put_device(struct device *dev)
 {
 	/* might_sleep(); */
 	if (dev)
-		kobject_put(&dev->kobj);
+		return kobject_put(&dev->kobj);
+	return 0;
 }
 EXPORT_SYMBOL_GPL(put_device);
 
diff --git a/include/linux/device.h b/include/linux/device.h
index e270cb740b9e..ab089d743667 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -949,7 +949,7 @@  extern int (*platform_notify_remove)(struct device *dev);
  *
  */
 struct device *get_device(struct device *dev);
-void put_device(struct device *dev);
+int put_device(struct device *dev);
 bool kill_device(struct device *dev);
 
 #ifdef CONFIG_DEVTMPFS
diff --git a/include/linux/kobject.h b/include/linux/kobject.h
index ea30529fba08..c83cc8a7a170 100644
--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -111,7 +111,7 @@  extern int __must_check kobject_move(struct kobject *, struct kobject *);
 extern struct kobject *kobject_get(struct kobject *kobj);
 extern struct kobject * __must_check kobject_get_unless_zero(
 						struct kobject *kobj);
-extern void kobject_put(struct kobject *kobj);
+extern int kobject_put(struct kobject *kobj);
 
 extern const void *kobject_namespace(struct kobject *kobj);
 extern void kobject_get_ownership(struct kobject *kobj,
diff --git a/lib/kobject.c b/lib/kobject.c
index ea53b30cf483..7ebdd6b99064 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -743,15 +743,16 @@  static void kobject_release(struct kref *kref)
  *
  * Decrement the refcount, and if 0, call kobject_cleanup().
  */
-void kobject_put(struct kobject *kobj)
+int kobject_put(struct kobject *kobj)
 {
 	if (kobj) {
 		if (!kobj->state_initialized)
 			WARN(1, KERN_WARNING
 				"kobject: '%s' (%p): is not initialized, yet kobject_put() is being called.\n",
 			     kobject_name(kobj), kobj);
-		kref_put(&kobj->kref, kobject_release);
+		return kref_put(&kobj->kref, kobject_release);
 	}
+	return 0;
 }
 EXPORT_SYMBOL(kobject_put);