[RFC] add module reference counts to sysfs attribute files

From: Greg KH (greg@kroah.com)
Date: Wed Jul 02 2003 - 16:58:47 EST


Hi all,

Here's a patch against 2.5.74 that adds module reference counting to
sysfs attribute files. We already protect kobject from going away when
attribute files are open, but we also need to protect the code that the
kobject is referencing from going away too. This patch fixes that hole,
and as a nice side benefit allows drivers to put attributes in kobjects
that they don't necessarily own (but you still have to be careful about
how you do this, grab the reference first, add the files. then on exit,
delete the files and then decrement the count.)

This patch works for me on testing of files in pci devices and
/sys/class/usb_host/ as well as other places in sysfs. Nice part of
this patch is that nothing is needed to be changed for drivers that
already use the *_ATTR() macros to create files. If you do not use
them, then you have to set the .owner field on your own.

If no one has any objections to it I'll send it on to Linus in a bit.

thanks,

greg k-h

p.s. You will also need a jiffies cleanup patch which I've included at
the end of this patch if you want to build this. I've told the author
of that file what needs to be fixed so that shouldn't be necessary for
long.

# SYSFS: add module referencing to sysfs attribute files.

diff -Nru a/fs/sysfs/file.c b/fs/sysfs/file.c
--- a/fs/sysfs/file.c Wed Jul 2 14:35:26 2003
+++ b/fs/sysfs/file.c Wed Jul 2 14:35:26 2003
@@ -247,6 +247,12 @@
         if (!kobj || !attr)
                 goto Einval;
 
+ /* Grab the module reference for this attribute if we have one */
+ if (!try_module_get(attr->owner)) {
+ error = -ENODEV;
+ goto Done;
+ }
+
         /* if the kobject has no ktype, then we assume that it is a subsystem
          * itself, and use ops for it.
          */
@@ -300,6 +306,7 @@
         goto Done;
  Eaccess:
         error = -EACCES;
+ module_put(attr->owner);
  Done:
         if (error && kobj)
                 kobject_put(kobj);
@@ -314,10 +321,12 @@
 static int sysfs_release(struct inode * inode, struct file * filp)
 {
         struct kobject * kobj = filp->f_dentry->d_parent->d_fsdata;
+ struct attribute * attr = filp->f_dentry->d_fsdata;
         struct sysfs_buffer * buffer = filp->private_data;
 
         if (kobj)
                 kobject_put(kobj);
+ module_put(attr->owner);
 
         if (buffer) {
                 if (buffer->page)
diff -Nru a/include/linux/device.h b/include/linux/device.h
--- a/include/linux/device.h Wed Jul 2 14:35:26 2003
+++ b/include/linux/device.h Wed Jul 2 14:35:26 2003
@@ -18,6 +18,7 @@
 #include <linux/spinlock.h>
 #include <linux/types.h>
 #include <linux/ioport.h>
+#include <linux/module.h>
 #include <asm/semaphore.h>
 #include <asm/atomic.h>
 
@@ -95,7 +96,7 @@
 
 #define BUS_ATTR(_name,_mode,_show,_store) \
 struct bus_attribute bus_attr_##_name = { \
- .attr = {.name = __stringify(_name), .mode = _mode }, \
+ .attr = {.name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE }, \
         .show = _show, \
         .store = _store, \
 };
@@ -136,7 +137,7 @@
 
 #define DRIVER_ATTR(_name,_mode,_show,_store) \
 struct driver_attribute driver_attr_##_name = { \
- .attr = {.name = __stringify(_name), .mode = _mode }, \
+ .attr = {.name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE }, \
         .show = _show, \
         .store = _store, \
 };
@@ -176,7 +177,7 @@
 
 #define CLASS_ATTR(_name,_mode,_show,_store) \
 struct class_attribute class_attr_##_name = { \
- .attr = {.name = __stringify(_name), .mode = _mode }, \
+ .attr = {.name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE }, \
         .show = _show, \
         .store = _store, \
 };
@@ -226,7 +227,7 @@
 
 #define CLASS_DEVICE_ATTR(_name,_mode,_show,_store) \
 struct class_device_attribute class_device_attr_##_name = { \
- .attr = {.name = __stringify(_name), .mode = _mode }, \
+ .attr = {.name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE }, \
         .show = _show, \
         .store = _store, \
 };
@@ -324,7 +325,7 @@
 
 #define DEVICE_ATTR(_name,_mode,_show,_store) \
 struct device_attribute dev_attr_##_name = { \
- .attr = {.name = __stringify(_name), .mode = _mode }, \
+ .attr = {.name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE }, \
         .show = _show, \
         .store = _store, \
 };
diff -Nru a/include/linux/sysfs.h b/include/linux/sysfs.h
--- a/include/linux/sysfs.h Wed Jul 2 14:35:26 2003
+++ b/include/linux/sysfs.h Wed Jul 2 14:35:26 2003
@@ -10,9 +10,11 @@
 #define _SYSFS_H_
 
 struct kobject;
+struct module;
 
 struct attribute {
         char * name;
+ struct module * owner;
         mode_t mode;
 };
 

# fix jiffies compiler warning.

diff -Nru a/arch/i386/kernel/timers/timer_tsc.c b/arch/i386/kernel/timers/timer_tsc.c
--- a/arch/i386/kernel/timers/timer_tsc.c Wed Jul 2 14:52:20 2003
+++ b/arch/i386/kernel/timers/timer_tsc.c Wed Jul 2 14:52:20 2003
@@ -21,7 +21,6 @@
 int tsc_disable __initdata = 0;
 
 extern spinlock_t i8253_lock;
-extern unsigned long jiffies;
 
 static int use_tsc;
 /* Number of usecs that the last interrupt was delayed */
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Mon Jul 07 2003 - 22:00:17 EST