Re: Linux 2.6.16-rc3

[James Bottomley]

On Tue, 2006-02-14 at 10:34 -0600, James Bottomley wrote:
> Well, I can't solve the problem that it requires memory allocation from
> IRQ context to operate.  Based on that, it's an unsafe interface.  I'm
> going to put it inside SCSI for 2.6.16, since it's better than what we
> have now, but I don't think we can export it globally.

OK, this is what I'm proposing as the device model fix.  What it does is
thread context checking APIs throughout the device subsystem.  SCSI can
then use it simply via device_put_process_context().  Since we have to
supply the kref_work; I'd plan to do that as an additional element in
struct scsi_device.

This, by itself, won't solve the SCSI target problem, but I plan to fix
that via a device model addition which would have target alloc waiting
around for any deleted targets to disappear.

Since this is planned for post 2.6.16, we have plenty of time to argue
about it.

James

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 6b355bd..4ae42de 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -338,10 +338,10 @@ struct device * get_device(struct device
  *	put_device - decrement reference count.
  *	@dev:	device in question.
  */
-void put_device(struct device * dev)
+void put_device_process_context(struct device * dev, struct kref_work *work)
 {
 	if (dev)
-		kobject_put(&dev->kobj);
+	  kobject_put_process_context(&dev->kobj, work);
 }
 
 
@@ -445,7 +445,7 @@ EXPORT_SYMBOL_GPL(device_register);
 EXPORT_SYMBOL_GPL(device_del);
 EXPORT_SYMBOL_GPL(device_unregister);
 EXPORT_SYMBOL_GPL(get_device);
-EXPORT_SYMBOL_GPL(put_device);
+EXPORT_SYMBOL_GPL(put_device_process_context);
 
 EXPORT_SYMBOL_GPL(device_create_file);
 EXPORT_SYMBOL_GPL(device_remove_file);
diff --git a/include/linux/device.h b/include/linux/device.h
index 58df18d..ac9d457 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -396,7 +396,13 @@ extern int (*platform_notify_remove)(str
  *
  */
 extern struct device * get_device(struct device * dev);
-extern void put_device(struct device * dev);
+extern void put_device_process_context(struct device * dev,
+				       struct kref_work *work);
+static inline void put_device(struct device *dev)
+{
+	put_device_process_context(dev, NULL);
+}
+						  
 
 
 /* drivers/base/power.c */
diff --git a/include/linux/kobject.h b/include/linux/kobject.h
index 2a8d8da..d079fea 100644
--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -76,7 +76,12 @@ extern int kobject_register(struct kobje
 extern void kobject_unregister(struct kobject *);
 
 extern struct kobject * kobject_get(struct kobject *);
-extern void kobject_put(struct kobject *);
+extern void kobject_put_process_context(struct kobject *, struct kref_work *);
+
+static inline void kobject_put(struct kobject *kobj)
+{
+	kobject_put_process_context(kobj, NULL);
+}
 
 extern char * kobject_get_path(struct kobject *, gfp_t);
 
diff --git a/include/linux/kref.h b/include/linux/kref.h
index 6fee353..16b15db 100644
--- a/include/linux/kref.h
+++ b/include/linux/kref.h
@@ -18,15 +18,29 @@
 #ifdef __KERNEL__
 
 #include <linux/types.h>
+#include <linux/workqueue.h>
 #include <asm/atomic.h>
 
 struct kref {
 	atomic_t refcount;
 };
 
+struct kref_work {
+	struct work_struct work;
+	struct kref *kref;
+	void (*release)(struct kref *kref);
+};
+
 void kref_init(struct kref *kref);
 void kref_get(struct kref *kref);
-int kref_put(struct kref *kref, void (*release) (struct kref *kref));
+int kref_put_process_context(struct kref *kref,
+			     void (*release) (struct kref *kref),
+			     struct kref_work *work);
+static inline int kref_put(struct kref *kref,
+			   void (*release) (struct kref *kref))
+{
+	return kref_put_process_context(kref, release, NULL);
+}
 
 #endif /* __KERNEL__ */
 #endif /* _KREF_H_ */
diff --git a/lib/kobject.c b/lib/kobject.c
index efe67fa..6b80c54 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -372,10 +372,10 @@ static void kobject_release(struct kref 
  *
  *	Decrement the refcount, and if 0, call kobject_cleanup().
  */
-void kobject_put(struct kobject * kobj)
+void kobject_put_process_context(struct kobject * kobj, struct kref_work *work)
 {
 	if (kobj)
-		kref_put(&kobj->kref, kobject_release);
+		kref_put_process_context(&kobj->kref, kobject_release, work);
 }
 
 
@@ -537,7 +537,7 @@ EXPORT_SYMBOL(kobject_init);
 EXPORT_SYMBOL(kobject_register);
 EXPORT_SYMBOL(kobject_unregister);
 EXPORT_SYMBOL(kobject_get);
-EXPORT_SYMBOL(kobject_put);
+EXPORT_SYMBOL(kobject_put_process_context);
 EXPORT_SYMBOL(kobject_add);
 EXPORT_SYMBOL(kobject_del);
 
diff --git a/lib/kref.c b/lib/kref.c
index 0d07cc3..66231cf 100644
--- a/lib/kref.c
+++ b/lib/kref.c
@@ -13,6 +13,7 @@
 
 #include <linux/kref.h>
 #include <linux/module.h>
+#include <linux/hardirq.h>
 
 /**
  * kref_init - initialize object.
@@ -33,27 +34,47 @@ void kref_get(struct kref *kref)
 	atomic_inc(&kref->refcount);
 }
 
+static void kref_release_process_context(void *data)
+{
+	struct kref_work *work = data;
+
+	work->release(work->kref);
+}
+
 /**
- * kref_put - decrement refcount for object.
+ * kref_put_user - decrement refcount for object and put in user context
  * @kref: object.
  * @release: pointer to the function that will clean up the object when the
  *	     last reference to the object is released.
  *	     This pointer is required, and it is not acceptable to pass kfree
  *	     in as this function.
+ * @work:    pointer to a kref_work used to take the release through user
+ *	     context (may be null)
  *
- * Decrement the refcount, and if 0, call release().
+ * Decrement the refcount, and if 0, call release().  If work is not null
+ * execute release via schedule_work if not in process context.
  * Return 1 if the object was removed, otherwise return 0.  Beware, if this
  * function returns 0, you still can not count on the kref from remaining in
  * memory.  Only use the return value if you want to see if the kref is now
  * gone, not present.
  */
-int kref_put(struct kref *kref, void (*release)(struct kref *kref))
+int kref_put_process_context(struct kref *kref,
+			     void (*release)(struct kref *kref),
+			     struct kref_work *work)
 {
 	WARN_ON(release == NULL);
 	WARN_ON(release == (void (*)(struct kref *))kfree);
 
 	if (atomic_dec_and_test(&kref->refcount)) {
-		release(kref);
+		if (!work || !in_interrupt())
+			release(kref);
+		else {
+			INIT_WORK(&work->work, kref_release_process_context,
+				  work);
+			schedule_work(&work->work);
+		}
+			
+			
 		return 1;
 	}
 	return 0;
@@ -61,4 +82,4 @@ int kref_put(struct kref *kref, void (*r
 
 EXPORT_SYMBOL(kref_init);
 EXPORT_SYMBOL(kref_get);
-EXPORT_SYMBOL(kref_put);
+EXPORT_SYMBOL(kref_put_process_context);


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/