sysfs: make sysfs disconnect immediately on deletion, take 2
Subsystems (27)
PATCH 3/14 sysfs: move release_sysfs_dirent() to dir.c
In reply to Tejun HeoThere is no reason this function should be inlined and soon to follow sysfs object reference simplification will make it heavier. Move it to dir.c.
--- fs/sysfs/dir.c | 12 ++++++++++++ fs/sysfs/sysfs.h | 13 +------------ 2 files changed, 13 insertions(+), 12 deletions(-)
fs/sysfs/dir.c +12 -0
@@ -14,6 +14,18 @@DECLARE_RWSEM (sysfs_rename_sem); +void release_sysfs_dirent (struct sysfs_dirent * sd) +{ +if (sd->s_type &SYSFS_KOBJ_LINK ) { +struct sysfs_symlink * sl = sd->s_element; +kfree (sl->link_name); +kobject_put (sl->target_kobj); +kfree (sl); + } +kfree (sd->s_iattr); +kmem_cache_free (sysfs_dir_cachep, sd); +} +static void sysfs_d_iput (struct dentry * dentry,struct inode * inode) {struct sysfs_dirent * sd = dentry->d_fsdata;
fs/sysfs/sysfs.h +1 -12
@@ -17,6 +17,7 @@ extern void sysfs_delete_inode(struct inode *inode);extern struct inode *sysfs_new_inode (mode_t mode,struct sysfs_dirent *);extern int sysfs_create (struct dentry *,int mode,int (*init)(struct inode *)); +extern void release_sysfs_dirent (struct sysfs_dirent * sd);extern int sysfs_dirent_exist (struct sysfs_dirent *,const unsigned char *);extern int sysfs_make_dirent (struct sysfs_dirent *,struct dentry *,void *,umode_t ,int );
@@ -97,18 +98,6 @@ static inline struct kobject *sysfs_get_kobject(struct dentry *dentry)return kobj; } -static inline void release_sysfs_dirent (struct sysfs_dirent * sd) -{ -if (sd->s_type &SYSFS_KOBJ_LINK ) { -struct sysfs_symlink * sl = sd->s_element; -kfree (sl->link_name); -kobject_put (sl->target_kobj); -kfree (sl); - } -kfree (sd->s_iattr); -kmem_cache_free (sysfs_dir_cachep, sd); -} -static inline struct sysfs_dirent *sysfs_get (struct sysfs_dirent * sd) {if (sd) {
Signature · 1 line
1.5.0.3
PATCH 1/1 sysfs: make sysfs disconnect immediately on deletion, take 2
Hello, all.
This is the second take of sysfs-immediate-disconnct patchset.
In the last take, rwsem was added to s_elem.dir to protect kobj only. This wasn't enough because attr and bin_attr need to hold onto not only the kobject of their parents but also the module backing themselves and ops too, so the first set still needed separate and duplicate attribute file orphaning mechanism.
In this take, the rwsem is generalized to become active reference count. Now each sysfs_dirent has two reference counts - s_count and s_active. s_count is a regular reference count which guarantees that the containing sysfs_dirent is accessible. As long as s_count reference is held, all sysfs internal fields in sysfs_dirent are accessible including s_parent and s_name.
The newly added s_active is active reference count. This is acquired by invoking sysfs_get_active() and it's the caller's responsibility to ensure sysfs_dirent itself is accessible (should be holding s_count one way or the other). Dereferencing sysfs_dirent to access objects out of sysfs proper requires active reference. This includes access to the associated kobjects, attributes and ops.
Because attr/bin_attr ops access both the node itself and its parent for kobject, they need to hold active references to both. sysfs_get/put_active_two() helpers are provided to help grabbing both references. Parent's is acquired first and released last.
Basically, s_count provides the reference counted objects to the upper layer while s_active guards low level access such that low level objects can just go away when they want to, and the same mechanism is applied to all types of sysfs nodes. I think it's conceptually cleaner and thus easier to understand this way.
With all the patches applied, the same test used in the last take ran 9+hrs without any problem.
Change from the last take are...
* Patch 3 now doesn't move sysfs_get_kobject() as the it's replaced by active references later.
* Patch 12 updated such that sdir->rwsem is generalized into active reference count.
Please read the original lifetime rules discussion[1] and description of the last take[2] for more info.
Thanks.
--
tejun
[1] http://thread.gmane.org/gmane.linux.kernel/510293
[2] http://thread.gmane.org/gmane.linux.kernel/513334
PATCH 4/14 sysfs: flatten cleanup paths in sysfs_add_link() and create_dir()
Flatten cleanup paths in sysfs_add_link() and create_dir() to improve readability and ease further changes to these functions. This is in preparation of object reference simplification.
--- fs/sysfs/dir.c | 73 ++++++++++++++++++++++++++++++--------------------- fs/sysfs/symlink.c | 27 ++++++++++-------- 2 files changed, 58 insertions(+), 42 deletions(-)
fs/sysfs/dir.c +43 -30
@@ -159,40 +159,53 @@ static int init_symlink(struct inode * inode)return 0 ; } -static int create_dir (struct kobject * k,struct dentry * p, -const char * n,struct dentry ** d) +static int create_dir (struct kobject *kobj,struct dentry *parent, +const char *name,struct dentry **p_dentry) {int error;umode_t mode =S_IFDIR |S_IRWXU |S_IRUGO |S_IXUGO ; +struct dentry *dentry; +struct sysfs_dirent *sd; -mutex_lock (&p->d_inode->i_mutex); - *d =lookup_one_len (n, p,strlen (n)); -if (!IS_ERR (*d)) { -if (sysfs_dirent_exist (p->d_fsdata, n)) - error = -EEXIST ; -else - error =sysfs_make_dirent (p->d_fsdata, *d, k, mode, -SYSFS_DIR ); -if (!error) { - error =sysfs_create (*d, mode, init_dir); -if (!error) { -inc_nlink (p->d_inode); - (*d)->d_op = &sysfs_dentry_ops; -d_rehash (*d); - } - } -if (error && (error != -EEXIST )) { -struct sysfs_dirent *sd = (*d)->d_fsdata; -if (sd) { -list_del_init (&sd->s_sibling); -sysfs_put (sd); - } -d_drop (*d); - } -dput (*d); - }else - error =PTR_ERR (*d); -mutex_unlock (&p->d_inode->i_mutex); +mutex_lock (&parent->d_inode->i_mutex); + + dentry =lookup_one_len (name, parent,strlen (name)); +if (IS_ERR (dentry)) { + error =PTR_ERR (dentry); + goto out_unlock; + } + + error = -EEXIST ; +if (sysfs_dirent_exist (parent->d_fsdata, name)) + goto out_dput; + + error =sysfs_make_dirent (parent->d_fsdata, dentry, kobj, mode, +SYSFS_DIR ); +if (error) + goto out_drop; + + error =sysfs_create (dentry, mode, init_dir); +if (error) + goto out_sput; + +inc_nlink (parent->d_inode); + dentry->d_op = &sysfs_dentry_ops; +d_rehash (dentry); + + *p_dentry = dentry; + error =0 ; + goto out_dput; + + out_sput: + sd = dentry->d_fsdata; +list_del_init (&sd->s_sibling); +sysfs_put (sd); + out_drop: +d_drop (dentry); + out_dput: +dput (dentry); + out_unlock: +mutex_unlock (&parent->d_inode->i_mutex);return error; }
fs/sysfs/symlink.c +15 -12
@@ -49,30 +49,33 @@ static int sysfs_add_link(struct dentry * parent, const char * name, struct kobj {struct sysfs_dirent * parent_sd = parent->d_fsdata;struct sysfs_symlink * sl; -int error =0 ; +int error; error = -ENOMEM ; - sl =kmalloc (sizeof (*sl),GFP_KERNEL ); + sl =kzalloc (sizeof (*sl),GFP_KERNEL );if (!sl) - goto exit1; + goto err_out; sl->link_name =kmalloc (strlen (name) +1 ,GFP_KERNEL );if (!sl->link_name) - goto exit2; + goto err_out;strcpy (sl->link_name, name); sl->target_kobj =kobject_get (target); error =sysfs_make_dirent (parent_sd,NULL , sl,S_IFLNK |S_IRWXUGO ,SYSFS_KOBJ_LINK ); -if (!error) -return 0 ; - -kobject_put (target); -kfree (sl->link_name); -exit2: -kfree (sl); -exit1: +if (error) + goto err_out; + +return 0 ; + + err_out: +if (sl) { +kobject_put (sl->target_kobj); +kfree (sl->link_name); +kfree (sl); + }return error; }
Signature · 1 line
1.5.0.3
PATCH 13/14 sysfs: kill attribute file orphaning
In reply to Tejun HeoNow that sysfs_dirent can be disconnected from kobject on deletion, there is no need to orphan each attribute files. All [bin_]attribute nodes are automatically orphaned when the parent node is deleted. Kill attribute file orphaning.
--- fs/sysfs/file.c | 65 ++++++++++------------------------------------------- fs/sysfs/inode.c | 25 -------------------- fs/sysfs/mount.c | 8 ------ fs/sysfs/sysfs.h | 16 ------------- 4 files changed, 13 insertions(+), 101 deletions(-)
fs/sysfs/file.c +13 -52
@@ -51,29 +51,15 @@ static struct sysfs_ops subsys_sysfs_ops = { .store = subsys_attr_store, }; -/** -* add_to_collection - add buffer to a collection -* @buffer: buffer to be added -* @node: inode of set to add to -*/ - -static inline void -add_to_collection (struct sysfs_buffer *buffer,struct inode *node) -{ -struct sysfs_buffer_collection *set = node->i_private; - -mutex_lock (&node->i_mutex); -list_add (&buffer->associates, &set->associates); -mutex_unlock (&node->i_mutex); -} - -static inline void -remove_from_collection (struct sysfs_buffer *buffer,struct inode *node) -{ -mutex_lock (&node->i_mutex); -list_del (&buffer->associates); -mutex_unlock (&node->i_mutex); -} +struct sysfs_buffer { +size_t count; +loff_t pos; +char * page; +struct sysfs_ops * ops; +struct semaphore sem; +int needs_read_fill; +int event; +};/** * fill_read_buffer - allocate and fill buffer from object.
@@ -175,10 +161,7 @@ sysfs_read_file(struct file *file, char __user *buf, size_t count, loff_t *ppos)down (&buffer->sem);if (buffer->needs_read_fill) { -if (buffer->orphaned) - retval = -ENODEV ; -else - retval =fill_read_buffer (file->f_path.dentry,buffer); + retval =fill_read_buffer (file->f_path.dentry,buffer);if (retval) goto out; }
@@ -276,16 +259,11 @@ sysfs_write_file(struct file *file, const char __user *buf, size_t count, loff_tssize_t len;down (&buffer->sem); -if (buffer->orphaned) { - len = -ENODEV ; - goto out; - } len =fill_write_buffer (buffer, buf, count);if (len >0 ) len =flush_write_buffer (file->f_path.dentry, buffer, len);if (len >0 ) *ppos += len; -out:up (&buffer->sem);return len; }
@@ -295,7 +273,6 @@ static int sysfs_open_file(struct inode *inode, struct file *file)struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;struct attribute *attr = attr_sd->s_elem.attr.attr;struct kobject *kobj = attr_sd->s_parent->s_elem.dir.kobj; -struct sysfs_buffer_collection *set;struct sysfs_buffer * buffer;struct sysfs_ops * ops =NULL ;int error;
@@ -319,26 +296,14 @@ static int sysfs_open_file(struct inode *inode, struct file *file)else ops = &subsys_sysfs_ops; + error = -EACCES ; +/* No sysfs operations, either from having no subsystem, * or the subsystem have no operations. */ - error = -EACCES ;if (!ops) goto err_mput; -/* make sure we have a collection to add our buffers to */ -mutex_lock (&inode->i_mutex); -if (!(set = inode->i_private)) { - error = -ENOMEM ; -if (!(set = inode->i_private =kmalloc (sizeof (struct sysfs_buffer_collection ),GFP_KERNEL ))) - goto err_mput; -else -INIT_LIST_HEAD (&set->associates); - } -mutex_unlock (&inode->i_mutex); - - error = -EACCES ; -/* File needs write support. * The inode's perms must say it's ok, * and we must have a store method.
@@ -365,11 +330,9 @@ static int sysfs_open_file(struct inode *inode, struct file *file)if (!buffer) goto err_mput; -INIT_LIST_HEAD (&buffer->associates);init_MUTEX (&buffer->sem); buffer->needs_read_fill =1 ; buffer->ops = ops; -add_to_collection (buffer, inode); file->private_data = buffer;/* open succeeded, put active references and pin attr_sd */
@@ -388,10 +351,8 @@ static int sysfs_release(struct inode * inode, struct file * filp) {struct sysfs_dirent *attr_sd = filp->f_path.dentry->d_fsdata;struct attribute *attr = attr_sd->s_elem.attr.attr; -struct sysfs_buffer * buffer = filp->private_data; +struct sysfs_buffer *buffer = filp->private_data; -if (buffer) -remove_from_collection (buffer, inode);sysfs_put (attr_sd);/* After this point, attr should not be accessed. */ module_put (attr->owner);
fs/sysfs/inode.c +0 -25
@@ -190,24 +190,6 @@ int sysfs_create(struct dentry * dentry, int mode, int (*init)(struct inode *))return error; } -static inline void orphan_all_buffers (struct inode *node) -{ -struct sysfs_buffer_collection *set; -struct sysfs_buffer *buf; - -mutex_lock_nested (&node->i_mutex,I_MUTEX_CHILD ); - set = node->i_private; -if (set) { -list_for_each_entry (buf, &set->associates, associates) { -down (&buf->sem); - buf->orphaned =1 ; -up (&buf->sem); - } - } -mutex_unlock (&node->i_mutex); -} - -/* * Unhashes the dentry corresponding to given sysfs_dirent * Called with parent inode's i_mutex held.
@@ -215,23 +197,16 @@ static inline void orphan_all_buffers(struct inode *node)void sysfs_drop_dentry (struct sysfs_dirent * sd,struct dentry * parent) {struct dentry * dentry = sd->s_dentry; -struct inode *inode;if (dentry) {spin_lock (&dcache_lock);spin_lock (&dentry->d_lock);if (!(d_unhashed (dentry) && dentry->d_inode)) { - inode = dentry->d_inode; -spin_lock (&inode->i_lock); -__iget (inode); -spin_unlock (&inode->i_lock);dget_locked (dentry);__d_drop (dentry);spin_unlock (&dentry->d_lock);spin_unlock (&dcache_lock);simple_unlink (parent->d_inode, dentry); -orphan_all_buffers (inode); -iput (inode); }else {spin_unlock (&dentry->d_lock);spin_unlock (&dcache_lock);
fs/sysfs/mount.c +0 -8
@@ -19,12 +19,9 @@ struct vfsmount *sysfs_mount;struct super_block * sysfs_sb =NULL ;struct kmem_cache *sysfs_dir_cachep; -static void sysfs_clear_inode (struct inode *inode); -static const struct super_operations sysfs_ops = { .statfs = simple_statfs, .drop_inode = sysfs_delete_inode, - .clear_inode = sysfs_clear_inode, };static struct sysfs_dirent sysfs_root = {
@@ -35,11 +32,6 @@ static struct sysfs_dirent sysfs_root = { .s_iattr = NULL, }; -static void sysfs_clear_inode (struct inode *inode) -{ -kfree (inode->i_private); -} -static int sysfs_fill_super (struct super_block *sb,void *data,int silent) {struct inode *inode;
fs/sysfs/sysfs.h +0 -16
@@ -75,22 +75,6 @@ extern const struct file_operations bin_fops;extern const struct inode_operations sysfs_dir_inode_operations;extern const struct inode_operations sysfs_symlink_inode_operations; -struct sysfs_buffer { -struct list_head associates; -size_t count; -loff_t pos; -char * page; -struct sysfs_ops * ops; -struct semaphore sem; -int orphaned; -int needs_read_fill; -int event; -}; - -struct sysfs_buffer_collection { -struct list_head associates; -}; -static inline struct sysfs_dirent *sysfs_get (struct sysfs_dirent * sd) {if (sd) {
Signature · 1 line
1.5.0.3
PATCH 7/14 sysfs: add sysfs_dirent->s_name
In reply to Tejun HeoAdd s_name to sysfs_dirent. This is to further reduce dependency to the associated dentry. Name is copied for directories and symlinks but not for attributes.
Where possible, name dereferences are converted to use sd->s_name. sysfs_symlink->link_name and sysfs_get_name() are unused now and removed.
This change allows symlink to be implemented using sysfs_dirent tree proper, which is the last remaining dentry-dependent sysfs walk.
--- fs/sysfs/dir.c | 33 +++++++++++++++++++++------------ fs/sysfs/file.c | 2 +- fs/sysfs/inode.c | 33 +-------------------------------- fs/sysfs/symlink.c | 8 +------- fs/sysfs/sysfs.h | 7 +++---- include/linux/sysfs.h | 1 + 6 files changed, 28 insertions(+), 56 deletions(-)
fs/sysfs/dir.c +21 -12
@@ -23,10 +23,11 @@ void release_sysfs_dirent(struct sysfs_dirent * sd)if (sd->s_type &SYSFS_KOBJ_LINK ) {struct sysfs_symlink * sl = sd->s_element; -kfree (sl->link_name);kobject_put (sl->target_kobj);kfree (sl); } +if (sd->s_type &SYSFS_COPY_NAME ) +kfree (sd->s_name);kfree (sd->s_iattr);kmem_cache_free (sysfs_dir_cachep, sd);
@@ -51,19 +52,30 @@ static struct dentry_operations sysfs_dentry_ops = { .d_iput = sysfs_d_iput, }; -struct sysfs_dirent *sysfs_new_dirent (void *element,umode_t mode,int type) +struct sysfs_dirent *sysfs_new_dirent (const char *name,void *element, +umode_t mode,int type) { +char *dup_name =NULL ;struct sysfs_dirent * sd; +if (type &SYSFS_COPY_NAME ) { + name = dup_name =kstrdup (name,GFP_KERNEL ); +if (!name) +return NULL ; + } + sd =kmem_cache_zalloc (sysfs_dir_cachep,GFP_KERNEL ); -if (!sd) +if (!sd) { +kfree (dup_name);return NULL ; + }atomic_set (&sd->s_count,1 );atomic_set (&sd->s_event,1 );INIT_LIST_HEAD (&sd->s_children);INIT_LIST_HEAD (&sd->s_sibling); + sd->s_name = name; sd->s_element = element; sd->s_mode = mode; sd->s_type = type;
@@ -100,8 +112,7 @@ int sysfs_dirent_exist(struct sysfs_dirent *parent_sd,list_for_each_entry (sd, &parent_sd->s_children, s_sibling) {if (sd->s_element) { -const unsigned char *existing =sysfs_get_name (sd); -if (strcmp (existing, new)) +if (strcmp (sd->s_name, new))continue ;else return -EEXIST ;
@@ -155,7 +166,7 @@ static int create_dir(struct kobject *kobj, struct dentry *parent, goto out_dput; error = -ENOMEM ; - sd =sysfs_new_dirent (kobj, mode,SYSFS_DIR ); + sd =sysfs_new_dirent (name, kobj, mode,SYSFS_DIR );if (!sd) goto out_drop;sysfs_attach_dirent (sd, parent->d_fsdata, dentry);
@@ -280,9 +291,7 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry,list_for_each_entry (sd, &parent_sd->s_children, s_sibling) {if (sd->s_type &SYSFS_NOT_PINNED ) { -const unsigned char * name =sysfs_get_name (sd); - -if (strcmp (name, dentry->d_name.name)) +if (strcmp (sd->s_name, dentry->d_name.name))continue ;if (sd->s_type &SYSFS_KOBJ_LINK )
@@ -472,7 +481,7 @@ static int sysfs_dir_open(struct inode *inode, struct file *file)struct sysfs_dirent * sd;mutex_lock (&dentry->d_inode->i_mutex); - sd =sysfs_new_dirent (NULL ,0 ,0 ); + sd =sysfs_new_dirent ("_DIR_" ,NULL ,0 ,0 );if (sd)sysfs_attach_dirent (sd, parent_sd,NULL );mutex_unlock (&dentry->d_inode->i_mutex);
@@ -539,7 +548,7 @@ static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir)if (!next->s_element)continue ; - name =sysfs_get_name (next); + name = next->s_name; len =strlen (name); ino = (unsigned long )next;
@@ -651,7 +660,7 @@ struct dentry *sysfs_create_shadow_dir(struct kobject *kobj)if (!shadow) goto nomem; - sd =sysfs_new_dirent (kobj, inode->i_mode,SYSFS_DIR ); + sd =sysfs_new_dirent ("_SHADOW_" , kobj, inode->i_mode,SYSFS_DIR );if (!sd) goto nomem;/* point to parent_sd but don't attach to it */
fs/sysfs/file.c +1 -1
@@ -484,7 +484,7 @@ int sysfs_add_file(struct dentry * dir, const struct attribute * attr, int type) goto out_unlock; } - sd =sysfs_new_dirent ((void *)attr, mode, type); + sd =sysfs_new_dirent (attr->name, (void *)attr, mode, type);if (!sd) { error = -ENOMEM ; goto out_unlock;
fs/sysfs/inode.c +1 -32
@@ -190,37 +190,6 @@ int sysfs_create(struct dentry * dentry, int mode, int (*init)(struct inode *))return error; } -/* -* Get the name for corresponding element represented by the given sysfs_dirent -*/ -const unsigned char *sysfs_get_name (struct sysfs_dirent *sd) -{ -struct attribute * attr; -struct bin_attribute * bin_attr; -struct sysfs_symlink * sl; - -BUG_ON (!sd || !sd->s_element); - -switch (sd->s_type) { -case SYSFS_DIR : -/* Always have a dentry so use that */ -return sd->s_dentry->d_name.name; - -case SYSFS_KOBJ_ATTR : - attr = sd->s_element; -return attr->name; - -case SYSFS_KOBJ_BIN_ATTR : - bin_attr = sd->s_element; -return bin_attr->attr.name; - -case SYSFS_KOBJ_LINK : - sl = sd->s_element; -return sl->link_name; - } -return NULL ; -} -static inline void orphan_all_buffers (struct inode *node) {struct sysfs_buffer_collection *set;
@@ -288,7 +257,7 @@ int sysfs_hash_and_remove(struct dentry * dir, const char * name)list_for_each_entry (sd, &parent_sd->s_children, s_sibling) {if (!sd->s_element)continue ; -if (!strcmp (sysfs_get_name (sd), name)) { +if (!strcmp (sd->s_name, name)) {list_del_init (&sd->s_sibling);sysfs_drop_dentry (sd, dir);sysfs_put (sd);
fs/sysfs/symlink.c +1 -7
@@ -57,14 +57,9 @@ static int sysfs_add_link(struct dentry * parent, const char * name, struct kobjif (!sl) goto err_out; - sl->link_name =kmalloc (strlen (name) +1 ,GFP_KERNEL ); -if (!sl->link_name) - goto err_out; - -strcpy (sl->link_name, name); sl->target_kobj =kobject_get (target); - sd =sysfs_new_dirent (sl,S_IFLNK |S_IRWXUGO ,SYSFS_KOBJ_LINK ); + sd =sysfs_new_dirent (name, sl,S_IFLNK |S_IRWXUGO ,SYSFS_KOBJ_LINK );if (!sd) goto err_out;sysfs_attach_dirent (sd, parent_sd,NULL );
@@ -74,7 +69,6 @@ static int sysfs_add_link(struct dentry * parent, const char * name, struct kobj err_out:if (sl) {kobject_put (sl->target_kobj); -kfree (sl->link_name);kfree (sl); }return error;
fs/sysfs/sysfs.h +3 -4
@@ -3,6 +3,7 @@ struct sysfs_dirent {struct sysfs_dirent * s_parent;struct list_head s_sibling;struct list_head s_children; +const char * s_name;void * s_element;int s_type;umode_t s_mode;
@@ -20,8 +21,8 @@ extern int sysfs_create(struct dentry *, int mode, int (*init)(struct inode *));extern void release_sysfs_dirent (struct sysfs_dirent * sd);extern int sysfs_dirent_exist (struct sysfs_dirent *,const unsigned char *); -extern struct sysfs_dirent *sysfs_new_dirent (void *element,umode_t mode, -int type); +extern struct sysfs_dirent *sysfs_new_dirent (const char *name,void *element, +umode_t mode,int type);extern void sysfs_attach_dirent (struct sysfs_dirent *sd,struct sysfs_dirent *parent_sd,struct dentry *dentry);
@@ -33,7 +34,6 @@ extern struct sysfs_dirent *sysfs_find(struct sysfs_dirent *dir, const char * naextern int sysfs_create_subdir (struct kobject *,const char *,struct dentry **);extern void sysfs_remove_subdir (struct dentry *); -extern const unsigned char *sysfs_get_name (struct sysfs_dirent *sd);extern void sysfs_drop_dentry (struct sysfs_dirent *sd,struct dentry *parent);extern int sysfs_setattr (struct dentry *dentry,struct iattr *iattr);
@@ -46,7 +46,6 @@ extern const struct inode_operations sysfs_dir_inode_operations;extern const struct inode_operations sysfs_symlink_inode_operations;struct sysfs_symlink { -char * link_name;struct kobject * target_kobj; };
include/linux/sysfs.h +1 -0
@@ -76,6 +76,7 @@ struct sysfs_ops {#define SYSFS_KOBJ_BIN_ATTR 0x0008#define SYSFS_KOBJ_LINK 0x0020#define SYSFS_NOT_PINNED (SYSFS_KOBJ_ATTR | SYSFS_KOBJ_BIN_ATTR | SYSFS_KOBJ_LINK) +#define SYSFS_COPY_NAME (SYSFS_DIR | SYSFS_KOBJ_LINK)#ifdef CONFIG_SYSFS
Signature · 1 line
1.5.0.3
PATCH 10/14 sysfs: reimplement symlink using sysfs_dirent tree
In reply to Tejun Heosysfs symlink is implemented by referencing dentry and kobject from sysfs_dirent - symlink entry references kobject, dentry is used to walk the tree. This complicates object lifetimes rules and is dangerous - for example, there is no way to tell to which module the target of a symlink belongs and referencing that kobject can make it linger after the module is gone.
This patch reimplements symlink using only sysfs_dirent tree. sd for a symlink points and holds reference to the target sysfs_dirent and all walking is done using sysfs_dirent tree. Simpler and safer.
Please read the following message for more info.
http://article.gmane.org/gmane.linux.kernel/510293
--- fs/sysfs/dir.c | 2 +- fs/sysfs/symlink.c | 88 +++++++++++++++++++++++++++------------------------ fs/sysfs/sysfs.h | 9 +++-- 3 files changed, 53 insertions(+), 46 deletions(-)
fs/sysfs/dir.c +1 -1
@@ -23,7 +23,7 @@ void release_sysfs_dirent(struct sysfs_dirent * sd) parent_sd = sd->s_parent;if (sd->s_type &SYSFS_KOBJ_LINK ) -kobject_put (sd->s_elem.symlink.target_kobj); +sysfs_put (sd->s_elem.symlink.target_sd);if (sd->s_type &SYSFS_COPY_NAME )kfree (sd->s_name);kfree (sd->s_iattr);
fs/sysfs/symlink.c +47 -41
162 lines not on this page · raw message
fs/sysfs/sysfs.h +5 -4
@@ -3,7 +3,7 @@ struct sysfs_elem_dir { };struct sysfs_elem_symlink { -struct kobject * target_kobj; +struct sysfs_dirent * target_sd; };struct sysfs_elem_attr {
@@ -98,10 +98,11 @@ static inline struct kobject *sysfs_get_kobject(struct dentry *dentry)spin_lock (&dcache_lock);if (!d_unhashed (dentry)) {struct sysfs_dirent * sd = dentry->d_fsdata; +if (sd->s_type &SYSFS_KOBJ_LINK ) - kobj =kobject_get (sd->s_elem.symlink.target_kobj); -else - kobj =kobject_get (sd->s_elem.dir.kobj); + sd = sd->s_elem.symlink.target_sd; + + kobj =kobject_get (sd->s_elem.dir.kobj); }spin_unlock (&dcache_lock);
Signature · 1 line
1.5.0.3
PATCH 12/14 sysfs: implement sysfs_dirent active reference and immediate disconnect
In reply to Tejun HeoOpening a sysfs node references its associated kobject, so userland can arbitrarily prolong lifetime of a kobject which complicates lifetime rules in drivers. This patch implements active reference and makes the association between kobject and sysfs immediately breakable.
Now each sysfs_dirent has two reference counts - s_count and s_active. s_count is a regular reference count which guarantees that the containing sysfs_dirent is accessible. As long as s_count reference is held, all sysfs internal fields in sysfs_dirent are accessible including s_parent and s_name.
The newly added s_active is active reference count. This is acquired by invoking sysfs_get_active() and it's the caller's responsibility to ensure sysfs_dirent itself is accessible (should be holding s_count one way or the other). Dereferencing sysfs_dirent to access objects out of sysfs proper requires active reference. This includes access to the associated kobjects, attributes and ops.
The active references can be drained and denied by calling sysfs_deactivate(). All sysfs_dirents must be deactivated after deletion but before the default reference is dropped. This enables immediate disconnect of sysfs nodes. Once a sysfs_dirent is deleted, it won't access any entity external to sysfs proper.
Because attr/bin_attr ops access both the node itself and its parent for kobject, they need to hold active references to both. sysfs_get/put_active_two() helpers are provided to help grabbing both references. Parent's is acquired first and released last.
Unlike other operations, mmapped area lingers on after mmap() is finished and the module implement implementing it and kobj need to stay referenced till all the mapped pages are gone. This is accomplished by holding one set of active references to the bin_attr and its parent if there have been any mmap during lifetime of an openfile. The references are dropped when the openfile is released.
This change makes sysfs lifetime rules independent from both kobject's and module's. It not only fixes several race conditions caused by sysfs not holding onto the proper module when referencing kobject, but also helps fixing and simplifying lifetime management in driver model and drivers by taking sysfs out of the equation.
Please read the following message for more info.
http://article.gmane.org/gmane.linux.kernel/510293
--- fs/sysfs/bin.c | 95 ++++++++++++++++++++++++++-------------- fs/sysfs/dir.c | 18 ++++++- fs/sysfs/file.c | 130 +++++++++++++++++++++++++++++++---------------------- fs/sysfs/inode.c | 8 +++- fs/sysfs/sysfs.h | 107 +++++++++++++++++++++++++++++++++++--------- 5 files changed, 245 insertions(+), 113 deletions(-)
fs/sysfs/bin.c +62 -33
@@ -23,6 +23,7 @@struct bin_buffer {struct mutex mutex;void *buffer; +int mmapped; }; static int
@@ -30,12 +31,20 @@ fill_read(struct dentry *dentry, char *buffer, loff_t off, size_t count) {struct sysfs_dirent *attr_sd = dentry->d_fsdata;struct bin_attribute *attr = attr_sd->s_elem.bin_attr.bin_attr; -struct kobject * kobj =to_kobj (dentry->d_parent); +struct kobject *kobj = attr_sd->s_parent->s_elem.dir.kobj; +int rc; + +/* need attr_sd for attr, its parent for kobj */ +if (!sysfs_get_active_two (attr_sd)) +return -ENODEV ; -if (!attr->read) -return -EIO ; + rc = -EIO ; +if (attr->read) + rc = attr->read (kobj, buffer, off, count); -return attr->read (kobj, buffer, off, count); +sysfs_put_active_two (attr_sd); + +return rc; } static ssize_t
@@ -79,12 +88,20 @@ flush_write(struct dentry *dentry, char *buffer, loff_t offset, size_t count) {struct sysfs_dirent *attr_sd = dentry->d_fsdata;struct bin_attribute *attr = attr_sd->s_elem.bin_attr.bin_attr; -struct kobject *kobj =to_kobj (dentry->d_parent); +struct kobject *kobj = attr_sd->s_parent->s_elem.dir.kobj; +int rc; + +/* need attr_sd for attr, its parent for kobj */ +if (!sysfs_get_active_two (attr_sd)) +return -ENODEV ; -if (!attr->write) -return -EIO ; + rc = -EIO ; +if (attr->write) + rc = attr->write (kobj, buffer, offset, count); -return attr->write (kobj, buffer, offset, count); +sysfs_put_active_two (attr_sd); + +return rc; } static ssize_t write(struct file *file, const char __user *userbuf,
@@ -124,14 +141,24 @@ static int mmap(struct file *file, struct vm_area_struct *vma)struct bin_buffer *bb = file->private_data;struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;struct bin_attribute *attr = attr_sd->s_elem.bin_attr.bin_attr; -struct kobject *kobj =to_kobj (file->f_path.dentry->d_parent); +struct kobject *kobj = attr_sd->s_parent->s_elem.dir.kobj;int rc; -if (!attr->mmap) -return -EINVAL ; -mutex_lock (&bb->mutex); - rc = attr->mmap (kobj, attr, vma); + +/* need attr_sd for attr, its parent for kobj */ +if (!sysfs_get_active_two (attr_sd)) +return -ENODEV ; + + rc = -EINVAL ; +if (attr->mmap) + rc = attr->mmap (kobj, attr, vma); + +if (rc ==0 && !bb->mmapped) + bb->mmapped =1 ; +else +sysfs_put_active_two (attr_sd); +mutex_unlock (&bb->mutex);return rc;
@@ -139,58 +166,60 @@ static int mmap(struct file *file, struct vm_area_struct *vma)static int open (struct inode * inode,struct file * file) { -struct kobject *kobj =sysfs_get_kobject (file->f_path.dentry->d_parent);struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;struct bin_attribute *attr = attr_sd->s_elem.bin_attr.bin_attr;struct bin_buffer *bb =NULL ; -int error = -EINVAL ; +int error; -if (!kobj || !attr) - goto Done; +/* need attr_sd for attr */ +if (!sysfs_get_active (attr_sd)) +return -ENODEV ; -/* Grab the module reference for this attribute if we have one */ +/* Grab the module reference for this attribute */ error = -ENODEV ; -if (!try_module_get (attr->attr.owner)) - goto Done; +if (!try_module_get (attr->attr.owner)) + goto err_sput; error = -EACCES ;if ((file->f_mode &FMODE_WRITE ) && !(attr->write || attr->mmap)) - goto Error; + goto err_mput;if ((file->f_mode &FMODE_READ ) && !(attr->read || attr->mmap)) - goto Error; + goto err_mput; error = -ENOMEM ; bb =kzalloc (sizeof (*bb),GFP_KERNEL );if (!bb) - goto Error; + goto err_mput; bb->buffer =kmalloc (PAGE_SIZE ,GFP_KERNEL );if (!bb->buffer) - goto Error; + goto err_mput;mutex_init (&bb->mutex); file->private_data = bb; - error =0 ; - goto Done; +/* open succeeded, put active reference and pin attr_sd */ +sysfs_put_active (attr_sd); +sysfs_get (attr_sd); +return 0 ; - Error: -kfree (bb); + err_mput:module_put (attr->attr.owner); - Done: -if (error) -kobject_put (kobj); + err_sput: +sysfs_put_active (attr_sd); +kfree (bb);return error; }static int release (struct inode * inode,struct file * file) { -struct kobject * kobj =to_kobj (file->f_path.dentry->d_parent);struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;struct bin_attribute *attr = attr_sd->s_elem.bin_attr.bin_attr;struct bin_buffer *bb = file->private_data; -kobject_put (kobj); +if (bb->mmapped) +sysfs_put_active_two (attr_sd); +sysfs_put (attr_sd);module_put (attr->attr.owner);kfree (bb->buffer);kfree (bb);
fs/sysfs/dir.c +15 -3
@@ -22,6 +22,9 @@ void release_sysfs_dirent(struct sysfs_dirent * sd) repeat: parent_sd = sd->s_parent; +/* s_active must have been write locked by sysfs_deactivate(), unlock */ +up_write (&sd->s_active); +if (sd->s_type &SYSFS_KOBJ_LINK )sysfs_put (sd->s_elem.symlink.target_sd);if (sd->s_type &SYSFS_COPY_NAME )
@@ -69,6 +72,7 @@ struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type)atomic_set (&sd->s_count,1 );atomic_set (&sd->s_event,1 ); +init_rwsem (&sd->s_active);INIT_LIST_HEAD (&sd->s_children);INIT_LIST_HEAD (&sd->s_sibling);
@@ -316,7 +320,6 @@ static void remove_dir(struct dentry * d)d_delete (d); sd = d->d_fsdata;list_del_init (&sd->s_sibling); -sysfs_put (sd);if (d->d_inode)simple_rmdir (parent->d_inode,d);
@@ -325,6 +328,9 @@ static void remove_dir(struct dentry * d)mutex_unlock (&parent->d_inode->i_mutex);dput (parent); + +sysfs_deactivate (sd); +sysfs_put (sd); }void sysfs_remove_subdir (struct dentry * d)
@@ -335,6 +341,7 @@ void sysfs_remove_subdir(struct dentry * d)static void __sysfs_remove_dir (struct dentry *dentry) { +LIST_HEAD (removed);struct sysfs_dirent * parent_sd;struct sysfs_dirent * sd, * tmp;
@@ -348,12 +355,17 @@ static void __sysfs_remove_dir(struct dentry *dentry)list_for_each_entry_safe (sd, tmp, &parent_sd->s_children, s_sibling) {if (!sd->s_type || !(sd->s_type &SYSFS_NOT_PINNED ))continue ; -list_del_init (&sd->s_sibling); +list_move (&sd->s_sibling, &removed);sysfs_drop_dentry (sd, dentry); -sysfs_put (sd); }mutex_unlock (&dentry->d_inode->i_mutex); +list_for_each_entry_safe (sd, tmp, &removed, s_sibling) { +list_del_init (&sd->s_sibling); +sysfs_deactivate (sd); +sysfs_put (sd); + } +remove_dir (dentry);/** * Drop reference from dget() on entrance.
fs/sysfs/file.c +76 -54
222 lines not on this page · raw message
fs/sysfs/inode.c +6 -2
@@ -260,12 +260,16 @@ int sysfs_hash_and_remove(struct dentry * dir, const char * name)if (!strcmp (sd->s_name, name)) {list_del_init (&sd->s_sibling);sysfs_drop_dentry (sd, dir); -sysfs_put (sd); found =1 ;break ; } }mutex_unlock (&dir->d_inode->i_mutex); -return found ?0 : -ENOENT ; +if (!found) +return -ENOENT ; + +sysfs_deactivate (sd); +sysfs_put (sd); +return 0 ; }
fs/sysfs/sysfs.h +86 -21
137 lines not on this page · raw message
Signature · 1 line
1.5.0.3
PATCH 14/14 sysfs: kill unnecessary attribute->owner
In reply to Tejun Heosysfs is now completely out of driver/module lifetime game. After deletion, a sysfs node doesn't access anything outside sysfs proper, so there's no reason to hold onto the attribute owners. Note that often the wrong modules were accounted for as owners leading to accessing removed modules.
This patch kills now unnecessary attribute->owner. Note that with this change, userland holding a sysfs node does not prevent the backing module from being unloaded.
For more info regarding lifetime rule cleanup, please read the following message.
http://article.gmane.org/gmane.linux.kernel/510293
--- drivers/base/class.c | 2 -- drivers/base/core.c | 4 ---- drivers/base/firmware_class.c | 2 +- drivers/block/pktcdvd.c | 3 +-- drivers/char/ipmi/ipmi_msghandler.c | 10 ---------- drivers/cpufreq/cpufreq_stats.c | 3 +-- drivers/cpufreq/cpufreq_userspace.c | 2 +- drivers/cpufreq/freq_table.c | 1 - drivers/firmware/dcdbas.h | 3 +-- drivers/firmware/dell_rbu.c | 6 +++--- drivers/firmware/edd.c | 2 +- drivers/firmware/efivars.c | 6 +++--- drivers/i2c/chips/eeprom.c | 1 - drivers/i2c/chips/max6875.c | 1 - drivers/infiniband/core/sysfs.c | 1 - drivers/input/mouse/psmouse.h | 1 - drivers/media/video/pvrusb2/pvrusb2-sysfs.c | 13 ------------- drivers/misc/asus-laptop.c | 3 +-- drivers/pci/hotplug/acpiphp_ibm.c | 1 - drivers/pci/pci-sysfs.c | 4 ---- drivers/pcmcia/socket_sysfs.c | 2 +- drivers/rtc/rtc-ds1553.c | 1 - drivers/rtc/rtc-ds1742.c | 1 - drivers/scsi/arcmsr/arcmsr_attr.c | 3 --- drivers/scsi/lpfc/lpfc_attr.c | 2 -- drivers/scsi/qla2xxx/qla_attr.c | 6 ------ drivers/spi/at25.c | 1 - drivers/video/aty/radeon_base.c | 2 -- drivers/video/backlight/backlight.c | 2 +- drivers/video/backlight/lcd.c | 2 +- drivers/w1/slaves/w1_ds2433.c | 1 - drivers/w1/slaves/w1_therm.c | 1 - drivers/w1/w1.c | 2 -- fs/ecryptfs/main.c | 2 -- fs/ocfs2/cluster/masklog.c | 1 - fs/partitions/check.c | 1 - fs/sysfs/bin.c | 19 +++++-------------- fs/sysfs/file.c | 21 +++++---------------- include/linux/sysdev.h | 3 +-- include/linux/sysfs.h | 7 +++---- kernel/module.c | 9 +++------ kernel/params.c | 1 - net/bridge/br_sysfs_br.c | 3 +-- net/bridge/br_sysfs_if.c | 3 +-- 44 files changed, 35 insertions(+), 130 deletions(-)
drivers/base/class.c +0 -2
@@ -624,7 +624,6 @@ int class_device_add(struct class_device *class_dev) goto out3; class_dev->uevent_attr.attr.name ="uevent" ; class_dev->uevent_attr.attr.mode =S_IWUSR ; - class_dev->uevent_attr.attr.owner = parent_class->owner; class_dev->uevent_attr.store = store_uevent; error =class_device_create_file (class_dev, &class_dev->uevent_attr);if (error)
@@ -639,7 +638,6 @@ int class_device_add(struct class_device *class_dev) } attr->attr.name ="dev" ; attr->attr.mode =S_IRUGO ; - attr->attr.owner = parent_class->owner; attr->show = show_dev; error =class_device_create_file (class_dev, attr);if (error) {
drivers/base/core.c +0 -4
@@ -563,8 +563,6 @@ int device_add(struct device *dev) dev->uevent_attr.attr.name ="uevent" ; dev->uevent_attr.attr.mode =S_IWUSR ; -if (dev->driver) - dev->uevent_attr.attr.owner = dev->driver->owner; dev->uevent_attr.store = store_uevent; error =device_create_file (dev, &dev->uevent_attr);if (error)
@@ -579,8 +577,6 @@ int device_add(struct device *dev) } attr->attr.name ="dev" ; attr->attr.mode =S_IRUGO ; -if (dev->driver) - attr->attr.owner = dev->driver->owner; attr->show = show_dev; error =device_create_file (dev, attr);if (error) {
drivers/base/firmware_class.c +1 -1
@@ -276,7 +276,7 @@ out: }static struct bin_attribute firmware_attr_data_tmpl = { - .attr = {.name ="data" , .mode =0644 , .owner =THIS_MODULE }, + .attr = {.name ="data" , .mode =0644 }, .size =0 , .read = firmware_data_read, .write = firmware_data_write,
drivers/block/pktcdvd.c +1 -2
@@ -146,8 +146,7 @@ static void pkt_kobj_release(struct kobject *kobj)**********************************************************/ #define DEF_ATTR (_obj,_name,_mode) \ - static struct attribute _obj = { \ - .name = _name, .owner = THIS_MODULE, .mode = _mode } + static struct attribute _obj = { .name = _name, .mode = _mode }/********************************************************** /sys/class/pktcdvd/pktcdvd[0-7]/
drivers/char/ipmi/ipmi_msghandler.c +0 -10
@@ -2171,52 +2171,42 @@ static int create_files(struct bmc_device *bmc)int err; bmc->device_id_attr.attr.name ="device_id" ; - bmc->device_id_attr.attr.owner =THIS_MODULE ; bmc->device_id_attr.attr.mode =S_IRUGO ; bmc->device_id_attr.show = device_id_show; bmc->provides_dev_sdrs_attr.attr.name ="provides_device_sdrs" ; - bmc->provides_dev_sdrs_attr.attr.owner =THIS_MODULE ; bmc->provides_dev_sdrs_attr.attr.mode =S_IRUGO ; bmc->provides_dev_sdrs_attr.show = provides_dev_sdrs_show; bmc->revision_attr.attr.name ="revision" ; - bmc->revision_attr.attr.owner =THIS_MODULE ; bmc->revision_attr.attr.mode =S_IRUGO ; bmc->revision_attr.show = revision_show; bmc->firmware_rev_attr.attr.name ="firmware_revision" ; - bmc->firmware_rev_attr.attr.owner =THIS_MODULE ; bmc->firmware_rev_attr.attr.mode =S_IRUGO ; bmc->firmware_rev_attr.show = firmware_rev_show; bmc->version_attr.attr.name ="ipmi_version" ; - bmc->version_attr.attr.owner =THIS_MODULE ; bmc->version_attr.attr.mode =S_IRUGO ; bmc->version_attr.show = ipmi_version_show; bmc->add_dev_support_attr.attr.name ="additional_device_support" ; - bmc->add_dev_support_attr.attr.owner =THIS_MODULE ; bmc->add_dev_support_attr.attr.mode =S_IRUGO ; bmc->add_dev_support_attr.show = add_dev_support_show; bmc->manufacturer_id_attr.attr.name ="manufacturer_id" ; - bmc->manufacturer_id_attr.attr.owner =THIS_MODULE ; bmc->manufacturer_id_attr.attr.mode =S_IRUGO ; bmc->manufacturer_id_attr.show = manufacturer_id_show; bmc->product_id_attr.attr.name ="product_id" ; - bmc->product_id_attr.attr.owner =THIS_MODULE ; bmc->product_id_attr.attr.mode =S_IRUGO ; bmc->product_id_attr.show = product_id_show; bmc->guid_attr.attr.name ="guid" ; - bmc->guid_attr.attr.owner =THIS_MODULE ; bmc->guid_attr.attr.mode =S_IRUGO ; bmc->guid_attr.show = guid_show; bmc->aux_firmware_rev_attr.attr.name ="aux_firmware_revision" ; - bmc->aux_firmware_rev_attr.attr.owner =THIS_MODULE ; bmc->aux_firmware_rev_attr.attr.mode =S_IRUGO ; bmc->aux_firmware_rev_attr.show = aux_firmware_rev_show;
drivers/cpufreq/cpufreq_stats.c +1 -2
@@ -25,8 +25,7 @@ static spinlock_t cpufreq_stats_lock;#define CPUFREQ_STATDEVICE_ATTR (_name,_mode,_show) \ static struct freq_attr _attr_##_name = {\ - .attr = {.name = __stringify(_name), .owner = THIS_MODULE, \ - .mode = _mode, }, \ + .attr = {.name = __stringify(_name), .mode = _mode, }, \ .show = _show,\ };
drivers/cpufreq/cpufreq_userspace.c +1 -1
@@ -120,7 +120,7 @@ store_speed (struct cpufreq_policy *policy, const char *buf, size_t count)static struct freq_attr freq_attr_scaling_setspeed = { - .attr = { .name ="scaling_setspeed" , .mode =0644 , .owner =THIS_MODULE }, + .attr = { .name ="scaling_setspeed" , .mode =0644 }, .show = show_speed, .store = store_speed, };
drivers/cpufreq/freq_table.c +0 -1
@@ -199,7 +199,6 @@ static ssize_t show_available_freqs (struct cpufreq_policy *policy, char *buf)struct freq_attr cpufreq_freq_attr_scaling_available_freqs = { .attr = { .name ="scaling_available_frequencies" , .mode =0444 , - .owner=THIS_MODULE }, .show = show_available_freqs, };
drivers/firmware/dcdbas.h +1 -2
@@ -67,8 +67,7 @@#define DCDBAS_BIN_ATTR_RW (_name) \ struct bin_attribute bin_attr_##_name = { \ .attr = { .name = __stringify(_name), \ - .mode = 0600, \ - .owner = THIS_MODULE }, \ + .mode = 0600 }, \ .read = _name##_read, \ .write = _name##_write, \ }
drivers/firmware/dell_rbu.c +3 -3
@@ -687,18 +687,18 @@ static ssize_t write_rbu_packet_size(struct kobject *kobj, char *buffer, }static struct bin_attribute rbu_data_attr = { - .attr = {.name ="data" ,.owner =THIS_MODULE ,.mode =0444 }, + .attr = {.name ="data" , .mode =0444 }, .read = read_rbu_data, };static struct bin_attribute rbu_image_type_attr = { - .attr = {.name ="image_type" ,.owner =THIS_MODULE ,.mode =0644 }, + .attr = {.name ="image_type" , .mode =0644 }, .read = read_rbu_image_type, .write = write_rbu_image_type, };static struct bin_attribute rbu_packet_size_attr = { - .attr = {.name ="packet_size" ,.owner =THIS_MODULE ,.mode =0644 }, + .attr = {.name ="packet_size" , .mode =0644 }, .read = read_rbu_packet_size, .write = write_rbu_packet_size, };
drivers/firmware/edd.c +1 -1
@@ -74,7 +74,7 @@ static struct edd_device *edd_devices[EDD_MBR_SIG_MAX];#define EDD_DEVICE_ATTR (_name,_mode,_show,_test) \ struct edd_attribute edd_attr_##_name = { \ - .attr = {.name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE }, \ + .attr = {.name = __stringify(_name), .mode = _mode }, \ .show = _show, \ .test = _test, \ };
drivers/firmware/efivars.c +3 -3
@@ -131,21 +131,21 @@ struct efivar_attribute {#define EFI_ATTR (_name, _mode, _show, _store) \ struct subsys_attribute efi_attr_##_name = { \ - .attr = {.name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE}, \ + .attr = {.name = __stringify(_name), .mode = _mode}, \ .show = _show, \ .store = _store, \ };#define EFIVAR_ATTR (_name, _mode, _show, _store) \ struct efivar_attribute efivar_attr_##_name = { \ - .attr = {.name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE}, \ + .attr = {.name = __stringify(_name), .mode = _mode}, \ .show = _show, \ .store = _store, \ };#define VAR_SUBSYS_ATTR (_name, _mode, _show, _store) \ struct subsys_attribute var_subsys_attr_##_name = { \ - .attr = {.name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE}, \ + .attr = {.name = __stringify(_name), .mode = _mode}, \ .show = _show, \ .store = _store, \ };
drivers/i2c/chips/eeprom.c +0 -1
@@ -143,7 +143,6 @@ static struct bin_attribute eeprom_attr = { .attr = { .name = "eeprom", .mode = S_IRUGO, - .owner = THIS_MODULE, }, .size = EEPROM_SIZE, .read = eeprom_read,
drivers/i2c/chips/max6875.c +0 -1
@@ -152,7 +152,6 @@ static struct bin_attribute user_eeprom_attr = { .attr = { .name = "eeprom", .mode = S_IRUGO, - .owner = THIS_MODULE, }, .size = USER_EEPROM_SIZE, .read = max6875_read,
drivers/infiniband/core/sysfs.c +0 -1
@@ -479,7 +479,6 @@ alloc_group_attrs(ssize_t (*show)(struct ib_port *, element->attr.attr.name = element->name; element->attr.attr.mode =S_IRUGO ; - element->attr.attr.owner =THIS_MODULE ; element->attr.show = show; element->index = i;
drivers/input/mouse/psmouse.h +0 -1
@@ -117,7 +117,6 @@ static struct psmouse_attribute psmouse_attr_##_name = { \ .attr = { \ .name = __stringify(_name), \ .mode = _mode, \ - .owner = THIS_MODULE, \ }, \ .show = psmouse_attr_show_helper, \ .store = psmouse_attr_set_helper, \
drivers/media/video/pvrusb2/pvrusb2-sysfs.c +0 -13
@@ -516,40 +516,32 @@ static void pvr2_sysfs_add_control(struct pvr2_sysfs *sfp,int ctl_id) } sfp->item_last = cip; - cip->attr_name.attr.owner =THIS_MODULE ; cip->attr_name.attr.name ="name" ; cip->attr_name.attr.mode =S_IRUGO ; cip->attr_name.show = fp->show_name; - cip->attr_type.attr.owner =THIS_MODULE ; cip->attr_type.attr.name ="type" ; cip->attr_type.attr.mode =S_IRUGO ; cip->attr_type.show = fp->show_type; - cip->attr_min.attr.owner =THIS_MODULE ; cip->attr_min.attr.name ="min_val" ; cip->attr_min.attr.mode =S_IRUGO ; cip->attr_min.show = fp->show_min; - cip->attr_max.attr.owner =THIS_MODULE ; cip->attr_max.attr.name ="max_val" ; cip->attr_max.attr.mode =S_IRUGO ; cip->attr_max.show = fp->show_max; - cip->attr_val.attr.owner =THIS_MODULE ; cip->attr_val.attr.name ="cur_val" ; cip->attr_val.attr.mode =S_IRUGO ; - cip->attr_custom.attr.owner =THIS_MODULE ; cip->attr_custom.attr.name ="custom_val" ; cip->attr_custom.attr.mode =S_IRUGO ; - cip->attr_enum.attr.owner =THIS_MODULE ; cip->attr_enum.attr.name ="enum_val" ; cip->attr_enum.attr.mode =S_IRUGO ; cip->attr_enum.show = fp->show_enum; - cip->attr_bits.attr.owner =THIS_MODULE ; cip->attr_bits.attr.name ="bit_val" ; cip->attr_bits.attr.mode =S_IRUGO ; cip->attr_bits.show = fp->show_bits;
@@ -614,12 +606,10 @@ static void pvr2_sysfs_add_debugifc(struct pvr2_sysfs *sfp) dip =kzalloc (sizeof (*dip),GFP_KERNEL );if (!dip)return ; - dip->attr_debugcmd.attr.owner =THIS_MODULE ; dip->attr_debugcmd.attr.name ="debugcmd" ; dip->attr_debugcmd.attr.mode =S_IRUGO |S_IWUSR |S_IWGRP ; dip->attr_debugcmd.show = debugcmd_show; dip->attr_debugcmd.store = debugcmd_store; - dip->attr_debuginfo.attr.owner =THIS_MODULE ; dip->attr_debuginfo.attr.name ="debuginfo" ; dip->attr_debuginfo.attr.mode =S_IRUGO ; dip->attr_debuginfo.show = debuginfo_show;
@@ -795,7 +785,6 @@ static void class_dev_create(struct pvr2_sysfs *sfp,return ; } - sfp->attr_v4l_minor_number.attr.owner =THIS_MODULE ; sfp->attr_v4l_minor_number.attr.name ="v4l_minor_number" ; sfp->attr_v4l_minor_number.attr.mode =S_IRUGO ; sfp->attr_v4l_minor_number.show = v4l_minor_number_show;
@@ -809,7 +798,6 @@ static void class_dev_create(struct pvr2_sysfs *sfp, sfp->v4l_minor_number_created_ok = !0 ; } - sfp->attr_v4l_radio_minor_number.attr.owner =THIS_MODULE ; sfp->attr_v4l_radio_minor_number.attr.name ="v4l_radio_minor_number" ; sfp->attr_v4l_radio_minor_number.attr.mode =S_IRUGO ; sfp->attr_v4l_radio_minor_number.show = v4l_radio_minor_number_show;
@@ -823,7 +811,6 @@ static void class_dev_create(struct pvr2_sysfs *sfp, sfp->v4l_radio_minor_number_created_ok = !0 ; } - sfp->attr_unit_number.attr.owner =THIS_MODULE ; sfp->attr_unit_number.attr.name ="unit_number" ; sfp->attr_unit_number.attr.mode =S_IRUGO ; sfp->attr_unit_number.show = unit_number_show;
drivers/misc/asus-laptop.c +1 -2
@@ -673,8 +673,7 @@ static void asus_hotk_notify(acpi_handle handle, u32 event, void *data) struct device_attribute dev_attr_##_name = { \ .attr = { \ .name =__stringify (_name), \ - .mode =0 , \ - .owner =THIS_MODULE }, \ + .mode =0 }, \ .show =NULL , \ .store =NULL , \ }
drivers/pci/hotplug/acpiphp_ibm.c +0 -1
@@ -117,7 +117,6 @@ static struct notification ibm_note;static struct bin_attribute ibm_apci_table_attr = { .attr = { .name ="apci_table" , - .owner =THIS_MODULE , .mode =S_IRUGO , }, .read = ibm_read_apci_table,
drivers/pci/pci-sysfs.c +0 -4
@@ -499,7 +499,6 @@ static int pci_create_resource_files(struct pci_dev *pdev)sprintf (res_attr_name,"resource%d" , i); res_attr->attr.name = res_attr_name; res_attr->attr.mode =S_IRUSR |S_IWUSR ; - res_attr->attr.owner =THIS_MODULE ; res_attr->size =pci_resource_len (pdev, i); res_attr->mmap = pci_mmap_resource; res_attr->private = &pdev->resource[i];
@@ -582,7 +581,6 @@ static struct bin_attribute pci_config_attr = { .attr = { .name = "config", .mode = S_IRUGO | S_IWUSR, - .owner = THIS_MODULE, }, .size = 256, .read = pci_read_config,
@@ -593,7 +591,6 @@ static struct bin_attribute pcie_config_attr = { .attr = { .name = "config", .mode = S_IRUGO | S_IWUSR, - .owner = THIS_MODULE, }, .size = 4096, .read = pci_read_config,
@@ -627,7 +624,6 @@ int __must_check pci_create_sysfs_dev_files (struct pci_dev *pdev) rom_attr->size =pci_resource_len (pdev,PCI_ROM_RESOURCE ); rom_attr->attr.name ="rom" ; rom_attr->attr.mode =S_IRUSR ; - rom_attr->attr.owner =THIS_MODULE ; rom_attr->read = pci_read_rom; rom_attr->write = pci_write_rom; retval =sysfs_create_bin_file (&pdev->dev.kobj, rom_attr);
drivers/pcmcia/socket_sysfs.c +1 -1
@@ -367,7 +367,7 @@ static struct device_attribute *pccard_socket_attributes[] = { };static struct bin_attribute pccard_cis_attr = { - .attr = { .name ="cis" , .mode =S_IRUGO |S_IWUSR , .owner =THIS_MODULE }, + .attr = { .name ="cis" , .mode =S_IRUGO |S_IWUSR }, .size =0x200 , .read = pccard_show_cis, .write = pccard_store_cis,
drivers/rtc/rtc-ds1553.c +0 -1
@@ -290,7 +290,6 @@ static struct bin_attribute ds1553_nvram_attr = { .attr = { .name = "nvram", .mode = S_IRUGO | S_IWUGO, - .owner = THIS_MODULE, }, .size = RTC_OFFSET, .read = ds1553_nvram_read,
drivers/rtc/rtc-ds1742.c +0 -1
@@ -159,7 +159,6 @@ static struct bin_attribute ds1742_nvram_attr = { .attr = { .name = "nvram", .mode = S_IRUGO | S_IWUGO, - .owner = THIS_MODULE, }, .read = ds1742_nvram_read, .write = ds1742_nvram_write,
drivers/scsi/arcmsr/arcmsr_attr.c +0 -3
@@ -189,7 +189,6 @@ static struct bin_attribute arcmsr_sysfs_message_read_attr = { .attr = { .name = "mu_read", .mode = S_IRUSR , - .owner = THIS_MODULE, }, .size = 1032, .read = arcmsr_sysfs_iop_message_read,
@@ -199,7 +198,6 @@ static struct bin_attribute arcmsr_sysfs_message_write_attr = { .attr = { .name = "mu_write", .mode = S_IWUSR, - .owner = THIS_MODULE, }, .size = 1032, .write = arcmsr_sysfs_iop_message_write,
@@ -209,7 +207,6 @@ static struct bin_attribute arcmsr_sysfs_message_clear_attr = { .attr = { .name = "mu_clear", .mode = S_IWUSR, - .owner = THIS_MODULE, }, .size = 1, .write = arcmsr_sysfs_iop_message_clear,
drivers/scsi/lpfc/lpfc_attr.c +0 -2
@@ -1145,7 +1145,6 @@ static struct bin_attribute sysfs_ctlreg_attr = { .attr = { .name = "ctlreg", .mode = S_IRUSR | S_IWUSR, - .owner = THIS_MODULE, }, .size = 256, .read = sysfs_ctlreg_read,
@@ -1356,7 +1355,6 @@ static struct bin_attribute sysfs_mbox_attr = { .attr = { .name = "mbox", .mode = S_IRUSR | S_IWUSR, - .owner = THIS_MODULE, }, .size = sizeof(MAILBOX_t), .read = sysfs_mbox_read,
drivers/scsi/qla2xxx/qla_attr.c +0 -6
@@ -73,7 +73,6 @@ static struct bin_attribute sysfs_fw_dump_attr = { .attr = { .name = "fw_dump", .mode = S_IRUSR | S_IWUSR, - .owner = THIS_MODULE, }, .size = 0, .read = qla2x00_sysfs_read_fw_dump,
@@ -149,7 +148,6 @@ static struct bin_attribute sysfs_nvram_attr = { .attr = { .name = "nvram", .mode = S_IRUSR | S_IWUSR, - .owner = THIS_MODULE, }, .size = 512, .read = qla2x00_sysfs_read_nvram,
@@ -198,7 +196,6 @@ static struct bin_attribute sysfs_optrom_attr = { .attr = { .name = "optrom", .mode = S_IRUSR | S_IWUSR, - .owner = THIS_MODULE, }, .size = OPTROM_SIZE_24XX, .read = qla2x00_sysfs_read_optrom,
@@ -279,7 +276,6 @@ static struct bin_attribute sysfs_optrom_ctl_attr = { .attr = { .name = "optrom_ctl", .mode = S_IWUSR, - .owner = THIS_MODULE, }, .size = 0, .write = qla2x00_sysfs_write_optrom_ctl,
@@ -327,7 +323,6 @@ static struct bin_attribute sysfs_vpd_attr = { .attr = { .name = "vpd", .mode = S_IRUSR | S_IWUSR, - .owner = THIS_MODULE, }, .size = 0, .read = qla2x00_sysfs_read_vpd,
@@ -375,7 +370,6 @@ static struct bin_attribute sysfs_sfp_attr = { .attr = { .name = "sfp", .mode = S_IRUSR | S_IWUSR, - .owner = THIS_MODULE, }, .size = SFP_DEV_SIZE * 2, .read = qla2x00_sysfs_read_sfp,
drivers/spi/at25.c +0 -1
@@ -314,7 +314,6 @@ static int at25_probe(struct spi_device *spi)*/ at25->bin.attr.name ="eeprom" ; at25->bin.attr.mode =S_IRUSR ; - at25->bin.attr.owner =THIS_MODULE ; at25->bin.read = at25_bin_read; at25->bin.size = at25->chip.byte_len;
drivers/video/aty/radeon_base.c +0 -2
@@ -2123,7 +2123,6 @@ static ssize_t radeon_show_edid2(struct kobject *kobj, char *buf, loff_t off, sistatic struct bin_attribute edid1_attr = { .attr = { .name ="edid1" , - .owner =THIS_MODULE , .mode =0444 , }, .size =EDID_LENGTH ,
@@ -2133,7 +2132,6 @@ static struct bin_attribute edid1_attr = {static struct bin_attribute edid2_attr = { .attr = { .name ="edid2" , - .owner =THIS_MODULE , .mode =0444 , }, .size =EDID_LENGTH ,
drivers/video/backlight/backlight.c +1 -1
@@ -172,7 +172,7 @@ static struct class backlight_class = {#define DECLARE_ATTR (_name,_mode,_show,_store) \ { \ - .attr = { .name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE }, \ + .attr = { .name = __stringify(_name), .mode = _mode }, \ .show = _show, \ .store = _store, \ }
drivers/video/backlight/lcd.c +1 -1
@@ -157,7 +157,7 @@ static struct class lcd_class = {#define DECLARE_ATTR (_name,_mode,_show,_store) \ { \ - .attr = { .name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE }, \ + .attr = { .name = __stringify(_name), .mode = _mode }, \ .show = _show, \ .store = _store, \ }
drivers/w1/slaves/w1_ds2433.c +0 -1
@@ -252,7 +252,6 @@ static struct bin_attribute w1_f23_bin_attr = { .attr = { .name = "eeprom", .mode = S_IRUGO | S_IWUSR, - .owner = THIS_MODULE, }, .size = W1_EEPROM_SIZE, .read = w1_f23_read_bin,
drivers/w1/slaves/w1_therm.c +0 -1
@@ -48,7 +48,6 @@ static struct bin_attribute w1_therm_bin_attr = { .attr = { .name = "w1_slave", .mode = S_IRUGO, - .owner = THIS_MODULE, }, .size = W1_SLAVE_DATA_SIZE, .read = w1_therm_read_bin,
drivers/w1/w1.c +0 -2
@@ -128,7 +128,6 @@ static struct bin_attribute w1_slave_attr_bin_id = { .attr = { .name = "id", .mode = S_IRUGO, - .owner = THIS_MODULE, }, .size = 8, .read = w1_slave_read_id,
@@ -167,7 +166,6 @@ static struct bin_attribute w1_default_attr = { .attr = { .name = "rw", .mode = S_IRUGO | S_IWUSR, - .owner = THIS_MODULE, }, .size = PAGE_SIZE, .read = w1_default_read,
fs/ecryptfs/main.c +0 -2
@@ -842,8 +842,6 @@ static int __init ecryptfs_init(void) goto out; }kset_set_kset_s (&ecryptfs_subsys, fs_subsys); - sysfs_attr_version.attr.owner =THIS_MODULE ; - sysfs_attr_version_str.attr.owner =THIS_MODULE ; rc =do_sysfs_registration ();if (rc) {printk (KERN_ERR "sysfs registration failed\n" );
fs/ocfs2/cluster/masklog.c +0 -1
@@ -74,7 +74,6 @@ struct mlog_attribute {#define define_mask (_name) { \ .attr = { \ .name = #_name, \ - .owner = THIS_MODULE, \ .mode = S_IRUGO | S_IWUSR, \ }, \ .mask = ML_##_name, \
fs/partitions/check.c +0 -1
@@ -393,7 +393,6 @@ void add_partition(struct gendisk *disk, int part, sector_t start, sector_t len,static struct attribute addpartattr = { .name ="whole_disk" , .mode =S_IRUSR |S_IRGRP |S_IROTH , - .owner =THIS_MODULE , };sysfs_create_file (&p->kobj, &addpartattr);
fs/sysfs/bin.c +5 -14
@@ -175,25 +175,20 @@ static int open(struct inode * inode, struct file * file)if (!sysfs_get_active (attr_sd))return -ENODEV ; -/* Grab the module reference for this attribute */ - error = -ENODEV ; -if (!try_module_get (attr->attr.owner)) - goto err_sput; - error = -EACCES ;if ((file->f_mode &FMODE_WRITE ) && !(attr->write || attr->mmap)) - goto err_mput; + goto err_out;if ((file->f_mode &FMODE_READ ) && !(attr->read || attr->mmap)) - goto err_mput; + goto err_out; error = -ENOMEM ; bb =kzalloc (sizeof (*bb),GFP_KERNEL );if (!bb) - goto err_mput; + goto err_out; bb->buffer =kmalloc (PAGE_SIZE ,GFP_KERNEL );if (!bb->buffer) - goto err_mput; + goto err_out;mutex_init (&bb->mutex); file->private_data = bb;
@@ -203,9 +198,7 @@ static int open(struct inode * inode, struct file * file)sysfs_get (attr_sd);return 0 ; - err_mput: -module_put (attr->attr.owner); - err_sput: + err_out:sysfs_put_active (attr_sd);kfree (bb);return error;
@@ -214,13 +207,11 @@ static int open(struct inode * inode, struct file * file)static int release (struct inode * inode,struct file * file) {struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata; -struct bin_attribute *attr = attr_sd->s_elem.bin_attr.bin_attr;struct bin_buffer *bb = file->private_data;if (bb->mmapped)sysfs_put_active_two (attr_sd);sysfs_put (attr_sd); -module_put (attr->attr.owner);kfree (bb->buffer);kfree (bb);return 0 ;
fs/sysfs/file.c +5 -16
@@ -271,7 +271,6 @@ sysfs_write_file(struct file *file, const char __user *buf, size_t count, loff_tstatic int sysfs_open_file (struct inode *inode,struct file *file) {struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata; -struct attribute *attr = attr_sd->s_elem.attr.attr;struct kobject *kobj = attr_sd->s_parent->s_elem.dir.kobj;struct sysfs_buffer * buffer;struct sysfs_ops * ops =NULL ;
@@ -281,11 +280,6 @@ static int sysfs_open_file(struct inode *inode, struct file *file)if (!sysfs_get_active_two (attr_sd))return -ENODEV ; -/* Grab the module reference for this attribute */ - error = -ENODEV ; -if (!try_module_get (attr->owner)) - goto err_sput; -/* if the kobject has no ktype, then we assume that it is a subsystem * itself, and use ops for it. */
@@ -302,7 +296,7 @@ static int sysfs_open_file(struct inode *inode, struct file *file)* or the subsystem have no operations. */ if (!ops) - goto err_mput; + goto err_out;/* File needs write support. * The inode's perms must say it's ok,
@@ -310,7 +304,7 @@ static int sysfs_open_file(struct inode *inode, struct file *file)*/ if (file->f_mode &FMODE_WRITE ) {if (!(inode->i_mode &S_IWUGO ) || !ops->store) - goto err_mput; + goto err_out; }/* File needs read support.
@@ -319,7 +313,7 @@ static int sysfs_open_file(struct inode *inode, struct file *file)*/ if (file->f_mode &FMODE_READ ) {if (!(inode->i_mode &S_IRUGO ) || !ops->show) - goto err_mput; + goto err_out; }/* No error? Great, allocate a buffer for the file, and store it
@@ -328,7 +322,7 @@ static int sysfs_open_file(struct inode *inode, struct file *file) error = -ENOMEM ; buffer =kzalloc (sizeof (struct sysfs_buffer ),GFP_KERNEL );if (!buffer) - goto err_mput; + goto err_out;init_MUTEX (&buffer->sem); buffer->needs_read_fill =1 ;
@@ -340,9 +334,7 @@ static int sysfs_open_file(struct inode *inode, struct file *file)sysfs_get (attr_sd);return 0 ; - err_mput: -module_put (attr->owner); - err_sput: + err_out:sysfs_put_active_two (attr_sd);return error; }
@@ -350,12 +342,9 @@ static int sysfs_open_file(struct inode *inode, struct file *file)static int sysfs_release (struct inode * inode,struct file * filp) {struct sysfs_dirent *attr_sd = filp->f_path.dentry->d_fsdata; -struct attribute *attr = attr_sd->s_elem.attr.attr;struct sysfs_buffer *buffer = filp->private_data;sysfs_put (attr_sd); -/* After this point, attr should not be accessed. */ -module_put (attr->owner);if (buffer) {if (buffer->page)
include/linux/sysdev.h +1 -2
@@ -100,8 +100,7 @@ struct sysdev_attribute {#define _SYSDEV_ATTR (_name,_mode,_show,_store) \ { \ - .attr = { .name = __stringify(_name), .mode = _mode, \ - .owner = THIS_MODULE }, \ + .attr = { .name = __stringify(_name), .mode = _mode }, \ .show = _show, \ .store = _store, \ }
include/linux/sysfs.h +3 -4
@@ -22,7 +22,6 @@ struct dentry;struct attribute {const char * name; -struct module * owner;mode_t mode; };
@@ -39,14 +38,14 @@ struct attribute_group {*/ #define __ATTR (_name,_mode,_show,_store) { \ - .attr = {.name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE }, \ + .attr = {.name = __stringify(_name), .mode = _mode }, \ .show = _show, \ .store = _store, \ }#define __ATTR_RO (_name) { \ - .attr = { .name = __stringify(_name), .mode = 0444, .owner = THIS_MODULE }, \ - .show = _name##_show, \ + .attr = { .name = __stringify(_name), .mode = 0444 }, \ + .show = _name##_show, \ }#define __ATTR_NULL { .attr = { .name = NULL } }
kernel/module.c +3 -6
@@ -485,8 +485,7 @@ static void free_modinfo_##field(struct module *mod) \ mod->field =NULL ; \ } \ static struct module_attribute modinfo_##field = { \ - .attr = { .name = __stringify(field), .mode = 0444, \ - .owner = THIS_MODULE }, \ + .attr = { .name = __stringify(field), .mode = 0444 }, \ .show = show_modinfo_##field, \ .setup = setup_modinfo_##field, \ .test = modinfo_##field##_exists, \
@@ -790,7 +789,7 @@ static ssize_t show_refcnt(struct module_attribute *mattr, }static struct module_attribute refcnt = { - .attr = { .name ="refcnt" , .mode =0444 , .owner =THIS_MODULE }, + .attr = { .name ="refcnt" , .mode =0444 }, .show = show_refcnt, };
@@ -848,7 +847,7 @@ static ssize_t show_initstate(struct module_attribute *mattr, }static struct module_attribute initstate = { - .attr = { .name ="initstate" , .mode =0444 , .owner =THIS_MODULE }, + .attr = { .name ="initstate" , .mode =0444 }, .show = show_initstate, };
@@ -1029,7 +1028,6 @@ static void add_sect_attrs(struct module *mod, unsigned int nsect, sattr->mattr.show = module_sect_show; sattr->mattr.store =NULL ; sattr->mattr.attr.name = sattr->name; - sattr->mattr.attr.owner = mod; sattr->mattr.attr.mode =S_IRUGO ; *(gattr++) = &(sattr++)->mattr.attr; }
@@ -1087,7 +1085,6 @@ int module_add_modinfo_attrs(struct module *mod)if (!attr->test || (attr->test && attr->test (mod))) {memcpy (temp_attr, attr,sizeof (*temp_attr)); - temp_attr->attr.owner = mod; error =sysfs_create_file (&mod->mkobj.kobj,&temp_attr->attr); ++temp_attr; }
kernel/params.c +0 -1
@@ -487,7 +487,6 @@ param_sysfs_setup(struct module_kobject *mk, pattr->mattr.show = param_attr_show; pattr->mattr.store = param_attr_store; pattr->mattr.attr.name = (char *)&kp->name[name_skip]; - pattr->mattr.attr.owner = mk->mod; pattr->mattr.attr.mode = kp->perm; *(gattr++) = &(pattr++)->mattr.attr; }
net/bridge/br_sysfs_br.c +1 -2
@@ -365,8 +365,7 @@ static ssize_t brforward_read(struct kobject *kobj, char *buf,static struct bin_attribute bridge_forward = { .attr = { .name =SYSFS_BRIDGE_FDB , - .mode =S_IRUGO , - .owner =THIS_MODULE , }, + .mode =S_IRUGO , }, .read = brforward_read, };
net/bridge/br_sysfs_if.c +1 -2
@@ -29,8 +29,7 @@ struct brport_attribute {#define BRPORT_ATTR (_name,_mode,_show,_store) \ struct brport_attribute brport_attr_##_name = { \ .attr = {.name = __stringify(_name), \ - .mode = _mode, \ - .owner = THIS_MODULE, }, \ + .mode = _mode }, \ .show = _show, \ .store = _store, \ };
Signature · 1 line
1.5.0.3
PATCH 11/14 sysfs: implement bin_buffer
In reply to Tejun HeoImplement bin_buffer which contains a mutex and pointer to PAGE_SIZE buffer to properly synchronize accesses to per-openfile buffer and prepare for immediate-kobj-disconnect.
--- fs/sysfs/bin.c | 64 ++++++++++++++++++++++++++++++++++++++++++------------- 1 files changed, 49 insertions(+), 15 deletions(-)
fs/sysfs/bin.c +49 -15
@@ -20,6 +20,11 @@#include "sysfs.h" +struct bin_buffer { +struct mutex mutex; +void *buffer; +}; +static int fill_read (struct dentry *dentry,char *buffer,loff_t off,size_t count) {
@@ -36,7 +41,7 @@ fill_read(struct dentry *dentry, char *buffer, loff_t off, size_t count)static ssize_t read (struct file *file,char __user *userbuf,size_t bytes,loff_t *off) { -char *buffer = file->private_data; +struct bin_buffer *bb = file->private_data;struct dentry *dentry = file->f_path.dentry;int size = dentry->d_inode->i_size;loff_t offs = *off;
@@ -49,17 +54,23 @@ read(struct file *file, char __user *userbuf, size_t bytes, loff_t *off) count = size - offs; } - count =fill_read (dentry, buffer, offs, count); +mutex_lock (&bb->mutex); + + count =fill_read (dentry, bb->buffer, offs, count);if (count <0 ) -return count; + goto out_unlock; -if (copy_to_user (userbuf, buffer, count)) -return -EFAULT ; +if (copy_to_user (userbuf, bb->buffer, count)) { + count = -EFAULT ; + goto out_unlock; + }pr_debug ("offs = %lld, *off = %lld, count = %zd\n" , offs, *off, count); *off = offs + count; + out_unlock: +mutex_unlock (&bb->mutex);return count; }
@@ -79,7 +90,7 @@ flush_write(struct dentry *dentry, char *buffer, loff_t offset, size_t count)static ssize_t write (struct file *file,const char __user *userbuf,size_t bytes,loff_t *off) { -char *buffer = file->private_data; +struct bin_buffer *bb = file->private_data;struct dentry *dentry = file->f_path.dentry;int size = dentry->d_inode->i_size;loff_t offs = *off;
@@ -92,25 +103,38 @@ static ssize_t write(struct file *file, const char __user *userbuf, count = size - offs; } -if (copy_from_user (buffer, userbuf, count)) -return -EFAULT ; +mutex_lock (&bb->mutex); + +if (copy_from_user (bb->buffer, userbuf, count)) { + count = -EFAULT ; + goto out_unlock; + } - count =flush_write (dentry, buffer, offs, count); + count =flush_write (dentry, bb->buffer, offs, count);if (count >0 ) *off = offs + count; + + out_unlock: +mutex_unlock (&bb->mutex);return count; }static int mmap (struct file *file,struct vm_area_struct *vma) { +struct bin_buffer *bb = file->private_data;struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;struct bin_attribute *attr = attr_sd->s_elem.bin_attr.bin_attr;struct kobject *kobj =to_kobj (file->f_path.dentry->d_parent); +int rc;if (!attr->mmap)return -EINVAL ; -return attr->mmap (kobj, attr, vma); +mutex_lock (&bb->mutex); + rc = attr->mmap (kobj, attr, vma); +mutex_unlock (&bb->mutex); + +return rc; }static int open (struct inode * inode,struct file * file)
@@ -118,6 +142,7 @@ static int open(struct inode * inode, struct file * file)struct kobject *kobj =sysfs_get_kobject (file->f_path.dentry->d_parent);struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;struct bin_attribute *attr = attr_sd->s_elem.bin_attr.bin_attr; +struct bin_buffer *bb =NULL ;int error = -EINVAL ;if (!kobj || !attr)
@@ -135,14 +160,22 @@ static int open(struct inode * inode, struct file * file) goto Error; error = -ENOMEM ; - file->private_data =kmalloc (PAGE_SIZE ,GFP_KERNEL ); -if (!file->private_data) + bb =kzalloc (sizeof (*bb),GFP_KERNEL ); +if (!bb) goto Error; + bb->buffer =kmalloc (PAGE_SIZE ,GFP_KERNEL ); +if (!bb->buffer) + goto Error; + +mutex_init (&bb->mutex); + file->private_data = bb; + error =0 ; - goto Done; + goto Done; Error: +kfree (bb);module_put (attr->attr.owner); Done:if (error)
@@ -155,11 +188,12 @@ static int release(struct inode * inode, struct file * file)struct kobject * kobj =to_kobj (file->f_path.dentry->d_parent);struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata;struct bin_attribute *attr = attr_sd->s_elem.bin_attr.bin_attr; -u8 * buffer = file->private_data; +struct bin_buffer *bb = file->private_data;kobject_put (kobj);module_put (attr->attr.owner); -kfree (buffer); +kfree (bb->buffer); +kfree (bb);return 0 ; }
Signature · 1 line
1.5.0.3
PATCH 9/14 sysfs: implement kobj_sysfs_assoc_lock
In reply to Tejun Heokobj->dentry can go away anytime unless the user controls when the associated sysfs node is deleted. This patch implements kobj_sysfs_assoc_lock which protects kobj->dentry. This will be used to maintain kobj based API when converting sysfs to use sysfs_dirent tree instead of dentry/kobject.
Note that this lock belongs to kobject/driver-model not sysfs. Once sysfs is converted to not use kobject in its interface, this can be removed from sysfs.
This is in preparation of object reference simplification.
--- fs/sysfs/dir.c | 8 +++++++- fs/sysfs/sysfs.h | 1 + 2 files changed, 8 insertions(+), 1 deletions(-)
fs/sysfs/dir.c +7 -1
@@ -13,6 +13,7 @@#include "sysfs.h" DECLARE_RWSEM (sysfs_rename_sem); +spinlock_t kobj_sysfs_assoc_lock =SPIN_LOCK_UNLOCKED ;void release_sysfs_dirent (struct sysfs_dirent * sd) {
@@ -371,8 +372,13 @@ static void __sysfs_remove_dir(struct dentry *dentry)void sysfs_remove_dir (struct kobject * kobj) { -__sysfs_remove_dir (kobj->dentry); +struct dentry *d = kobj->dentry; + +spin_lock (&kobj_sysfs_assoc_lock); kobj->dentry =NULL ; +spin_unlock (&kobj_sysfs_assoc_lock); + +__sysfs_remove_dir (d); } int sysfs_rename_dir(struct kobject * kobj, struct dentry *new_parent,
fs/sysfs/sysfs.h +1 -0
@@ -60,6 +60,7 @@ extern void sysfs_remove_subdir(struct dentry *);extern void sysfs_drop_dentry (struct sysfs_dirent *sd,struct dentry *parent);extern int sysfs_setattr (struct dentry *dentry,struct iattr *iattr); +extern spinlock_t kobj_sysfs_assoc_lock;extern struct rw_semaphore sysfs_rename_sem;extern struct super_block * sysfs_sb;extern const struct file_operations sysfs_dir_operations;
Signature · 1 line
1.5.0.3
PATCH 8/14 sysfs: make sysfs_dirent->s_element a union
In reply to Tejun HeoMake sd->s_element a union of sysfs_elem_{dir|symlink|attr|bin_attr} and rename it to s_elem. This is to achieve...
* some level of type checking : changing symlink to point to sysfs_dirent instead of kobject is much safer and less painful now.
* easier / standardized dereferencing
* allow sysfs_elem_* to contain more than one entry
Where possible, pointer is obtained by directly deferencing from sd instead of going through other entities. This reduces dependencies to dentry, inode and kobject. to_attr() and to_bin_attr() are unused now and removed.
This is in preparation of object reference simplification.
--- fs/sysfs/bin.c | 18 ++++++++++------ fs/sysfs/dir.c | 31 +++++++++++++--------------- fs/sysfs/file.c | 19 +++++++++-------- fs/sysfs/inode.c | 2 +- fs/sysfs/mount.c | 1 - fs/sysfs/symlink.c | 23 +++----------------- fs/sysfs/sysfs.h | 56 ++++++++++++++++++++++++++++----------------------- 7 files changed, 71 insertions(+), 79 deletions(-)
fs/sysfs/bin.c +11 -7
@@ -23,7 +23,8 @@static int fill_read (struct dentry *dentry,char *buffer,loff_t off,size_t count) { -struct bin_attribute * attr =to_bin_attr (dentry); +struct sysfs_dirent *attr_sd = dentry->d_fsdata; +struct bin_attribute *attr = attr_sd->s_elem.bin_attr.bin_attr;struct kobject * kobj =to_kobj (dentry->d_parent);if (!attr->read)
@@ -65,7 +66,8 @@ read(struct file *file, char __user *userbuf, size_t bytes, loff_t *off)static int flush_write (struct dentry *dentry,char *buffer,loff_t offset,size_t count) { -struct bin_attribute *attr =to_bin_attr (dentry); +struct sysfs_dirent *attr_sd = dentry->d_fsdata; +struct bin_attribute *attr = attr_sd->s_elem.bin_attr.bin_attr;struct kobject *kobj =to_kobj (dentry->d_parent);if (!attr->write)
@@ -101,9 +103,9 @@ static ssize_t write(struct file *file, const char __user *userbuf,static int mmap (struct file *file,struct vm_area_struct *vma) { -struct dentry *dentry = file->f_path.dentry; -struct bin_attribute *attr =to_bin_attr (dentry); -struct kobject *kobj =to_kobj (dentry->d_parent); +struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata; +struct bin_attribute *attr = attr_sd->s_elem.bin_attr.bin_attr; +struct kobject *kobj =to_kobj (file->f_path.dentry->d_parent);if (!attr->mmap)return -EINVAL ;
@@ -114,7 +116,8 @@ static int mmap(struct file *file, struct vm_area_struct *vma)static int open (struct inode * inode,struct file * file) {struct kobject *kobj =sysfs_get_kobject (file->f_path.dentry->d_parent); -struct bin_attribute * attr =to_bin_attr (file->f_path.dentry); +struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata; +struct bin_attribute *attr = attr_sd->s_elem.bin_attr.bin_attr;int error = -EINVAL ;if (!kobj || !attr)
@@ -150,7 +153,8 @@ static int open(struct inode * inode, struct file * file)static int release (struct inode * inode,struct file * file) {struct kobject * kobj =to_kobj (file->f_path.dentry->d_parent); -struct bin_attribute * attr =to_bin_attr (file->f_path.dentry); +struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata; +struct bin_attribute *attr = attr_sd->s_elem.bin_attr.bin_attr;u8 * buffer = file->private_data;kobject_put (kobj);
fs/sysfs/dir.c +14 -17
@@ -21,11 +21,8 @@ void release_sysfs_dirent(struct sysfs_dirent * sd) repeat: parent_sd = sd->s_parent; -if (sd->s_type &SYSFS_KOBJ_LINK ) { -struct sysfs_symlink * sl = sd->s_element; -kobject_put (sl->target_kobj); -kfree (sl); - } +if (sd->s_type &SYSFS_KOBJ_LINK ) +kobject_put (sd->s_elem.symlink.target_kobj);if (sd->s_type &SYSFS_COPY_NAME )kfree (sd->s_name);kfree (sd->s_iattr);
@@ -52,8 +49,7 @@ static struct dentry_operations sysfs_dentry_ops = { .d_iput = sysfs_d_iput, }; -struct sysfs_dirent *sysfs_new_dirent (const char *name,void *element, -umode_t mode,int type) +struct sysfs_dirent *sysfs_new_dirent (const char *name,umode_t mode,int type) {char *dup_name =NULL ;struct sysfs_dirent * sd;
@@ -76,7 +72,6 @@ struct sysfs_dirent *sysfs_new_dirent(const char *name, void *element,INIT_LIST_HEAD (&sd->s_sibling); sd->s_name = name; - sd->s_element = element; sd->s_mode = mode; sd->s_type = type;
@@ -111,7 +106,7 @@ int sysfs_dirent_exist(struct sysfs_dirent *parent_sd,struct sysfs_dirent * sd;list_for_each_entry (sd, &parent_sd->s_children, s_sibling) { -if (sd->s_element) { +if (sd->s_type) {if (strcmp (sd->s_name, new))continue ;else
@@ -166,9 +161,10 @@ static int create_dir(struct kobject *kobj, struct dentry *parent, goto out_dput; error = -ENOMEM ; - sd =sysfs_new_dirent (name, kobj, mode,SYSFS_DIR ); + sd =sysfs_new_dirent (name, mode,SYSFS_DIR );if (!sd) goto out_drop; + sd->s_elem.dir.kobj = kobj;sysfs_attach_dirent (sd, parent->d_fsdata, dentry); error =sysfs_create (dentry, mode, init_dir);
@@ -241,10 +237,10 @@ static int sysfs_attach_attr(struct sysfs_dirent * sd, struct dentry * dentry)int error =0 ;if (sd->s_type &SYSFS_KOBJ_BIN_ATTR ) { - bin_attr = sd->s_element; + bin_attr = sd->s_elem.bin_attr.bin_attr; attr = &bin_attr->attr; }else { - attr = sd->s_element; + attr = sd->s_elem.attr.attr; init = init_file; }
@@ -349,7 +345,7 @@ static void __sysfs_remove_dir(struct dentry *dentry)mutex_lock (&dentry->d_inode->i_mutex); parent_sd = dentry->d_fsdata;list_for_each_entry_safe (sd, tmp, &parent_sd->s_children, s_sibling) { -if (!sd->s_element || !(sd->s_type &SYSFS_NOT_PINNED )) +if (!sd->s_type || !(sd->s_type &SYSFS_NOT_PINNED ))continue ;list_del_init (&sd->s_sibling);sysfs_drop_dentry (sd, dentry);
@@ -481,7 +477,7 @@ static int sysfs_dir_open(struct inode *inode, struct file *file)struct sysfs_dirent * sd;mutex_lock (&dentry->d_inode->i_mutex); - sd =sysfs_new_dirent ("_DIR_" ,NULL ,0 ,0 ); + sd =sysfs_new_dirent ("_DIR_" ,0 ,0 );if (sd)sysfs_attach_dirent (sd, parent_sd,NULL );mutex_unlock (&dentry->d_inode->i_mutex);
@@ -545,7 +541,7 @@ static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir) next = list_entry(p, struct sysfs_dirent, s_sibling); -if (!next->s_element) +if (!next->s_type)continue ; name = next->s_name;
@@ -593,7 +589,7 @@ static loff_t sysfs_dir_lseek(struct file * file, loff_t offset, int origin)struct sysfs_dirent *next; next = list_entry(p, struct sysfs_dirent, s_sibling); -if (next->s_element) +if (next->s_type) n--; p = p->next; }
@@ -660,9 +656,10 @@ struct dentry *sysfs_create_shadow_dir(struct kobject *kobj)if (!shadow) goto nomem; - sd =sysfs_new_dirent ("_SHADOW_" , kobj, inode->i_mode,SYSFS_DIR ); + sd =sysfs_new_dirent ("_SHADOW_" , inode->i_mode,SYSFS_DIR );if (!sd) goto nomem; + sd->s_elem.dir.kobj = kobj;/* point to parent_sd but don't attach to it */ sd->s_parent =sysfs_get (parent_sd);sysfs_attach_dirent (sd,NULL , shadow);
fs/sysfs/file.c +10 -9
@@ -89,7 +89,6 @@ remove_from_collection(struct sysfs_buffer *buffer, struct inode *node)static int fill_read_buffer (struct dentry * dentry,struct sysfs_buffer * buffer) {struct sysfs_dirent * sd = dentry->d_fsdata; -struct attribute * attr =to_attr (dentry);struct kobject * kobj =to_kobj (dentry->d_parent);struct sysfs_ops * ops = buffer->ops;int ret =0 ;
@@ -101,7 +100,7 @@ static int fill_read_buffer(struct dentry * dentry, struct sysfs_buffer * bufferreturn -ENOMEM ; buffer->event =atomic_read (&sd->s_event); - count = ops->show (kobj,attr,buffer->page); + count = ops->show (kobj, sd->s_elem.attr.attr, buffer->page);BUG_ON (count > (ssize_t )PAGE_SIZE );if (count >=0 ) { buffer->needs_read_fill =0 ;
@@ -229,11 +228,11 @@ fill_write_buffer(struct sysfs_buffer * buffer, const char __user * buf, size_tstatic int flush_write_buffer (struct dentry * dentry,struct sysfs_buffer * buffer,size_t count) { -struct attribute * attr =to_attr (dentry); +struct sysfs_dirent *attr_sd = dentry->d_fsdata;struct kobject * kobj =to_kobj (dentry->d_parent);struct sysfs_ops * ops = buffer->ops; -return ops->store (kobj,attr,buffer->page,count); +return ops->store (kobj, attr_sd->s_elem.attr.attr, buffer->page, count); }
@@ -278,7 +277,8 @@ out:static int sysfs_open_file (struct inode *inode,struct file *file) {struct kobject *kobj =sysfs_get_kobject (file->f_path.dentry->d_parent); -struct attribute * attr =to_attr (file->f_path.dentry); +struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata; +struct attribute *attr = attr_sd->s_elem.attr.attr;struct sysfs_buffer_collection *set;struct sysfs_buffer * buffer;struct sysfs_ops * ops =NULL ;
@@ -371,15 +371,15 @@ static int sysfs_open_file(struct inode *inode, struct file *file)static int sysfs_release (struct inode * inode,struct file * filp) {struct kobject * kobj =to_kobj (filp->f_path.dentry->d_parent); -struct attribute * attr =to_attr (filp->f_path.dentry); -struct module * owner = attr->owner; +struct sysfs_dirent *attr_sd = filp->f_path.dentry->d_fsdata; +struct attribute *attr = attr_sd->s_elem.attr.attr;struct sysfs_buffer * buffer = filp->private_data;if (buffer)remove_from_collection (buffer, inode);kobject_put (kobj);/* After this point, attr should not be accessed. */ -module_put (owner); +module_put (attr->owner);if (buffer) {if (buffer->page)
@@ -484,11 +484,12 @@ int sysfs_add_file(struct dentry * dir, const struct attribute * attr, int type) goto out_unlock; } - sd =sysfs_new_dirent (attr->name, (void *)attr, mode, type); + sd =sysfs_new_dirent (attr->name, mode, type);if (!sd) { error = -ENOMEM ; goto out_unlock; } + sd->s_elem.attr.attr = (void *)attr;sysfs_attach_dirent (sd, parent_sd,NULL ); out_unlock:
fs/sysfs/inode.c +1 -1
@@ -255,7 +255,7 @@ int sysfs_hash_and_remove(struct dentry * dir, const char * name) parent_sd = dir->d_fsdata;mutex_lock_nested (&dir->d_inode->i_mutex,I_MUTEX_PARENT );list_for_each_entry (sd, &parent_sd->s_children, s_sibling) { -if (!sd->s_element) +if (!sd->s_type)continue ;if (!strcmp (sd->s_name, name)) {list_del_init (&sd->s_sibling);
fs/sysfs/mount.c +0 -1
@@ -31,7 +31,6 @@ static struct sysfs_dirent sysfs_root = { .s_count = ATOMIC_INIT(1), .s_sibling = LIST_HEAD_INIT(sysfs_root.s_sibling), .s_children = LIST_HEAD_INIT(sysfs_root.s_children), - .s_element = NULL, .s_type = SYSFS_ROOT, .s_iattr = NULL, };
fs/sysfs/symlink.c +4 -19
@@ -48,30 +48,15 @@ static void fill_object_path(struct kobject * kobj, char * buffer, int length)static int sysfs_add_link (struct dentry * parent,const char * name,struct kobject * target) {struct sysfs_dirent * parent_sd = parent->d_fsdata; -struct sysfs_symlink * sl;struct sysfs_dirent * sd; -int error; - error = -ENOMEM ; - sl =kzalloc (sizeof (*sl),GFP_KERNEL ); -if (!sl) - goto err_out; - - sl->target_kobj =kobject_get (target); - - sd =sysfs_new_dirent (name, sl,S_IFLNK |S_IRWXUGO ,SYSFS_KOBJ_LINK ); + sd =sysfs_new_dirent (name,S_IFLNK |S_IRWXUGO ,SYSFS_KOBJ_LINK );if (!sd) - goto err_out; -sysfs_attach_dirent (sd, parent_sd,NULL ); +return -ENOMEM ; + sd->s_elem.symlink.target_kobj =kobject_get (target); +sysfs_attach_dirent (sd, parent_sd,NULL );return 0 ; - - err_out: -if (sl) { -kobject_put (sl->target_kobj); -kfree (sl); - } -return error; }/**
fs/sysfs/sysfs.h +31 -25
89 lines not on this page · raw message
Signature · 1 line
1.5.0.3
PATCH 5/14 sysfs: consolidate sysfs_dirent creation functions
In reply to Tejun HeoCurrently there are four functions to create sysfs_dirent - __sysfs_new_dirent(), sysfs_new_dirent(), __sysfs_make_dirent() and sysfs_make_dirent(). Other than sysfs_make_dirent(), no function has two users if calls to implement other functions are excluded.
This patch consolidates sysfs_dirent creation functions into the following two.
* sysfs_new_dirent() : allocate and initialize
* sysfs_attach_dirent() : attach to sysfs_dirent hierarchy and/or associate with dentry
This simplifies interface and gives callers more flexibility. This is in preparation of object reference simplification.
--- fs/sysfs/dir.c | 82 ++++++++++++++++------------------------------------ fs/sysfs/file.c | 21 ++++++++++--- fs/sysfs/symlink.c | 7 ++-- fs/sysfs/sysfs.h | 7 +++- 4 files changed, 50 insertions(+), 67 deletions(-)
fs/sysfs/dir.c +25 -57
@@ -42,10 +42,7 @@ static struct dentry_operations sysfs_dentry_ops = { .d_iput = sysfs_d_iput, }; -/* -* Allocates a new sysfs_dirent and links it to the parent sysfs_dirent -*/ -static struct sysfs_dirent *__sysfs_new_dirent (void * element) +struct sysfs_dirent *sysfs_new_dirent (void *element,umode_t mode,int type) {struct sysfs_dirent * sd;
@@ -57,25 +54,25 @@ static struct sysfs_dirent * __sysfs_new_dirent(void * element)atomic_set (&sd->s_event,1 );INIT_LIST_HEAD (&sd->s_children);INIT_LIST_HEAD (&sd->s_sibling); + sd->s_element = element; + sd->s_mode = mode; + sd->s_type = type;return sd; } -static void __sysfs_list_dirent (struct sysfs_dirent *parent_sd, -struct sysfs_dirent *sd) +void sysfs_attach_dirent (struct sysfs_dirent *sd, +struct sysfs_dirent *parent_sd,struct dentry *dentry) { -if (sd) -list_add (&sd->s_sibling, &parent_sd->s_children); -} +if (dentry) { + sd->s_dentry = dentry; + dentry->d_fsdata =sysfs_get (sd); + dentry->d_op = &sysfs_dentry_ops; + } -static struct sysfs_dirent *sysfs_new_dirent (struct sysfs_dirent *parent_sd, -void * element) -{ -struct sysfs_dirent *sd; - sd =__sysfs_new_dirent (element); -__sysfs_list_dirent (parent_sd, sd); -return sd; +if (parent_sd) +list_add (&sd->s_sibling, &parent_sd->s_children); }/*
@@ -103,39 +100,6 @@ int sysfs_dirent_exist(struct sysfs_dirent *parent_sd,return 0 ; } - -static struct sysfs_dirent * -__sysfs_make_dirent (struct dentry *dentry,void *element,mode_t mode,int type) -{ -struct sysfs_dirent * sd; - - sd =__sysfs_new_dirent (element); -if (!sd) - goto out; - - sd->s_mode = mode; - sd->s_type = type; - sd->s_dentry = dentry; -if (dentry) { - dentry->d_fsdata =sysfs_get (sd); - dentry->d_op = &sysfs_dentry_ops; - } - -out: -return sd; -} - -int sysfs_make_dirent (struct sysfs_dirent * parent_sd,struct dentry * dentry, -void * element,umode_t mode,int type) -{ -struct sysfs_dirent *sd; - - sd =__sysfs_make_dirent (dentry, element, mode, type); -__sysfs_list_dirent (parent_sd, sd); - -return sd ?0 : -ENOMEM ; -} -static int init_dir (struct inode * inode) { inode->i_op = &sysfs_dir_inode_operations;
@@ -179,10 +143,11 @@ static int create_dir(struct kobject *kobj, struct dentry *parent,if (sysfs_dirent_exist (parent->d_fsdata, name)) goto out_dput; - error =sysfs_make_dirent (parent->d_fsdata, dentry, kobj, mode, -SYSFS_DIR ); -if (error) + error = -ENOMEM ; + sd =sysfs_new_dirent (kobj, mode,SYSFS_DIR ); +if (!sd) goto out_drop; +sysfs_attach_dirent (sd, parent->d_fsdata, dentry); error =sysfs_create (dentry, mode, init_dir);if (error)
@@ -197,7 +162,6 @@ static int create_dir(struct kobject *kobj, struct dentry *parent, goto out_dput; out_sput: - sd = dentry->d_fsdata;list_del_init (&sd->s_sibling);sysfs_put (sd); out_drop:
@@ -494,13 +458,16 @@ static int sysfs_dir_open(struct inode *inode, struct file *file) {struct dentry * dentry = file->f_path.dentry;struct sysfs_dirent * parent_sd = dentry->d_fsdata; +struct sysfs_dirent * sd;mutex_lock (&dentry->d_inode->i_mutex); - file->private_data =sysfs_new_dirent (parent_sd,NULL ); + sd =sysfs_new_dirent (NULL ,0 ,0 ); +if (sd) +sysfs_attach_dirent (sd, parent_sd,NULL );mutex_unlock (&dentry->d_inode->i_mutex); -return file->private_data ?0 : -ENOMEM ; - + file->private_data = sd; +return sd ?0 : -ENOMEM ; }static int sysfs_dir_close (struct inode *inode,struct file *file)
@@ -673,9 +640,10 @@ struct dentry *sysfs_create_shadow_dir(struct kobject *kobj)if (!shadow) goto nomem; - sd =__sysfs_make_dirent (shadow, kobj, inode->i_mode,SYSFS_DIR ); + sd =sysfs_new_dirent (kobj, inode->i_mode,SYSFS_DIR );if (!sd) goto nomem; +sysfs_attach_dirent (sd,NULL , shadow);d_instantiate (shadow,igrab (inode));inc_nlink (inode);
fs/sysfs/file.c +16 -5
@@ -474,14 +474,25 @@ int sysfs_add_file(struct dentry * dir, const struct attribute * attr, int type) {struct sysfs_dirent * parent_sd = dir->d_fsdata;umode_t mode = (attr->mode &S_IALLUGO ) |S_IFREG ; -int error = -EEXIST ; +struct sysfs_dirent *sd; +int error =0 ;mutex_lock (&dir->d_inode->i_mutex); -if (!sysfs_dirent_exist (parent_sd, attr->name)) - error =sysfs_make_dirent (parent_sd,NULL , (void *)attr, - mode, type); -mutex_unlock (&dir->d_inode->i_mutex); +if (sysfs_dirent_exist (parent_sd, attr->name)) { + error = -EEXIST ; + goto out_unlock; + } + + sd =sysfs_new_dirent ((void *)attr, mode, type); +if (!sd) { + error = -ENOMEM ; + goto out_unlock; + } +sysfs_attach_dirent (sd, parent_sd,NULL ); + + out_unlock: +mutex_unlock (&dir->d_inode->i_mutex);return error; }
fs/sysfs/symlink.c +4 -3
@@ -49,6 +49,7 @@ static int sysfs_add_link(struct dentry * parent, const char * name, struct kobj {struct sysfs_dirent * parent_sd = parent->d_fsdata;struct sysfs_symlink * sl; +struct sysfs_dirent * sd;int error; error = -ENOMEM ;
@@ -63,10 +64,10 @@ static int sysfs_add_link(struct dentry * parent, const char * name, struct kobjstrcpy (sl->link_name, name); sl->target_kobj =kobject_get (target); - error =sysfs_make_dirent (parent_sd,NULL , sl,S_IFLNK |S_IRWXUGO , -SYSFS_KOBJ_LINK ); -if (error) + sd =sysfs_new_dirent (sl,S_IFLNK |S_IRWXUGO ,SYSFS_KOBJ_LINK ); +if (!sd) goto err_out; +sysfs_attach_dirent (sd, parent_sd,NULL );return 0 ;
fs/sysfs/sysfs.h +5 -2
@@ -19,8 +19,11 @@ extern int sysfs_create(struct dentry *, int mode, int (*init)(struct inode *));extern void release_sysfs_dirent (struct sysfs_dirent * sd);extern int sysfs_dirent_exist (struct sysfs_dirent *,const unsigned char *); -extern int sysfs_make_dirent (struct sysfs_dirent *,struct dentry *,void *, -umode_t ,int ); +extern struct sysfs_dirent *sysfs_new_dirent (void *element,umode_t mode, +int type); +extern void sysfs_attach_dirent (struct sysfs_dirent *sd, +struct sysfs_dirent *parent_sd, +struct dentry *dentry);extern int sysfs_add_file (struct dentry *,const struct attribute *,int );extern int sysfs_hash_and_remove (struct dentry * dir,const char * name);
Signature · 1 line
1.5.0.3
PATCH 1/14 sysfs: fix i_ino handling in sysfs
In reply to Tejun HeoInode number handling was incorrect in two ways.
1. sysfs uses the inode number allocated by new_inode() and never hashes it. When reporting the inode number, it uses iunique() if inode is inaccessible. This is incorrect because iunique() assumes the inodes are hashed. This can cause duplicate inode numbers and the condition is likely to happen because new_inode() and iunique() use separate increasing static counters to scan for empty slot.
2. sysfs_dirent->s_dentry can go away anytime and can't be referenced unless the caller knows the dentry is not and not going to be deleted.
This patch makes sysfs report the pointer to sysfs_dirent as ino. ino_t is always as big as or larger than unsigned long && sysfs_dirent hierarchy is the internal representation of the sysfs tree, so it makes sense and simple to implement.
--- fs/sysfs/dir.c | 11 ++++------- fs/sysfs/inode.c | 1 + 2 files changed, 5 insertions(+), 7 deletions(-)
fs/sysfs/dir.c +4 -7
@@ -504,19 +504,19 @@ static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir)struct sysfs_dirent * parent_sd = dentry->d_fsdata;struct sysfs_dirent *cursor = filp->private_data;struct list_head *p, *q = &cursor->s_sibling; -ino_t ino; +unsigned long ino;int i = filp->f_pos;switch (i) {case 0 : - ino = dentry->d_inode->i_ino; + ino = (unsigned long )parent_sd;if (filldir (dirent,"." ,1 , i, ino,DT_DIR ) <0 )break ; filp->f_pos++; i++;/* fallthrough */ case 1 : - ino =parent_ino (dentry); + ino = (unsigned long )dentry->d_parent->d_fsdata;if (filldir (dirent,".." ,2 , i, ino,DT_DIR ) <0 )break ; filp->f_pos++;
@@ -538,10 +538,7 @@ static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir) name =sysfs_get_name (next); len =strlen (name); -if (next->s_dentry) - ino = next->s_dentry->d_inode->i_ino; -else - ino =iunique (sysfs_sb,2 ); + ino = (unsigned long )next;if (filldir (dirent, name, len, filp->f_pos, ino,dt_type (next)) <0 )
fs/sysfs/inode.c +1 -0
@@ -140,6 +140,7 @@ struct inode * sysfs_new_inode(mode_t mode, struct sysfs_dirent * sd) inode->i_mapping->a_ops = &sysfs_aops; inode->i_mapping->backing_dev_info = &sysfs_backing_dev_info; inode->i_op = &sysfs_inode_operations; + inode->i_ino = (unsigned long )sd;lockdep_set_class (&inode->i_mutex, &sysfs_inode_imutex_key);if (sd->s_iattr) {
Signature · 1 line
1.5.0.3
PATCH 2/14 sysfs: fix error handling in binattr write()
In reply to Tejun HeoError handling in fs/sysfs/bin.c:write() was wrong because size_t count is used to receive return value from flush_write() which is negative on failure.
This patch updates write() such that int variable is used instead. read() is updated the same way for consistency.
--- fs/sysfs/bin.c | 21 ++++++++------------- 1 files changed, 8 insertions(+), 13 deletions(-)
fs/sysfs/bin.c +8 -13
@@ -33,16 +33,13 @@ fill_read(struct dentry *dentry, char *buffer, loff_t off, size_t count) }static ssize_t -read (struct file * file,char __user * userbuf,size_t count,loff_t * off) +read (struct file *file,char __user *userbuf,size_t bytes,loff_t *off) {char *buffer = file->private_data;struct dentry *dentry = file->f_path.dentry;int size = dentry->d_inode->i_size;loff_t offs = *off; -int ret; - -if (count >PAGE_SIZE ) - count =PAGE_SIZE ; +int count =min_t (size_t, bytes,PAGE_SIZE );if (size) {if (offs > size)
@@ -51,10 +48,9 @@ read(struct file * file, char __user * userbuf, size_t count, loff_t * off) count = size - offs; } - ret =fill_read (dentry, buffer, offs, count); -if (ret <0 ) -return ret; - count = ret; + count =fill_read (dentry, buffer, offs, count); +if (count <0 ) +return count;if (copy_to_user (userbuf, buffer, count))return -EFAULT ;
@@ -78,16 +74,15 @@ flush_write(struct dentry *dentry, char *buffer, loff_t offset, size_t count)return attr->write (kobj, buffer, offset, count); } -static ssize_t write (struct file * file,const char __user * userbuf, -size_t count,loff_t * off) +static ssize_t write (struct file *file,const char __user *userbuf, +size_t bytes,loff_t *off) {char *buffer = file->private_data;struct dentry *dentry = file->f_path.dentry;int size = dentry->d_inode->i_size;loff_t offs = *off; +int count =min_t (size_t, bytes,PAGE_SIZE ); -if (count >PAGE_SIZE ) - count =PAGE_SIZE ;if (size) {if (offs > size)return 0 ;
Signature · 1 line
1.5.0.3
PATCH 6/14 sysfs: add sysfs_dirent->s_parent
In reply to Tejun HeoAdd sysfs_dirent->s_parent. With this patch, each sd points to and holds a reference to its parent. This allows walking sysfs tree without referencing sd->s_dentry which can go away anytime if the user doesn't control when it's deleted.
sd->s_parent is initialized and parent is referenced in sysfs_attach_dirent(). Reference to parent is released when the sd is released, so as long as reference to a sd is held, s_parent can be followed.
dentry walk in sysfs_readdir() is convereted to s_parent walk.
This will be used to reimplement symlink such that it uses only sysfs_dirent tree.
--- fs/sysfs/dir.c | 27 ++++++++++++++++++++------- fs/sysfs/mount.c | 1 + fs/sysfs/sysfs.h | 1 + 3 files changed, 22 insertions(+), 7 deletions(-)
fs/sysfs/dir.c +20 -7
@@ -16,6 +16,11 @@ DECLARE_RWSEM(sysfs_rename_sem);void release_sysfs_dirent (struct sysfs_dirent * sd) { +struct sysfs_dirent *parent_sd; + + repeat: + parent_sd = sd->s_parent; +if (sd->s_type &SYSFS_KOBJ_LINK ) {struct sysfs_symlink * sl = sd->s_element;kfree (sl->link_name);
@@ -24,6 +29,10 @@ void release_sysfs_dirent(struct sysfs_dirent * sd) }kfree (sd->s_iattr);kmem_cache_free (sysfs_dir_cachep, sd); + + sd = parent_sd; +if (sd &&atomic_dec_and_test (&sd->s_count)) + goto repeat; }static void sysfs_d_iput (struct dentry * dentry,struct inode * inode)
@@ -71,8 +80,10 @@ void sysfs_attach_dirent(struct sysfs_dirent *sd, dentry->d_op = &sysfs_dentry_ops; } -if (parent_sd) +if (parent_sd) { + sd->s_parent =sysfs_get (parent_sd);list_add (&sd->s_sibling, &parent_sd->s_children); + } }/*
@@ -508,7 +519,7 @@ static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir) i++;/* fallthrough */ case 1 : - ino = (unsigned long )dentry->d_parent->d_fsdata; + ino = (unsigned long )parent_sd->s_parent;if (filldir (dirent,".." ,2 , i, ino,DT_DIR ) <0 )break ; filp->f_pos++;
@@ -625,13 +636,13 @@ int sysfs_make_shadowed_dir(struct kobject *kobj,struct dentry *sysfs_create_shadow_dir (struct kobject *kobj) { +struct dentry *dir = kobj->dentry; +struct inode *inode = dir->d_inode; +struct dentry *parent = dir->d_parent; +struct sysfs_dirent *parent_sd = parent->d_fsdata; +struct dentry *shadow;struct sysfs_dirent *sd; -struct dentry *parent, *dir, *shadow; -struct inode *inode; - dir = kobj->dentry; - inode = dir->d_inode; - parent = dir->d_parent; shadow =ERR_PTR (-EINVAL );if (!sysfs_is_shadowed_inode (inode)) goto out;
@@ -643,6 +654,8 @@ struct dentry *sysfs_create_shadow_dir(struct kobject *kobj) sd =sysfs_new_dirent (kobj, inode->i_mode,SYSFS_DIR );if (!sd) goto nomem; +/* point to parent_sd but don't attach to it */ + sd->s_parent =sysfs_get (parent_sd);sysfs_attach_dirent (sd,NULL , shadow);d_instantiate (shadow,igrab (inode));
fs/sysfs/mount.c +1 -0
@@ -28,6 +28,7 @@ static const struct super_operations sysfs_ops = { };static struct sysfs_dirent sysfs_root = { + .s_count =ATOMIC_INIT (1 ), .s_sibling =LIST_HEAD_INIT (sysfs_root.s_sibling), .s_children =LIST_HEAD_INIT (sysfs_root.s_children), .s_element =NULL ,
fs/sysfs/sysfs.h +1 -0
@@ -1,5 +1,6 @@struct sysfs_dirent {atomic_t s_count; +struct sysfs_dirent * s_parent;struct list_head s_sibling;struct list_head s_children;void * s_element;
Signature · 1 line
1.5.0.3
Re: [PATCH 14/14] sysfs: kill unnecessary attribute->owner
In reply to Tejun HeoQuoted 10 lines · Tejun Heo
sysfs is now completely out of driver/module lifetime game. After deletion, a sysfs node doesn't access anything outside sysfs proper, so there's no reason to hold onto the attribute owners. Note that often the wrong modules were accounted for as owners leading to accessing removed modules.
This patch kills now unnecessary
attribute->owner. Note that with this change, userland holding a sysfs node does not prevent the backing module from being unloaded.
Quoted 6 lines
For more info regarding lifetime rule cleanup, please read the following message.
http://article.gmane.org/gmane.linux.kernel/510293
Signed-off-by Tejun Heo <[email protected]>
You missed some s390 attributes :)
--- arch/s390/kernel/ipl.c | 2 -- drivers/s390/cio/chp.c | 2 -- drivers/s390/net/qeth_sys.c | 2 +- 3 files changed, 1 insertion(+), 5 deletions(-)
arch/s390/kernel/ipl.c +0 -2
@@ -314,7 +314,6 @@ static struct bin_attribute ipl_paramete .attr = { .name = "binary_parameter", .mode = S_IRUGO, - .owner = THIS_MODULE, }, .size = PAGE_SIZE, .read = &ipl_parameter_read,
@@ -338,7 +337,6 @@ static struct bin_attribute ipl_scp_data .attr = { .name = "scp_data", .mode = S_IRUGO, - .owner = THIS_MODULE, }, .size = PAGE_SIZE, .read = &ipl_scp_data_read,
drivers/s390/cio/chp.c +0 -2
@@ -165,7 +165,6 @@ static struct bin_attribute chp_measurem .attr = { .name = "measurement_chars", .mode = S_IRUSR, - .owner = THIS_MODULE, }, .size = sizeof(struct cmg_chars), .read = chp_measurement_chars_read,
@@ -217,7 +216,6 @@ static struct bin_attribute chp_measurem .attr = { .name = "measurement", .mode = S_IRUSR, - .owner = THIS_MODULE, }, .size = sizeof(struct cmg_entry), .read = chp_measurement_read,
drivers/s390/net/qeth_sys.c +1 -1
@@ -993,7 +993,7 @@ static struct attribute_group qeth_osn_d#define QETH_DEVICE_ATTR (_id,_name,_mode,_show,_store) \ struct device_attribute dev_attr_##_id = { \ - .attr = {.name=__stringify(_name), .mode=_mode, .owner=THIS_MODULE },\ + .attr = {.name=__stringify(_name), .mode=_mode, },\ .show = _show, \ .store = _store, \ };
Re: [PATCH 14/14] sysfs: kill unnecessary attribute->owner
Quoted 1 line · Cornelia Huck
You missed some s390 attributes :)
Oops, wrong tree, sorry.
--- arch/s390/kernel/ipl.c | 2 -- drivers/s390/cio/chsc.c | 2 -- drivers/s390/net/qeth_sys.c | 2 +- 3 files changed, 1 insertion(+), 5 deletions(-)
arch/s390/kernel/ipl.c +0 -2
@@ -255,7 +255,6 @@ static struct bin_attribute ipl_paramete .attr = { .name = "binary_parameter", .mode = S_IRUGO, - .owner = THIS_MODULE, }, .size = PAGE_SIZE, .read = &ipl_parameter_read,
@@ -279,7 +278,6 @@ static struct bin_attribute ipl_scp_data .attr = { .name = "scp_data", .mode = S_IRUGO, - .owner = THIS_MODULE, }, .size = PAGE_SIZE, .read = &ipl_scp_data_read,
drivers/s390/net/qeth_sys.c +1 -1
@@ -993,7 +993,7 @@ static struct attribute_group qeth_osn_d#define QETH_DEVICE_ATTR (_id,_name,_mode,_show,_store) \ struct device_attribute dev_attr_##_id = { \ - .attr = {.name=__stringify(_name), .mode=_mode, .owner=THIS_MODULE },\ + .attr = {.name=__stringify(_name), .mode=_mode, },\ .show = _show, \ .store = _store, \ };
drivers/s390/cio/chsc.c +0 -2
@@ -898,7 +898,6 @@ static struct bin_attribute chp_measurem .attr = { .name = "measurement_chars", .mode = S_IRUSR, - .owner = THIS_MODULE, }, .size = sizeof(struct cmg_chars), .read = chp_measurement_chars_read,
@@ -950,7 +949,6 @@ static struct bin_attribute chp_measurem .attr = { .name = "measurement", .mode = S_IRUSR, - .owner = THIS_MODULE, }, .size = sizeof(struct cmg_entry), .read = chp_measurement_read,
Quoted 2 lines · Tejun Heo
With all the patches applied, the same test used in the last take ran 9+hrs without any problem.
I get the following on startup:
=====================================
[ BUG: bad unlock balance detected! ]
-------------------------------------
start_udev/197 is trying to release lock (&sd->s_active) at:
[<0000000000209024>] release_sysfs_dirent+0x8c/0x118 but there are no more locks to release!
other info that might help us debug this:
no locks held by start_udev/197.
stack backtrace:
000000001ea4fad8 0000000000000002 0000000000000000 000000001ea4fbe8
000000001ea4fb60 00000000004bebf6 00000000004bebf6 0000000000103854
0000000000000000 0000000000000000 0000000000000000 0000000000602cb0
0000000000000000 000000001ea4fba8 000000001ea4fb48 000000000000000e
000000000042a810 000000000010389e 000000001ea4fb48 000000001ea4fb98
Call Trace:
([<00000000001037f4>] show_trace+0xf8/0xfc)
[<00000000001038ce>] show_stack+0xd6/0x100
[<0000000000103926>] dump_stack+0x2e/0x3c
[<0000000000153c98>] print_unlock_inbalance_bug+0x114/0x138
[<0000000000156dce>] lock_release+0x166/0x1bc
[<000000000014f8c2>] up_write+0x3e/0x98
[<0000000000209024>] release_sysfs_dirent+0x8c/0x118
[<000000000020912e>] sysfs_dir_close+0x7e/0x90
[<00000000001a7088>] __fput+0xe4/0x20c
[<00000000001a71fc>] fput+0x4c/0x5c
[<00000000001a3aa4>] filp_close+0x7c/0xb4
[<00000000001a3fcc>] sys_close+0xdc/0x128
[<00000000001107c8>] sysc_noemu+0x10/0x16
[<0000020000107dd0>] 0x20000107dd0PATCH 12/14 sysfs: implement sysfs_dirent active reference and immediate disconnect
In reply to Tejun Heo[PATCH] sysfs: implement sysfs_dirent active reference and immediate disconnect
Opening a sysfs node references its associated kobject, so userland can arbitrarily prolong lifetime of a kobject which complicates lifetime rules in drivers. This patch implements active reference and makes the association between kobject and sysfs immediately breakable.
Now each sysfs_dirent has two reference counts - s_count and s_active. s_count is a regular reference count which guarantees that the containing sysfs_dirent is accessible. As long as s_count reference is held, all sysfs internal fields in sysfs_dirent are accessible including s_parent and s_name.
The newly added s_active is active reference count. This is acquired by invoking sysfs_get_active() and it's the caller's responsibility to ensure sysfs_dirent itself is accessible (should be holding s_count one way or the other). Dereferencing sysfs_dirent to access objects out of sysfs proper requires active reference. This includes access to the associated kobjects, attributes and ops.
The active references can be drained and denied by calling sysfs_deactivate(). All active sysfs_dirents must be deactivated after deletion but before the default reference is dropped. This enables immediate disconnect of sysfs nodes. Once a sysfs_dirent is deleted, it won't access any entity external to sysfs proper.
Because attr/bin_attr ops access both the node itself and its parent for kobject, they need to hold active references to both. sysfs_get/put_active_two() helpers are provided to help grabbing both references. Parent's is acquired first and released last.
Unlike other operations, mmapped area lingers on after mmap() is finished and the module implement implementing it and kobj need to stay referenced till all the mapped pages are gone. This is accomplished by holding one set of active references to the bin_attr and its parent if there have been any mmap during lifetime of an openfile. The references are dropped when the openfile is released.
This change makes sysfs lifetime rules independent from both kobject's and module's. It not only fixes several race conditions caused by sysfs not holding onto the proper module when referencing kobject, but also helps fixing and simplifying lifetime management in driver model and drivers by taking sysfs out of the equation.
Please read the following message for more info.
http://article.gmane.org/gmane.linux.kernel/510293
---
Cornelia, this should fix the problem you reported. It's caused by doing up_write() on sysfs_dirent which is used for readdir cursor which is not deactivated before being released. release_sysfs_dirent() is udpated such that it does down_write_trylock() on s_active before doing up_write(). This also covers error paths where allocated sysfs_dirent is put due to errors during initialization.
Andrew, please use this patch instead of the original one.
Thanks.
fs/sysfs/bin.c | 95 +++++++++++++++++++++++++-------------- fs/sysfs/dir.c | 22 +++++++-- fs/sysfs/file.c | 132 ++++++++++++++++++++++++++++++++----------------------- fs/sysfs/inode.c | 8 ++- fs/sysfs/sysfs.h | 107 +++++++++++++++++++++++++++++++++++--------- 5 files changed, 250 insertions(+), 114 deletions(-)
fs/sysfs/dir.c +19 -3
@@ -22,6 +22,13 @@ void release_sysfs_dirent(struct sysfs_d repeat: parent_sd = sd->s_parent; +/* If @sd is being released after deletion, s_active is write +* locked. If @sd is cursor for directory walk or being +* released prematurely, s_active has no reader or writer. +*/ +down_write_trylock (&sd->s_active); +up_write (&sd->s_active); +if (sd->s_type &SYSFS_KOBJ_LINK )sysfs_put (sd->s_elem.symlink.target_sd);if (sd->s_type &SYSFS_COPY_NAME )
@@ -69,6 +76,7 @@ struct sysfs_dirent *sysfs_new_dirent(coatomic_set (&sd->s_count,1 );atomic_set (&sd->s_event,1 ); +init_rwsem (&sd->s_active);INIT_LIST_HEAD (&sd->s_children);INIT_LIST_HEAD (&sd->s_sibling);
@@ -316,7 +324,6 @@ static void remove_dir(struct dentry * dd_delete (d); sd = d->d_fsdata;list_del_init (&sd->s_sibling); -sysfs_put (sd);if (d->d_inode)simple_rmdir (parent->d_inode,d);
@@ -325,6 +332,9 @@ static void remove_dir(struct dentry * dmutex_unlock (&parent->d_inode->i_mutex);dput (parent); + +sysfs_deactivate (sd); +sysfs_put (sd); }void sysfs_remove_subdir (struct dentry * d)
@@ -335,6 +345,7 @@ void sysfs_remove_subdir(struct dentry *static void __sysfs_remove_dir (struct dentry *dentry) { +LIST_HEAD (removed);struct sysfs_dirent * parent_sd;struct sysfs_dirent * sd, * tmp;
@@ -348,12 +359,17 @@ static void __sysfs_remove_dir(struct delist_for_each_entry_safe (sd, tmp, &parent_sd->s_children, s_sibling) {if (!sd->s_type || !(sd->s_type &SYSFS_NOT_PINNED ))continue ; -list_del_init (&sd->s_sibling); +list_move (&sd->s_sibling, &removed);sysfs_drop_dentry (sd, dentry); -sysfs_put (sd); }mutex_unlock (&dentry->d_inode->i_mutex); +list_for_each_entry_safe (sd, tmp, &removed, s_sibling) { +list_del_init (&sd->s_sibling); +sysfs_deactivate (sd); +sysfs_put (sd); + } +remove_dir (dentry);/** * Drop reference from dget() on entrance.
fs/sysfs/sysfs.h +86 -21
137 lines not on this page · raw message
fs/sysfs/bin.c +62 -33
164 lines not on this page · raw message
fs/sysfs/file.c +77 -55
223 lines not on this page · raw message
fs/sysfs/inode.c +6 -2
@@ -260,12 +260,16 @@ int sysfs_hash_and_remove(struct dentryif (!strcmp (sd->s_name, name)) {list_del_init (&sd->s_sibling);sysfs_drop_dentry (sd, dir); -sysfs_put (sd); found =1 ;break ; } }mutex_unlock (&dir->d_inode->i_mutex); -return found ?0 : -ENOENT ; +if (!found) +return -ENOENT ; + +sysfs_deactivate (sd); +sysfs_put (sd); +return 0 ; }
Quoted 6 lines · Cornelia Huck
Quoted 2 lines · Tejun Heo
With all the patches applied, the same test used in the last take ran 9+hrs without any problem.
Quoted 8 lines
I get the following on startup:
=====================================
[ BUG: bad unlock balance detected! ]
-------------------------------------
start_udev/197 is trying to release lock (&sd->s_active) at:
[<0000000000209024>]release_sysfs_dirent+0x8c/0x118but there are no more locks to release!
Thanks a lot for spotting this. I thought I enabled lock debugging but apparently, didn't. Anyways, the bug doesn't affect actual behavior. It's just doing up_write() on an unlocked rwsem which is about to be freed. I've just posted the updated version of patch 12 to fix this.
Signature · 1 line
tejun
Re: [PATCH 14/14] sysfs: kill unnecessary attribute->owner
In reply to Cornelia HuckQuoted 4 lines · Cornelia Huck
Quoted 1 line · Cornelia Huck
You missed some s390 attributes :)
Yeap, I used allyesconfig on x86 to do the patch, so attrs on other archs are not there. I'll merge this patch and post the updated version.
Thanks.
Signature · 1 line
tejun
PATCH 14/14 sysfs: kill unnecessary attribute->owner
In reply to Tejun Heosysfs is now completely out of driver/module lifetime game. After deletion, a sysfs node doesn't access anything outside sysfs proper, so there's no reason to hold onto the attribute owners. Note that often the wrong modules were accounted for as owners leading to accessing removed modules.
This patch kills now unnecessary attribute->owner. Note that with this change, userland holding a sysfs node does not prevent the backing module from being unloaded.
For more info regarding lifetime rule cleanup, please read the following message.
http://article.gmane.org/gmane.linux.kernel/510293
s390 attributes changes are from Cornelia Huck.
---
Andrew, please use this patch instead of the original one.
Thanks.
arch/s390/kernel/ipl.c | 2 -- drivers/base/class.c | 2 -- drivers/base/core.c | 4 ---- drivers/base/firmware_class.c | 2 +- drivers/block/pktcdvd.c | 3 +-- drivers/char/ipmi/ipmi_msghandler.c | 10 ---------- drivers/cpufreq/cpufreq_stats.c | 3 +-- drivers/cpufreq/cpufreq_userspace.c | 2 +- drivers/cpufreq/freq_table.c | 1 - drivers/firmware/dcdbas.h | 3 +-- drivers/firmware/dell_rbu.c | 6 +++--- drivers/firmware/edd.c | 2 +- drivers/firmware/efivars.c | 6 +++--- drivers/i2c/chips/eeprom.c | 1 - drivers/i2c/chips/max6875.c | 1 - drivers/infiniband/core/sysfs.c | 1 - drivers/input/mouse/psmouse.h | 1 - drivers/media/video/pvrusb2/pvrusb2-sysfs.c | 13 ------------- drivers/misc/asus-laptop.c | 3 +-- drivers/pci/hotplug/acpiphp_ibm.c | 1 - drivers/pci/pci-sysfs.c | 4 ---- drivers/pcmcia/socket_sysfs.c | 2 +- drivers/rtc/rtc-ds1553.c | 1 - drivers/rtc/rtc-ds1742.c | 1 - drivers/s390/cio/chsc.c | 2 -- drivers/s390/net/qeth_sys.c | 2 +- drivers/scsi/arcmsr/arcmsr_attr.c | 3 --- drivers/scsi/lpfc/lpfc_attr.c | 2 -- drivers/scsi/qla2xxx/qla_attr.c | 6 ------ drivers/spi/at25.c | 1 - drivers/video/aty/radeon_base.c | 2 -- drivers/video/backlight/backlight.c | 2 +- drivers/video/backlight/lcd.c | 2 +- drivers/w1/slaves/w1_ds2433.c | 1 - drivers/w1/slaves/w1_therm.c | 1 - drivers/w1/w1.c | 2 -- fs/ecryptfs/main.c | 2 -- fs/ocfs2/cluster/masklog.c | 1 - fs/partitions/check.c | 1 - fs/sysfs/bin.c | 19 +++++-------------- fs/sysfs/file.c | 21 +++++---------------- include/linux/sysdev.h | 3 +-- include/linux/sysfs.h | 7 +++---- kernel/module.c | 9 +++------ kernel/params.c | 1 - net/bridge/br_sysfs_br.c | 3 +-- net/bridge/br_sysfs_if.c | 3 +-- 47 files changed, 36 insertions(+), 135 deletions(-)
fs/sysfs/bin.c +5 -14
@@ -175,25 +175,20 @@ static int open(struct inode * inode, stif (!sysfs_get_active (attr_sd))return -ENODEV ; -/* Grab the module reference for this attribute */ - error = -ENODEV ; -if (!try_module_get (attr->attr.owner)) - goto err_sput; - error = -EACCES ;if ((file->f_mode &FMODE_WRITE ) && !(attr->write || attr->mmap)) - goto err_mput; + goto err_out;if ((file->f_mode &FMODE_READ ) && !(attr->read || attr->mmap)) - goto err_mput; + goto err_out; error = -ENOMEM ; bb =kzalloc (sizeof (*bb),GFP_KERNEL );if (!bb) - goto err_mput; + goto err_out; bb->buffer =kmalloc (PAGE_SIZE ,GFP_KERNEL );if (!bb->buffer) - goto err_mput; + goto err_out;mutex_init (&bb->mutex); file->private_data = bb;
@@ -203,9 +198,7 @@ static int open(struct inode * inode, stsysfs_get (attr_sd);return 0 ; - err_mput: -module_put (attr->attr.owner); - err_sput: + err_out:sysfs_put_active (attr_sd);kfree (bb);return error;
@@ -214,13 +207,11 @@ static int open(struct inode * inode, ststatic int release (struct inode * inode,struct file * file) {struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata; -struct bin_attribute *attr = attr_sd->s_elem.bin_attr.bin_attr;struct bin_buffer *bb = file->private_data;if (bb->mmapped)sysfs_put_active_two (attr_sd);sysfs_put (attr_sd); -module_put (attr->attr.owner);kfree (bb->buffer);kfree (bb);return 0 ;
fs/sysfs/file.c +5 -16
@@ -271,7 +271,6 @@ sysfs_write_file(struct file *file, consstatic int sysfs_open_file (struct inode *inode,struct file *file) {struct sysfs_dirent *attr_sd = file->f_path.dentry->d_fsdata; -struct attribute *attr = attr_sd->s_elem.attr.attr;struct kobject *kobj = attr_sd->s_parent->s_elem.dir.kobj;struct sysfs_buffer * buffer;struct sysfs_ops * ops =NULL ;
@@ -281,11 +280,6 @@ static int sysfs_open_file(struct inodeif (!sysfs_get_active_two (attr_sd))return -ENODEV ; -/* Grab the module reference for this attribute */ - error = -ENODEV ; -if (!try_module_get (attr->owner)) - goto err_sput; -/* if the kobject has no ktype, then we assume that it is a subsystem * itself, and use ops for it. */
@@ -302,7 +296,7 @@ static int sysfs_open_file(struct inode* or the subsystem have no operations. */ if (!ops) - goto err_mput; + goto err_out;/* File needs write support. * The inode's perms must say it's ok,
@@ -310,7 +304,7 @@ static int sysfs_open_file(struct inode*/ if (file->f_mode &FMODE_WRITE ) {if (!(inode->i_mode &S_IWUGO ) || !ops->store) - goto err_mput; + goto err_out; }/* File needs read support.
@@ -319,7 +313,7 @@ static int sysfs_open_file(struct inode*/ if (file->f_mode &FMODE_READ ) {if (!(inode->i_mode &S_IRUGO ) || !ops->show) - goto err_mput; + goto err_out; }/* No error? Great, allocate a buffer for the file, and store it
@@ -328,7 +322,7 @@ static int sysfs_open_file(struct inode error = -ENOMEM ; buffer =kzalloc (sizeof (struct sysfs_buffer ),GFP_KERNEL );if (!buffer) - goto err_mput; + goto err_out;init_MUTEX (&buffer->sem); buffer->needs_read_fill =1 ;
@@ -340,9 +334,7 @@ static int sysfs_open_file(struct inodesysfs_get (attr_sd);return 0 ; - err_mput: -module_put (attr->owner); - err_sput: + err_out:sysfs_put_active_two (attr_sd);return error; }
@@ -350,12 +342,9 @@ static int sysfs_open_file(struct inodestatic int sysfs_release (struct inode * inode,struct file * filp) {struct sysfs_dirent *attr_sd = filp->f_path.dentry->d_fsdata; -struct attribute *attr = attr_sd->s_elem.attr.attr;struct sysfs_buffer *buffer = filp->private_data;sysfs_put (attr_sd); -/* After this point, attr should not be accessed. */ -module_put (attr->owner);if (buffer) {if (buffer->page)
include/linux/sysfs.h +3 -4
@@ -22,7 +22,6 @@ struct dentry;struct attribute {const char * name; -struct module * owner;mode_t mode; };
@@ -39,14 +38,14 @@ struct attribute_group {*/ #define __ATTR (_name,_mode,_show,_store) { \ - .attr = {.name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE }, \ + .attr = {.name = __stringify(_name), .mode = _mode }, \ .show = _show, \ .store = _store, \ }#define __ATTR_RO (_name) { \ - .attr = { .name = __stringify(_name), .mode = 0444, .owner = THIS_MODULE }, \ - .show = _name##_show, \ + .attr = { .name = __stringify(_name), .mode = 0444 }, \ + .show = _name##_show, \ }#define __ATTR_NULL { .attr = { .name = NULL } }
include/linux/sysdev.h +1 -2
@@ -100,8 +100,7 @@ struct sysdev_attribute {#define _SYSDEV_ATTR (_name,_mode,_show,_store) \ { \ - .attr = { .name = __stringify(_name), .mode = _mode, \ - .owner = THIS_MODULE }, \ + .attr = { .name = __stringify(_name), .mode = _mode }, \ .show = _show, \ .store = _store, \ }
drivers/base/class.c +0 -2
@@ -624,7 +624,6 @@ int class_device_add(struct class_device goto out3; class_dev->uevent_attr.attr.name ="uevent" ; class_dev->uevent_attr.attr.mode =S_IWUSR ; - class_dev->uevent_attr.attr.owner = parent_class->owner; class_dev->uevent_attr.store = store_uevent; error =class_device_create_file (class_dev, &class_dev->uevent_attr);if (error)
@@ -639,7 +638,6 @@ int class_device_add(struct class_device } attr->attr.name ="dev" ; attr->attr.mode =S_IRUGO ; - attr->attr.owner = parent_class->owner; attr->show = show_dev; error =class_device_create_file (class_dev, attr);if (error) {
drivers/base/core.c +0 -4
@@ -563,8 +563,6 @@ int device_add(struct device *dev) dev->uevent_attr.attr.name ="uevent" ; dev->uevent_attr.attr.mode =S_IWUSR ; -if (dev->driver) - dev->uevent_attr.attr.owner = dev->driver->owner; dev->uevent_attr.store = store_uevent; error =device_create_file (dev, &dev->uevent_attr);if (error)
@@ -579,8 +577,6 @@ int device_add(struct device *dev) } attr->attr.name ="dev" ; attr->attr.mode =S_IRUGO ; -if (dev->driver) - attr->attr.owner = dev->driver->owner; attr->show = show_dev; error =device_create_file (dev, attr);if (error) {
drivers/base/firmware_class.c +1 -1
@@ -276,7 +276,7 @@ out: }static struct bin_attribute firmware_attr_data_tmpl = { - .attr = {.name ="data" , .mode =0644 , .owner =THIS_MODULE }, + .attr = {.name ="data" , .mode =0644 }, .size =0 , .read = firmware_data_read, .write = firmware_data_write,
drivers/block/pktcdvd.c +1 -2
@@ -146,8 +146,7 @@ static void pkt_kobj_release(struct kobj**********************************************************/ #define DEF_ATTR (_obj,_name,_mode) \ - static struct attribute _obj = { \ - .name = _name, .owner = THIS_MODULE, .mode = _mode } + static struct attribute _obj = { .name = _name, .mode = _mode }/********************************************************** /sys/class/pktcdvd/pktcdvd[0-7]/
drivers/char/ipmi/ipmi_msghandler.c +0 -10
@@ -2171,52 +2171,42 @@ static int create_files(struct bmc_devicint err; bmc->device_id_attr.attr.name ="device_id" ; - bmc->device_id_attr.attr.owner =THIS_MODULE ; bmc->device_id_attr.attr.mode =S_IRUGO ; bmc->device_id_attr.show = device_id_show; bmc->provides_dev_sdrs_attr.attr.name ="provides_device_sdrs" ; - bmc->provides_dev_sdrs_attr.attr.owner =THIS_MODULE ; bmc->provides_dev_sdrs_attr.attr.mode =S_IRUGO ; bmc->provides_dev_sdrs_attr.show = provides_dev_sdrs_show; bmc->revision_attr.attr.name ="revision" ; - bmc->revision_attr.attr.owner =THIS_MODULE ; bmc->revision_attr.attr.mode =S_IRUGO ; bmc->revision_attr.show = revision_show; bmc->firmware_rev_attr.attr.name ="firmware_revision" ; - bmc->firmware_rev_attr.attr.owner =THIS_MODULE ; bmc->firmware_rev_attr.attr.mode =S_IRUGO ; bmc->firmware_rev_attr.show = firmware_rev_show; bmc->version_attr.attr.name ="ipmi_version" ; - bmc->version_attr.attr.owner =THIS_MODULE ; bmc->version_attr.attr.mode =S_IRUGO ; bmc->version_attr.show = ipmi_version_show; bmc->add_dev_support_attr.attr.name ="additional_device_support" ; - bmc->add_dev_support_attr.attr.owner =THIS_MODULE ; bmc->add_dev_support_attr.attr.mode =S_IRUGO ; bmc->add_dev_support_attr.show = add_dev_support_show; bmc->manufacturer_id_attr.attr.name ="manufacturer_id" ; - bmc->manufacturer_id_attr.attr.owner =THIS_MODULE ; bmc->manufacturer_id_attr.attr.mode =S_IRUGO ; bmc->manufacturer_id_attr.show = manufacturer_id_show; bmc->product_id_attr.attr.name ="product_id" ; - bmc->product_id_attr.attr.owner =THIS_MODULE ; bmc->product_id_attr.attr.mode =S_IRUGO ; bmc->product_id_attr.show = product_id_show; bmc->guid_attr.attr.name ="guid" ; - bmc->guid_attr.attr.owner =THIS_MODULE ; bmc->guid_attr.attr.mode =S_IRUGO ; bmc->guid_attr.show = guid_show; bmc->aux_firmware_rev_attr.attr.name ="aux_firmware_revision" ; - bmc->aux_firmware_rev_attr.attr.owner =THIS_MODULE ; bmc->aux_firmware_rev_attr.attr.mode =S_IRUGO ; bmc->aux_firmware_rev_attr.show = aux_firmware_rev_show;
drivers/cpufreq/cpufreq_stats.c +1 -2
@@ -25,8 +25,7 @@ static spinlock_t cpufreq_stats_lock;#define CPUFREQ_STATDEVICE_ATTR (_name,_mode,_show) \ static struct freq_attr _attr_##_name = {\ - .attr = {.name = __stringify(_name), .owner = THIS_MODULE, \ - .mode = _mode, }, \ + .attr = {.name = __stringify(_name), .mode = _mode, }, \ .show = _show,\ };
drivers/cpufreq/cpufreq_userspace.c +1 -1
@@ -120,7 +120,7 @@ store_speed (struct cpufreq_policy *polistatic struct freq_attr freq_attr_scaling_setspeed = { - .attr = { .name ="scaling_setspeed" , .mode =0644 , .owner =THIS_MODULE }, + .attr = { .name ="scaling_setspeed" , .mode =0644 }, .show = show_speed, .store = store_speed, };
drivers/cpufreq/freq_table.c +0 -1
@@ -199,7 +199,6 @@ static ssize_t show_available_freqs (strstruct freq_attr cpufreq_freq_attr_scaling_available_freqs = { .attr = { .name ="scaling_available_frequencies" , .mode =0444 , - .owner=THIS_MODULE }, .show = show_available_freqs, };
drivers/firmware/edd.c +1 -1
@@ -74,7 +74,7 @@ static struct edd_device *edd_devices[ED#define EDD_DEVICE_ATTR (_name,_mode,_show,_test) \ struct edd_attribute edd_attr_##_name = { \ - .attr = {.name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE }, \ + .attr = {.name = __stringify(_name), .mode = _mode }, \ .show = _show, \ .test = _test, \ };
fs/ecryptfs/main.c +0 -2
@@ -842,8 +842,6 @@ static int __init ecryptfs_init(void) goto out; }kset_set_kset_s (&ecryptfs_subsys, fs_subsys); - sysfs_attr_version.attr.owner =THIS_MODULE ; - sysfs_attr_version_str.attr.owner =THIS_MODULE ; rc =do_sysfs_registration ();if (rc) {printk (KERN_ERR "sysfs registration failed\n" );
fs/ocfs2/cluster/masklog.c +0 -1
@@ -74,7 +74,6 @@ struct mlog_attribute {#define define_mask (_name) { \ .attr = { \ .name = #_name, \ - .owner = THIS_MODULE, \ .mode = S_IRUGO | S_IWUSR, \ }, \ .mask = ML_##_name, \
fs/partitions/check.c +0 -1
@@ -393,7 +393,6 @@ void add_partition(struct gendisk *disk,static struct attribute addpartattr = { .name ="whole_disk" , .mode =S_IRUSR |S_IRGRP |S_IROTH , - .owner =THIS_MODULE , };sysfs_create_file (&p->kobj, &addpartattr);
kernel/module.c +3 -6
@@ -485,8 +485,7 @@ static void free_modinfo_##field(struct mod->field =NULL ; \ } \ static struct module_attribute modinfo_##field = { \ - .attr = { .name = __stringify(field), .mode = 0444, \ - .owner = THIS_MODULE }, \ + .attr = { .name = __stringify(field), .mode = 0444 }, \ .show = show_modinfo_##field, \ .setup = setup_modinfo_##field, \ .test = modinfo_##field##_exists, \
@@ -790,7 +789,7 @@ static ssize_t show_refcnt(struct module }static struct module_attribute refcnt = { - .attr = { .name ="refcnt" , .mode =0444 , .owner =THIS_MODULE }, + .attr = { .name ="refcnt" , .mode =0444 }, .show = show_refcnt, };
@@ -848,7 +847,7 @@ static ssize_t show_initstate(struct mod }static struct module_attribute initstate = { - .attr = { .name ="initstate" , .mode =0444 , .owner =THIS_MODULE }, + .attr = { .name ="initstate" , .mode =0444 }, .show = show_initstate, };
@@ -1029,7 +1028,6 @@ static void add_sect_attrs(struct module sattr->mattr.show = module_sect_show; sattr->mattr.store =NULL ; sattr->mattr.attr.name = sattr->name; - sattr->mattr.attr.owner = mod; sattr->mattr.attr.mode =S_IRUGO ; *(gattr++) = &(sattr++)->mattr.attr; }
@@ -1087,7 +1085,6 @@ int module_add_modinfo_attrs(struct moduif (!attr->test || (attr->test && attr->test (mod))) {memcpy (temp_attr, attr,sizeof (*temp_attr)); - temp_attr->attr.owner = mod; error =sysfs_create_file (&mod->mkobj.kobj,&temp_attr->attr); ++temp_attr; }
kernel/params.c +0 -1
@@ -487,7 +487,6 @@ param_sysfs_setup(struct module_kobject pattr->mattr.show = param_attr_show; pattr->mattr.store = param_attr_store; pattr->mattr.attr.name = (char *)&kp->name[name_skip]; - pattr->mattr.attr.owner = mk->mod; pattr->mattr.attr.mode = kp->perm; *(gattr++) = &(pattr++)->mattr.attr; }
net/bridge/br_sysfs_br.c +1 -2
@@ -365,8 +365,7 @@ static ssize_t brforward_read(struct kobstatic struct bin_attribute bridge_forward = { .attr = { .name =SYSFS_BRIDGE_FDB , - .mode =S_IRUGO , - .owner =THIS_MODULE , }, + .mode =S_IRUGO , }, .read = brforward_read, };
net/bridge/br_sysfs_if.c +1 -2
@@ -29,8 +29,7 @@ struct brport_attribute {#define BRPORT_ATTR (_name,_mode,_show,_store) \ struct brport_attribute brport_attr_##_name = { \ .attr = {.name = __stringify(_name), \ - .mode = _mode, \ - .owner = THIS_MODULE, }, \ + .mode = _mode }, \ .show = _show, \ .store = _store, \ };
drivers/firmware/dcdbas.h +1 -2
@@ -67,8 +67,7 @@#define DCDBAS_BIN_ATTR_RW (_name) \ struct bin_attribute bin_attr_##_name = { \ .attr = { .name = __stringify(_name), \ - .mode = 0600, \ - .owner = THIS_MODULE }, \ + .mode = 0600 }, \ .read = _name##_read, \ .write = _name##_write, \ }
drivers/firmware/dell_rbu.c +3 -3
@@ -687,18 +687,18 @@ static ssize_t write_rbu_packet_size(str }static struct bin_attribute rbu_data_attr = { - .attr = {.name ="data" ,.owner =THIS_MODULE ,.mode =0444 }, + .attr = {.name ="data" , .mode =0444 }, .read = read_rbu_data, };static struct bin_attribute rbu_image_type_attr = { - .attr = {.name ="image_type" ,.owner =THIS_MODULE ,.mode =0644 }, + .attr = {.name ="image_type" , .mode =0644 }, .read = read_rbu_image_type, .write = write_rbu_image_type, };static struct bin_attribute rbu_packet_size_attr = { - .attr = {.name ="packet_size" ,.owner =THIS_MODULE ,.mode =0644 }, + .attr = {.name ="packet_size" , .mode =0644 }, .read = read_rbu_packet_size, .write = write_rbu_packet_size, };
drivers/firmware/efivars.c +3 -3
@@ -131,21 +131,21 @@ struct efivar_attribute {#define EFI_ATTR (_name, _mode, _show, _store) \ struct subsys_attribute efi_attr_##_name = { \ - .attr = {.name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE}, \ + .attr = {.name = __stringify(_name), .mode = _mode}, \ .show = _show, \ .store = _store, \ };#define EFIVAR_ATTR (_name, _mode, _show, _store) \ struct efivar_attribute efivar_attr_##_name = { \ - .attr = {.name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE}, \ + .attr = {.name = __stringify(_name), .mode = _mode}, \ .show = _show, \ .store = _store, \ };#define VAR_SUBSYS_ATTR (_name, _mode, _show, _store) \ struct subsys_attribute var_subsys_attr_##_name = { \ - .attr = {.name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE}, \ + .attr = {.name = __stringify(_name), .mode = _mode}, \ .show = _show, \ .store = _store, \ };
drivers/i2c/chips/eeprom.c +0 -1
@@ -143,7 +143,6 @@ static struct bin_attribute eeprom_attr .attr = { .name = "eeprom", .mode = S_IRUGO, - .owner = THIS_MODULE, }, .size = EEPROM_SIZE, .read = eeprom_read,
drivers/i2c/chips/max6875.c +0 -1
@@ -152,7 +152,6 @@ static struct bin_attribute user_eeprom_ .attr = { .name = "eeprom", .mode = S_IRUGO, - .owner = THIS_MODULE, }, .size = USER_EEPROM_SIZE, .read = max6875_read,
drivers/infiniband/core/sysfs.c +0 -1
@@ -479,7 +479,6 @@ alloc_group_attrs(ssize_t (*show)(struct element->attr.attr.name = element->name; element->attr.attr.mode =S_IRUGO ; - element->attr.attr.owner =THIS_MODULE ; element->attr.show = show; element->index = i;
drivers/input/mouse/psmouse.h +0 -1
@@ -117,7 +117,6 @@ static struct psmouse_attribute psmouse_ .attr = { \ .name = __stringify(_name), \ .mode = _mode, \ - .owner = THIS_MODULE, \ }, \ .show = psmouse_attr_show_helper, \ .store = psmouse_attr_set_helper, \
drivers/media/video/pvrusb2/pvrusb2-sysfs.c +0 -13
@@ -516,40 +516,32 @@ static void pvr2_sysfs_add_control(struc } sfp->item_last = cip; - cip->attr_name.attr.owner =THIS_MODULE ; cip->attr_name.attr.name ="name" ; cip->attr_name.attr.mode =S_IRUGO ; cip->attr_name.show = fp->show_name; - cip->attr_type.attr.owner =THIS_MODULE ; cip->attr_type.attr.name ="type" ; cip->attr_type.attr.mode =S_IRUGO ; cip->attr_type.show = fp->show_type; - cip->attr_min.attr.owner =THIS_MODULE ; cip->attr_min.attr.name ="min_val" ; cip->attr_min.attr.mode =S_IRUGO ; cip->attr_min.show = fp->show_min; - cip->attr_max.attr.owner =THIS_MODULE ; cip->attr_max.attr.name ="max_val" ; cip->attr_max.attr.mode =S_IRUGO ; cip->attr_max.show = fp->show_max; - cip->attr_val.attr.owner =THIS_MODULE ; cip->attr_val.attr.name ="cur_val" ; cip->attr_val.attr.mode =S_IRUGO ; - cip->attr_custom.attr.owner =THIS_MODULE ; cip->attr_custom.attr.name ="custom_val" ; cip->attr_custom.attr.mode =S_IRUGO ; - cip->attr_enum.attr.owner =THIS_MODULE ; cip->attr_enum.attr.name ="enum_val" ; cip->attr_enum.attr.mode =S_IRUGO ; cip->attr_enum.show = fp->show_enum; - cip->attr_bits.attr.owner =THIS_MODULE ; cip->attr_bits.attr.name ="bit_val" ; cip->attr_bits.attr.mode =S_IRUGO ; cip->attr_bits.show = fp->show_bits;
@@ -614,12 +606,10 @@ static void pvr2_sysfs_add_debugifc(stru dip =kzalloc (sizeof (*dip),GFP_KERNEL );if (!dip)return ; - dip->attr_debugcmd.attr.owner =THIS_MODULE ; dip->attr_debugcmd.attr.name ="debugcmd" ; dip->attr_debugcmd.attr.mode =S_IRUGO |S_IWUSR |S_IWGRP ; dip->attr_debugcmd.show = debugcmd_show; dip->attr_debugcmd.store = debugcmd_store; - dip->attr_debuginfo.attr.owner =THIS_MODULE ; dip->attr_debuginfo.attr.name ="debuginfo" ; dip->attr_debuginfo.attr.mode =S_IRUGO ; dip->attr_debuginfo.show = debuginfo_show;
@@ -795,7 +785,6 @@ static void class_dev_create(struct pvr2return ; } - sfp->attr_v4l_minor_number.attr.owner =THIS_MODULE ; sfp->attr_v4l_minor_number.attr.name ="v4l_minor_number" ; sfp->attr_v4l_minor_number.attr.mode =S_IRUGO ; sfp->attr_v4l_minor_number.show = v4l_minor_number_show;
@@ -809,7 +798,6 @@ static void class_dev_create(struct pvr2 sfp->v4l_minor_number_created_ok = !0 ; } - sfp->attr_v4l_radio_minor_number.attr.owner =THIS_MODULE ; sfp->attr_v4l_radio_minor_number.attr.name ="v4l_radio_minor_number" ; sfp->attr_v4l_radio_minor_number.attr.mode =S_IRUGO ; sfp->attr_v4l_radio_minor_number.show = v4l_radio_minor_number_show;
@@ -823,7 +811,6 @@ static void class_dev_create(struct pvr2 sfp->v4l_radio_minor_number_created_ok = !0 ; } - sfp->attr_unit_number.attr.owner =THIS_MODULE ; sfp->attr_unit_number.attr.name ="unit_number" ; sfp->attr_unit_number.attr.mode =S_IRUGO ; sfp->attr_unit_number.show = unit_number_show;
drivers/misc/asus-laptop.c +1 -2
@@ -673,8 +673,7 @@ static void asus_hotk_notify(acpi_handle struct device_attribute dev_attr_##_name = { \ .attr = { \ .name =__stringify (_name), \ - .mode =0 , \ - .owner =THIS_MODULE }, \ + .mode =0 }, \ .show =NULL , \ .store =NULL , \ }
drivers/pci/hotplug/acpiphp_ibm.c +0 -1
@@ -117,7 +117,6 @@ static struct notification ibm_note;static struct bin_attribute ibm_apci_table_attr = { .attr = { .name ="apci_table" , - .owner =THIS_MODULE , .mode =S_IRUGO , }, .read = ibm_read_apci_table,
drivers/pci/pci-sysfs.c +0 -4
@@ -499,7 +499,6 @@ static int pci_create_resource_files(strsprintf (res_attr_name,"resource%d" , i); res_attr->attr.name = res_attr_name; res_attr->attr.mode =S_IRUSR |S_IWUSR ; - res_attr->attr.owner =THIS_MODULE ; res_attr->size =pci_resource_len (pdev, i); res_attr->mmap = pci_mmap_resource; res_attr->private = &pdev->resource[i];
@@ -582,7 +581,6 @@ static struct bin_attribute pci_config_a .attr = { .name = "config", .mode = S_IRUGO | S_IWUSR, - .owner = THIS_MODULE, }, .size = 256, .read = pci_read_config,
@@ -593,7 +591,6 @@ static struct bin_attribute pcie_config_ .attr = { .name = "config", .mode = S_IRUGO | S_IWUSR, - .owner = THIS_MODULE, }, .size = 4096, .read = pci_read_config,
@@ -627,7 +624,6 @@ int __must_check pci_create_sysfs_dev_fi rom_attr->size =pci_resource_len (pdev,PCI_ROM_RESOURCE ); rom_attr->attr.name ="rom" ; rom_attr->attr.mode =S_IRUSR ; - rom_attr->attr.owner =THIS_MODULE ; rom_attr->read = pci_read_rom; rom_attr->write = pci_write_rom; retval =sysfs_create_bin_file (&pdev->dev.kobj, rom_attr);
drivers/pcmcia/socket_sysfs.c +1 -1
@@ -367,7 +367,7 @@ static struct device_attribute *pccard_s };static struct bin_attribute pccard_cis_attr = { - .attr = { .name ="cis" , .mode =S_IRUGO |S_IWUSR , .owner =THIS_MODULE }, + .attr = { .name ="cis" , .mode =S_IRUGO |S_IWUSR }, .size =0x200 , .read = pccard_show_cis, .write = pccard_store_cis,
drivers/rtc/rtc-ds1553.c +0 -1
@@ -290,7 +290,6 @@ static struct bin_attribute ds1553_nvram .attr = { .name = "nvram", .mode = S_IRUGO | S_IWUGO, - .owner = THIS_MODULE, }, .size = RTC_OFFSET, .read = ds1553_nvram_read,
drivers/rtc/rtc-ds1742.c +0 -1
@@ -159,7 +159,6 @@ static struct bin_attribute ds1742_nvram .attr = { .name = "nvram", .mode = S_IRUGO | S_IWUGO, - .owner = THIS_MODULE, }, .read = ds1742_nvram_read, .write = ds1742_nvram_write,
drivers/scsi/arcmsr/arcmsr_attr.c +0 -3
@@ -189,7 +189,6 @@ static struct bin_attribute arcmsr_sysfs .attr = { .name = "mu_read", .mode = S_IRUSR , - .owner = THIS_MODULE, }, .size = 1032, .read = arcmsr_sysfs_iop_message_read,
@@ -199,7 +198,6 @@ static struct bin_attribute arcmsr_sysfs .attr = { .name = "mu_write", .mode = S_IWUSR, - .owner = THIS_MODULE, }, .size = 1032, .write = arcmsr_sysfs_iop_message_write,
@@ -209,7 +207,6 @@ static struct bin_attribute arcmsr_sysfs .attr = { .name = "mu_clear", .mode = S_IWUSR, - .owner = THIS_MODULE, }, .size = 1, .write = arcmsr_sysfs_iop_message_clear,
drivers/scsi/lpfc/lpfc_attr.c +0 -2
@@ -1145,7 +1145,6 @@ static struct bin_attribute sysfs_ctlreg .attr = { .name = "ctlreg", .mode = S_IRUSR | S_IWUSR, - .owner = THIS_MODULE, }, .size = 256, .read = sysfs_ctlreg_read,
@@ -1356,7 +1355,6 @@ static struct bin_attribute sysfs_mbox_a .attr = { .name = "mbox", .mode = S_IRUSR | S_IWUSR, - .owner = THIS_MODULE, }, .size = sizeof(MAILBOX_t), .read = sysfs_mbox_read,
drivers/scsi/qla2xxx/qla_attr.c +0 -6
@@ -73,7 +73,6 @@ static struct bin_attribute sysfs_fw_dum .attr = { .name = "fw_dump", .mode = S_IRUSR | S_IWUSR, - .owner = THIS_MODULE, }, .size = 0, .read = qla2x00_sysfs_read_fw_dump,
@@ -149,7 +148,6 @@ static struct bin_attribute sysfs_nvram_ .attr = { .name = "nvram", .mode = S_IRUSR | S_IWUSR, - .owner = THIS_MODULE, }, .size = 512, .read = qla2x00_sysfs_read_nvram,
@@ -198,7 +196,6 @@ static struct bin_attribute sysfs_optrom .attr = { .name = "optrom", .mode = S_IRUSR | S_IWUSR, - .owner = THIS_MODULE, }, .size = OPTROM_SIZE_24XX, .read = qla2x00_sysfs_read_optrom,
@@ -279,7 +276,6 @@ static struct bin_attribute sysfs_optrom .attr = { .name = "optrom_ctl", .mode = S_IWUSR, - .owner = THIS_MODULE, }, .size = 0, .write = qla2x00_sysfs_write_optrom_ctl,
@@ -327,7 +323,6 @@ static struct bin_attribute sysfs_vpd_at .attr = { .name = "vpd", .mode = S_IRUSR | S_IWUSR, - .owner = THIS_MODULE, }, .size = 0, .read = qla2x00_sysfs_read_vpd,
@@ -375,7 +370,6 @@ static struct bin_attribute sysfs_sfp_at .attr = { .name = "sfp", .mode = S_IRUSR | S_IWUSR, - .owner = THIS_MODULE, }, .size = SFP_DEV_SIZE * 2, .read = qla2x00_sysfs_read_sfp,
drivers/spi/at25.c +0 -1
@@ -314,7 +314,6 @@ static int at25_probe(struct spi_device*/ at25->bin.attr.name ="eeprom" ; at25->bin.attr.mode =S_IRUSR ; - at25->bin.attr.owner =THIS_MODULE ; at25->bin.read = at25_bin_read; at25->bin.size = at25->chip.byte_len;
drivers/video/aty/radeon_base.c +0 -2
@@ -2123,7 +2123,6 @@ static ssize_t radeon_show_edid2(structstatic struct bin_attribute edid1_attr = { .attr = { .name ="edid1" , - .owner =THIS_MODULE , .mode =0444 , }, .size =EDID_LENGTH ,
@@ -2133,7 +2132,6 @@ static struct bin_attribute edid1_attr =static struct bin_attribute edid2_attr = { .attr = { .name ="edid2" , - .owner =THIS_MODULE , .mode =0444 , }, .size =EDID_LENGTH ,
drivers/video/backlight/backlight.c +1 -1
@@ -172,7 +172,7 @@ static struct class backlight_class = {#define DECLARE_ATTR (_name,_mode,_show,_store) \ { \ - .attr = { .name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE }, \ + .attr = { .name = __stringify(_name), .mode = _mode }, \ .show = _show, \ .store = _store, \ }
drivers/video/backlight/lcd.c +1 -1
@@ -157,7 +157,7 @@ static struct class lcd_class = {#define DECLARE_ATTR (_name,_mode,_show,_store) \ { \ - .attr = { .name = __stringify(_name), .mode = _mode, .owner = THIS_MODULE }, \ + .attr = { .name = __stringify(_name), .mode = _mode }, \ .show = _show, \ .store = _store, \ }
drivers/w1/slaves/w1_ds2433.c +0 -1
@@ -252,7 +252,6 @@ static struct bin_attribute w1_f23_bin_a .attr = { .name = "eeprom", .mode = S_IRUGO | S_IWUSR, - .owner = THIS_MODULE, }, .size = W1_EEPROM_SIZE, .read = w1_f23_read_bin,
drivers/w1/slaves/w1_therm.c +0 -1
@@ -48,7 +48,6 @@ static struct bin_attribute w1_therm_bin .attr = { .name = "w1_slave", .mode = S_IRUGO, - .owner = THIS_MODULE, }, .size = W1_SLAVE_DATA_SIZE, .read = w1_therm_read_bin,
drivers/w1/w1.c +0 -2
@@ -128,7 +128,6 @@ static struct bin_attribute w1_slave_att .attr = { .name = "id", .mode = S_IRUGO, - .owner = THIS_MODULE, }, .size = 8, .read = w1_slave_read_id,
@@ -167,7 +166,6 @@ static struct bin_attribute w1_default_a .attr = { .name = "rw", .mode = S_IRUGO | S_IWUSR, - .owner = THIS_MODULE, }, .size = PAGE_SIZE, .read = w1_default_read,
arch/s390/kernel/ipl.c +0 -2
@@ -255,7 +255,6 @@ static struct bin_attribute ipl_paramete .attr = { .name = "binary_parameter", .mode = S_IRUGO, - .owner = THIS_MODULE, }, .size = PAGE_SIZE, .read = &ipl_parameter_read,
@@ -279,7 +278,6 @@ static struct bin_attribute ipl_scp_data .attr = { .name = "scp_data", .mode = S_IRUGO, - .owner = THIS_MODULE, }, .size = PAGE_SIZE, .read = &ipl_scp_data_read,
drivers/s390/cio/chsc.c +0 -2
@@ -898,7 +898,6 @@ static struct bin_attribute chp_measurem .attr = { .name = "measurement_chars", .mode = S_IRUSR, - .owner = THIS_MODULE, }, .size = sizeof(struct cmg_chars), .read = chp_measurement_chars_read,
@@ -950,7 +949,6 @@ static struct bin_attribute chp_measurem .attr = { .name = "measurement", .mode = S_IRUSR, - .owner = THIS_MODULE, }, .size = sizeof(struct cmg_entry), .read = chp_measurement_read,
drivers/s390/net/qeth_sys.c +1 -1
@@ -993,7 +993,7 @@ static struct attribute_group qeth_osn_d#define QETH_DEVICE_ATTR (_id,_name,_mode,_show,_store) \ struct device_attribute dev_attr_##_id = { \ - .attr = {.name=__stringify(_name), .mode=_mode, .owner=THIS_MODULE },\ + .attr = {.name=__stringify(_name), .mode=_mode, },\ .show = _show, \ .store = _store, \ };
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
In reply to Tejun HeoQuoted 7 lines · Tejun Heo
Cornelia, this should fix the problem you reported. It's caused by doing
up_write()onsysfs_direntwhich is used for readdir cursor which is not deactivated before being released.release_sysfs_dirent()is udpated such that it doesdown_write_trylock()ons_activebefore doingup_write(). This also covers error paths where allocatedsysfs_direntis put due to errors during initialization.
I can confirm that this fixes the problem I saw earlier. However, I now have a new one:
1. Define virtunal ctcs 7000 and 7001: This
creates /sys/bus/ccw/devices/{0.0.7000,0.0.7001}.
2. Do echo 0.0.7000,0.0.7001 > /sys/bus/ccwgroup/drivers/ctc/group. This creates /sys/bus/ccwgroup/devices/0.0.7000, which has the two ccw devices from above as slave devices.
3. Detach virtual ctcs 7000 and 7001. This causes the subchannel beneath 0.0.7000 resp. 0.0.7001 to be unregistered (from a workqueue) which will in turn trigger an unregistration of ccw device 0.0.7000, ccw group device 0.0.7000 and ccw device 0.0.7001. Now lockdep is unhappy:
crw_info : CRW reports slct=0, oflw=0, chn=0, rsc=3, anc=1, erc=4, rsid=F crw_info : CRW reports slct=0, oflw=0, chn=0, rsc=3, anc=1, erc=4, rsid=10
DEV: Unregistering device. ID = '0.0.000f'
bus css: remove device 0.0.000f
DEV: Unregistering device. ID = '0.0.7000'
bus ccw: remove device 0.0.7000
DEV: Unregistering device. ID = '0.0.7000'
bus ccwgroup: remove device 0.0.7000
=======================================================
[ INFO: possible circular locking dependency detected ] 2.6.21-rc6-ge666c753-dirty #54
-------------------------------------------------------
kslowcrw/64 is trying to acquire lock:
(&sch->reg_mutex){--..}, at: [<00000000004233b2>] mutex_lock+0x3e/0x4c
but task is already holding lock:
(&sd->s_active){----}, at: [<0000000000209578>] remove_dir+0xec/0x144
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&sd->s_active){----}:
[<0000000000156998>] __lock_acquire+0x1008/0x1124
[<0000000000156e9c>] lock_acquire+0x78/0xa8
[<000000000014fc96>] down_write+0x5a/0xa0
[<0000000000206f0a>] sysfs_hash_and_remove+0x112/0x15c
[<00000000002071a6>] sysfs_remove_file+0x32/0x40
[<00000000002a76bc>] device_remove_file+0x44/0x5c
[<00000000002a813a>] device_del+0x1ae/0x26c
[<00000000002a8236>] device_unregister+0x3e/0x58
[<00000000002f0bc0>] css_sch_device_unregister+0x3c/0x54
[<00000000002f174a>] css_evaluate_subchannel+0x266/0x3b8
[<00000000002f1976>] css_trigger_slow_path+0xda/0x154
[<0000000000144a6c>] run_workqueue+0x174/0x220
[<0000000000144d26>] worker_thread+0x116/0x164
[<000000000014ab9c>] kthread+0x158/0x160
[<0000000000107c3a>] kernel_thread_starter+0x6/0xc
[<0000000000107c34>] kernel_thread_starter+0x0/0xc-> #0 (&sch->reg_mutex){--..}:
[<000000000015671c>] __lock_acquire+0xd8c/0x1124
[<0000000000156e9c>] lock_acquire+0x78/0xa8
[<0000000000423070>] __mutex_lock_slowpath+0xbc/0x3c0
[<00000000004233b2>] mutex_lock+0x3e/0x4c
[<00000000002f0bb6>] css_sch_device_unregister+0x32/0x54
[<00000000002f174a>] css_evaluate_subchannel+0x266/0x3b8
[<00000000002f1976>] css_trigger_slow_path+0xda/0x154
[<0000000000144a6c>] run_workqueue+0x174/0x220
[<0000000000144d26>] worker_thread+0x116/0x164
[<000000000014ab9c>] kthread+0x158/0x160
[<0000000000107c3a>] kernel_thread_starter+0x6/0xc
[<0000000000107c34>] kernel_thread_starter+0x0/0xcother info that might help us debug this:
1 lock held by kslowcrw/64:
#0: (&sd->s_active){----}, at: [<0000000000209578>] remove_dir+0xec/0x144
stack backtrace:
0000000000000002 0000000000000002 0000000000000000 000000001fd1ba50
000000001fd1b9c8 00000000004bebf6 00000000004bebf6 0000000000103854
0000000000000000 0000000000000001 0000000000000000 0000000000602cb0
0000000000000000 000000001fd1ba10 000000001fd1b9b0 000000000000000e
000000000042a780 000000000010389e 000000001fd1b9b0 000000001fd1ba00
Call Trace:
([<00000000001037f4>] show_trace+0xf8/0xfc)
[<00000000001038ce>] show_stack+0xd6/0x100
[<0000000000103926>] dump_stack+0x2e/0x3c
[<0000000000154c6e>] print_circular_bug_tail+0x9e/0xb0
[<000000000015671c>] __lock_acquire+0xd8c/0x1124
[<0000000000156e9c>] lock_acquire+0x78/0xa8
[<0000000000423070>] __mutex_lock_slowpath+0xbc/0x3c0
[<00000000004233b2>] mutex_lock+0x3e/0x4c
[<00000000002f0bb6>] css_sch_device_unregister+0x32/0x54
[<00000000002f174a>] css_evaluate_subchannel+0x266/0x3b8
[<00000000002f1976>] css_trigger_slow_path+0xda/0x154
[<0000000000144a6c>] run_workqueue+0x174/0x220
[<0000000000144d26>] worker_thread+0x116/0x164
[<000000000014ab9c>] kthread+0x158/0x160
[<0000000000107c3a>] kernel_thread_starter+0x6/0xc
[<0000000000107c34>] kernel_thread_starter+0x0/0xcINFO: lockdep is turned off.
DEV: Unregistering device. ID = '0.0.0010'
bus css: remove device 0.0.0010
DEV: Unregistering device. ID = '0.0.7001'
bus ccw: remove device 0.0.7001
(We use sch->reg_mutex to protect against concurrent register/unregister of subchannels.)
I'll see whether I can figure out this one.
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
Hello,
Quoted 11 lines · Cornelia Huck
=======================================================
[ INFO: possible circular locking dependency detected ] 2.6.21-rc6-ge666c753-dirty #54
-------------------------------------------------------
kslowcrw/64 is trying to acquire lock:
(&sch->reg_mutex){--..}, at: [<00000000004233b2>]mutex_lock+0x3e/0x4cbut task is already holding lock:
(&sd->s_active){----}, at: [<0000000000209578>]remove_dir+0xec/0x144which lock already depends on the new lock.
This is probably because s_active has different write locked and unlocked by different threads but it doesn't tell lockdep about it. I'll read lockdep docs and update it.
Thanks for testing.
Signature · 1 line
tejun
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
Quoted 10 lines · Tejun Heo
Hello,
Quoted 7 lines · Cornelia Huck
=======================================================
[ INFO: possible circular locking dependency detected ] 2.6.21-rc6-ge666c753-dirty #54
-------------------------------------------------------
kslowcrw/64 is trying to acquire lock:
(&sch->reg_mutex){--..}, at: [<00000000004233b2>]mutex_lock+0x3e/0x4c
Quoted 7 lines
Quoted 4 lines
but task is already holding lock:
(&sd->s_active){----}, at: [<0000000000209578>]remove_dir+0xec/0x144which lock already depends on the new lock.
This is probably because
s_activehas different write locked and unlocked by different threads but it doesn't tell lockdep about it.
s/has different/is/
Sorry.
Signature · 1 line
tejun
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
In reply to Tejun HeoQuoted 10 lines · Tejun Heo
Hello,
Quoted 7 lines · Cornelia Huck
=======================================================
[ INFO: possible circular locking dependency detected ] 2.6.21-rc6-ge666c753-dirty #54
-------------------------------------------------------
kslowcrw/64 is trying to acquire lock:
(&sch->reg_mutex){--..}, at: [<00000000004233b2>]mutex_lock+0x3e/0x4c
Quoted 8 lines
Quoted 4 lines
but task is already holding lock:
(&sd->s_active){----}, at: [<0000000000209578>]remove_dir+0xec/0x144which lock already depends on the new lock.
This is probably because
s_activehas different write locked and unlocked by different threads but it doesn't tell lockdep about it. I'll read lockdep docs and update it.
Does this patch fix the problem?
fs/sysfs/dir.c +2 -1
@@ -26,7 +26,8 @@ void release_sysfs_dirent(struct sysfs_d* locked. If @sd is cursor for directory walk or being * released prematurely, s_active has no reader or writer. */ -down_write_trylock (&sd->s_active); +if (!down_write_trylock (&sd->s_active)) +rwsem_acquire (&sd->s_active.dep_map,0 ,0 , _RET_IP_);up_write (&sd->s_active);if (sd->s_type &SYSFS_KOBJ_LINK )
fs/sysfs/sysfs.h +1 -0
@@ -171,6 +171,7 @@ static inline void sysfs_put_active_two(static inline void sysfs_deactivate (struct sysfs_dirent *sd) {down_write (&sd->s_active); +rwsem_release (&sd->s_active.dep_map,1 , _RET_IP_); }static inline int sysfs_is_shadowed_inode (struct inode *inode)
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
Quoted 21 lines · Tejun Heo
Does this patch fix the problem?
Index:
work/fs/sysfs/dir.c
===================================================================fs/sysfs/dir.c +2
-1@@ -26,7 +26,8 @@ void release_sysfs_dirent(struct sysfs_d* locked. If @sd is cursor for directory walk or being * released prematurely, s_active has no reader or writer. */ -down_write_trylock (&sd->s_active); +if (!down_write_trylock (&sd->s_active)) +rwsem_acquire (&sd->s_active.dep_map,0 ,0 , _RET_IP_);up_write (&sd->s_active);if (sd->s_type &SYSFS_KOBJ_LINK )Index:
work/fs/sysfs/sysfs.h
===================================================================
fs/sysfs/sysfs.h static inline void sysfs_put_active_two( · line 177
Quoted 7 lines
fs/sysfs/sysfs.h +1
-0@@ -171,6 +171,7 @@ static inline void sysfs_put_active_two(static inline void sysfs_deactivate (struct sysfs_dirent *sd) {down_write (&sd->s_active); +rwsem_release (&sd->s_active.dep_map,1 , _RET_IP_); }static inline int sysfs_is_shadowed_inode (struct inode *inode)
This seems to work fine now, thanks.
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
In reply to Tejun HeoQuoted 1 line · Tejun Heo
[PATCH] sysfs: implement
sysfs_direntactive reference and immediate disconnect
For some reason, this doesn't apply on top of your patches:
Applying patch sysfs-implement-sysfs_dirent-active-reference-and-immediate-disconnect.patch patching file fs/sysfs/dir.c
patching file fs/sysfs/sysfs.h
patching file fs/sysfs/bin.c
patching file fs/sysfs/file.c
Hunk #1 succeeded at 96 (offset 8 lines).
Hunk #2 succeeded at 107 (offset 8 lines).
Hunk #3 succeeded at 240 (offset 8 lines).
Hunk #4 FAILED at 300.
Hunk #5 succeeded at 336 (offset 14 lines).
Hunk #6 succeeded at 368 (offset 14 lines).
Hunk #7 succeeded at 435 (offset 14 lines).
1 out of 7 hunks FAILED -- rejects in file fs/sysfs/file.c
patching file fs/sysfs/inode.c
So I'll only apply the first 11 for now. Can you respin the last 3 and send them to me again?
thanks,
greg k-h
Re: [PATCH 12/14 UPDATED] sysfs: implement sysfs_dirent active reference and immediate disconnect
Quoted 20 lines · Greg KH
Quoted 1 line · Tejun Heo
[PATCH] sysfs: implement
sysfs_direntactive reference and immediate disconnectFor some reason, this doesn't apply on top of your patches:
Applying patch sysfs-implement-sysfs_dirent-active-reference-and-immediate-disconnect.patch patching file
fs/sysfs/dir.c
patching filefs/sysfs/sysfs.h
patching filefs/sysfs/bin.c
patching filefs/sysfs/file.c
Hunk #1 succeeded at 96 (offset 8 lines).
Hunk #2 succeeded at 107 (offset 8 lines).
Hunk #3 succeeded at 240 (offset 8 lines).
Hunk #4 FAILED at 300.
Hunk #5 succeeded at 336 (offset 14 lines).
Hunk #6 succeeded at 368 (offset 14 lines).
Hunk #7 succeeded at 435 (offset 14 lines).
1 out of 7 hunks FAILED -- rejects in filefs/sysfs/file.c
patching filefs/sysfs/inode.c
Quoted 2 lines
So I'll only apply the first 11 for now. Can you respin the last 3 and send them to me again?
Oh nevermind, this was my fault, it was due to something else of mine that isn't in mainline, sorry for the noise...
greg k-h
Quoted 43 lines · Tejun Heo
Hello, all.
This is the second take of sysfs-immediate-disconnct patchset.
In the last take, rwsem was added to s_elem.dir to protect kobj only. This wasn't enough because attr and
bin_attrneed to hold onto not only the kobject of their parents but also the module backing themselves and ops too, so the first set still needed separate and duplicate attribute file orphaning mechanism.In this take, the rwsem is generalized to become active reference count. Now each
sysfs_direnthas two reference counts -s_countands_active.s_countis a regular reference count which guarantees that the containingsysfs_direntis accessible. As long ass_countreference is held, all sysfs internal fields insysfs_direntare accessible includings_parentands_name.The newly added
s_activeis active reference count. This is acquired by invokingsysfs_get_active()and it's the caller's responsibility to ensuresysfs_direntitself is accessible (should be holdings_countone way or the other). Dereferencingsysfs_direntto access objects out of sysfs proper requires active reference. This includes access to the associated kobjects, attributes and ops.Because attr/bin_attr ops access both the node itself and its parent for kobject, they need to hold active references to both. sysfs_get/put_active_two() helpers are provided to help grabbing both references. Parent's is acquired first and released last.
Basically,
s_countprovides the reference counted objects to the upper layer whiles_activeguards low level access such that low level objects can just go away when they want to, and the same mechanism is applied to all types of sysfs nodes. I think it's conceptually cleaner and thus easier to understand this way.With all the patches applied, the same test used in the last take ran 9+hrs without any problem.
Change from the last take are...
* Patch 3 now doesn't move
sysfs_get_kobject()as the it's replaced by active references later.
Quoted 7 lines
* Patch 12 updated such that
sdir->rwsemis generalized into active reference count.Please read the original lifetime rules discussion[1] and description of the last take[2] for more info.
Thanks.
Hi Tejun,
I started looking at these patches and parallely also did some testing on a 8 CPU system. I am using the patches from Greg's tree at
http://www.kernel.org/pub/scm/linux/kernel/git/gregkh/patches.git/
I ran following loops parallelly
# while true; do insmod drivers/net/dummy.ko; sleep 1;rmmod dummy; done # while true; do find /sys/class/net/dummy0 | xargs cat; sleep 1; done # while true; do umount /sys; sleep 1; mount -t sysfs none /sys; done # while true; do find /sys | xargs cat > /dev/null; sleep 1; done
and got the following oops
Unable to handle kernel NULL pointer dereference at 000000000000004c RIP: [<ffffffff802935b4>] simple_unlink+0x14/0x5c PGD 21955c067 PUD 215b52067 PMD 0 Oops: 0002 [1] SMP CPU 6 Modules linked in: dummy i2c_dev i2c_core Pid: 21161, comm: rmmod Not tainted 2.6.21-rc6 #3 RIP: 0010:[<ffffffff802935b4>] [<ffffffff802935b4>] simple_unlink+0x14/0x5c RSP: 0000:ffff81021b38be28 EFLAGS: 00010292 RAX: 0000000046232944 RBX: ffff8102173528b0 RCX: 0000000046232944 RDX: 00000000256a534c RSI: 00000000256a534c RDI: ffff8102173528b0 RBP: ffff81021be04a38 R08: ffff81021b38a000 R09: ffff81021b38bdc8 R10: ffffffff8085d1a0 R11: ffff8102150c5480 R12: 0000000000000000 R13: ffff81021487f8a0 R14: ffff81021be043f8 R15: ffffffff80632f68 FS: 00002b3f92906240(0000) GS:ffff8102284b14c0(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b CR2: 000000000000004c CR3: 0000000218573000 CR4: 00000000000006e0 Process rmmod (pid: 21161, threadinfo ffff81021b38a000, task ffff81022730d8b0) Stack: ffff81021be04a10 ffff81021be04f60 ffff81021d361150 ffffffff802b31ee 0000000000200200 ffff81022505c000 ffff81022505c3e0 ffff81022505c4d0 0000000000000000 00007fff181c4160 0000000000000880 ffffffff803a0c1a Call Trace: [<ffffffff802b31ee>] sysfs_hash_and_remove+0x7c/0xef [<ffffffff803a0c1a>] device_del+0x66/0x20a [<ffffffff804d2d7e>] netdev_run_todo+0xc6/0x225 [<ffffffff8800d025>] :dummy:dummy_free_one+0x1c/0x2d [<ffffffff8800d0a2>] :dummy:dummy_cleanup_module+0xe/0x23 [<ffffffff8024ceed>] sys_delete_module+0x1b1/0x1e0 [<ffffffff803437e7>] __up_write+0x21/0x10e [<ffffffff80209bbe>] system_call+0x7e/0x83 Code: 41 ff 4c 24 4c 48 89 83 90 00 00 00 4c 89 ef 48 89 93 98 00 RIP [<ffffffff802935b4>] simple_unlink+0x14/0x5c RSP <ffff81021b38be28>
CR2: 000000000000004c
Thanks
Maneesh
Signature · 4 lines
Maneesh Soni Linux Technology Center, IBM India Systems and Technology Lab, Bangalore, India
Hello, Maneesh.
Quoted 11 lines · Maneesh Soni
I started looking at these patches and parallely also did some testing on a 8 CPU system. I am using the patches from Greg's tree at
http://www.kernel.org/pub/scm/linux/kernel/git/gregkh/patches.git/I ran following loops parallelly
# while true; do insmod drivers/net/dummy.ko; sleep 1;rmmod dummy; done # while true; do find /sys/class/net/dummy0 | xargs cat; sleep 1; done # while true; do umount /sys; sleep 1; mount -t sysfs none /sys; done # while true; do find /sys | xargs cat > /dev/null; sleep 1; done
Quoted 4 lines
and got the following oops
Unable to handle kernel NULL pointer dereference at 000000000000004c RIP: [<ffffffff802935b4>] simple_unlink+0x14/0x5c
Eeek... I'll try to replicate and track down the bug here. FWIW, SCSI also oopses if udev is running due to a bug in SCSI open/close handling.
Thanks for testing.
Signature · 1 line
tejun
Quoted 13 lines · Maneesh Soni
I started looking at these patches and parallely also did some testing on a 8 CPU system. I am using the patches from Greg's tree at
http://www.kernel.org/pub/scm/linux/kernel/git/gregkh/patches.git/I ran following loops parallelly
# while true; do insmod drivers/net/dummy.ko; sleep 1;rmmod dummy; done # while true; do find /sys/class/net/dummy0 | xargs cat; sleep 1; done # while true; do umount /sys; sleep 1; mount -t sysfs none /sys; done # while true; do find /sys | xargs cat > /dev/null; sleep 1; doneand got the following oops
Quoted 9 lines
Unable to handle kernel NULL pointer dereference at 000000000000004c RIP: [<ffffffff802935b4>] simple_unlink+0x14/0x5c PGD 21955c067 PUD 215b52067 PMD 0 Oops: 0002 [1] SMP CPU 6 Modules linked in: dummy i2c_dev i2c_core Pid: 21161, comm: rmmod Not tainted 2.6.21-rc6 #3 RIP: 0010:[<ffffffff802935b4>] [<ffffffff802935b4>] simple_unlink+0x14/0x5c RSP: 0000:ffff81021b38be28 EFLAGS: 00010292
Okay, got it. The problem here is the race between dcache shrinking triggered by umount and sysfs deletion. It seems to be introduced when dentries for attr and symlink nodes are made unpinned. sd->s_entry clearing is done without synchronization and sysfs_drop_entry() ends up deleting already deleted dentry (dentry->inode is NULL).
sd->s_entry is broken in other ways too. Consider the following scenario.
thread shrinking dcache thread looking up sysfs entry
--------------------------------------------------------------------
1. sysfs dentry for A is chosen as
victim.
2. prune_one_dentry() drops the dentry
and calls dentry_iput().
3. dentry_iput() unlinks d_alias and
releases spin locks.
4. looks up dentry for A which is not in dcache.
5. new dentry is created and sysfs_lookup() is invoked, which instantiates the dentry and set sd->s_dentry to it.
6. sysfs_d_iput() is called.
BUG_ON(sd->s_dentry != dentry)
triggers and sd->s_dentry is
cleared. You're screwed.
I think it can be fixed by making deletion more like conventional filesystem. Brewing a patch...
Signature · 1 line
tejun
Re: [PATCH 01/14] sysfs: fix i_ino handling in sysfs
In reply to Tejun HeoQuoted 9 lines · Tejun Heo
Inode number handling was incorrect in two ways.
1. sysfs uses the inode number allocated by
new_inode()and never hashes it. When reporting the inode number, it usesiunique()if inode is inaccessible. This is incorrect becauseiunique()assumes the inodes are hashed. This can cause duplicate inode numbers and the condition is likely to happen becausenew_inode()andiunique()use separate increasing static counters to scan for empty slot.
Quoted 8 lines
2.
sysfs_dirent->s_dentrycan go away anytime and can't be referenced unless the caller knows the dentry is not and not going to be deleted.This patch makes sysfs report the pointer to
sysfs_direntas ino.ino_tis always as big as or larger than unsigned long &&sysfs_direnthierarchy is the internal representation of the sysfs tree, so it makes sense and simple to implement.
what about 32-bit stats from 32-bit apps on 64-bit systems? This will make 64-bit inode numbers commonplace in sysfs; will this cause problems? it seems that if they get truncated to 32 bits the possibility of duplicate inode nrs will come back...
-Eric
Re: [PATCH 01/14] sysfs: fix i_ino handling in sysfs
Quoted 16 lines · Eric Sandeen
Quoted 6 lines · Tejun Heo
Inode number handling was incorrect in two ways.
1. sysfs uses the inode number allocated bynew_inode()and never hashes it. When reporting the inode number, it usesiunique()if inode is inaccessible. This is incorrect becauseiunique()assumes the inodes are hashed. This can cause duplicate inode numbers and the condition is likely to happen becausenew_inode()andiunique()Quoted 8 lines
use separate increasing static counters to scan for empty slot.
2.sysfs_dirent->s_dentrycan go away anytime and can't be referenced unless the caller knows the dentry is not and not going to be deleted.
This patch makes sysfs report the pointer tosysfs_direntas ino.ino_tis always as big as or larger than unsigned long &&sysfs_direnthierarchy is the internal representation of the sysfs tree, so it makes sense and simple to implement.
Quoted 4 lines
what about 32-bit stats from 32-bit apps on 64-bit systems? This will make 64-bit inode numbers commonplace in sysfs; will this cause problems? it seems that if they get truncated to 32 bits the possibility of duplicate inode nrs will come back...
Yes, this turned out to be a problem as the ppc people found out :)
Tejun had some follow-on patches fixing this issue up.
thanks,
greg k-h
Re: [PATCH 01/14] sysfs: fix i_ino handling in sysfs
Quoted 9 lines · Greg KH
Quoted 4 lines · Eric Sandeen
what about 32-bit stats from 32-bit apps on 64-bit systems? This will make 64-bit inode numbers commonplace in sysfs; will this cause problems? it seems that if they get truncated to 32 bits the possibility of duplicate inode nrs will come back...
Yes, this turned out to be a problem as the ppc people found out :)
Tejun had some follow-on patches fixing this issue up.
Oops, missed that. I'll go looking, thanks.
-Eric