From e72ceb8ccac5f770b3e696e09bb673dca7024b20 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Thu, 11 Feb 2010 15:18:38 -0800 Subject: sysfs: Remove sysfs_get/put_active_two It turns out that holding an active reference on a directory is pointless. The purpose of the active references are to allows us to block when removing sysfs entries that have custom methods so we don't remove modules while running modular code and to keep those custom methods from accessing data structures after the files have been removed. Further sysfs_remove_dir remove all elements in the directory before removing the directory itself, so there is no chance we will remove a directory with active children. Signed-off-by: Eric W. Biederman Cc: Tejun Heo Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/sysfs.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs/sysfs/sysfs.h') diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h index cdd9377a6e06..bb7723c4f5a5 100644 --- a/fs/sysfs/sysfs.h +++ b/fs/sysfs/sysfs.h @@ -124,8 +124,8 @@ extern const struct file_operations sysfs_dir_operations; extern const struct inode_operations sysfs_dir_inode_operations; struct dentry *sysfs_get_dentry(struct sysfs_dirent *sd); -struct sysfs_dirent *sysfs_get_active_two(struct sysfs_dirent *sd); -void sysfs_put_active_two(struct sysfs_dirent *sd); +struct sysfs_dirent *sysfs_get_active(struct sysfs_dirent *sd); +void sysfs_put_active(struct sysfs_dirent *sd); void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *parent_sd); int __sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd); -- cgit v1.2.3 From a2db6842873c8e5a70652f278d469128cb52db70 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Thu, 11 Feb 2010 15:20:00 -0800 Subject: sysfs: Only take active references on attributes. If we exclude directories and symlinks from the set of sysfs dirents where we need active references we are left with sysfs attributes (binary or not). - Tweak sysfs_deactivate to only do something on attributes - Move lockdep initialization into sysfs_file_add_mode to limit it to just attributes. Signed-off-by: Eric W. Biederman Acked-by: WANG Cong Cc: Tejun Heo Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/dir.c | 5 ++++- fs/sysfs/file.c | 1 + fs/sysfs/sysfs.h | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) (limited to 'fs/sysfs/sysfs.h') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index 1bdc42f4fd93..481fdec09f4e 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -156,6 +156,10 @@ static void sysfs_deactivate(struct sysfs_dirent *sd) int v; BUG_ON(sd->s_sibling || !(sd->s_flags & SYSFS_FLAG_REMOVED)); + + if (!(sysfs_type(sd) & SYSFS_ACTIVE_REF)) + return; + sd->s_sibling = (void *)&wait; rwsem_acquire(&sd->dep_map, 0, 0, _RET_IP_); @@ -315,7 +319,6 @@ struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type) atomic_set(&sd->s_count, 1); atomic_set(&sd->s_active, 0); - sysfs_dirent_init_lockdep(sd); sd->s_name = name; sd->s_mode = mode; diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index 40961366e929..e222b2582746 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -509,6 +509,7 @@ int sysfs_add_file_mode(struct sysfs_dirent *dir_sd, if (!sd) return -ENOMEM; sd->s_attr.attr = (void *)attr; + sysfs_dirent_init_lockdep(sd); sysfs_addrm_start(&acxt, dir_sd); rc = sysfs_add_one(&acxt, sd); diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h index bb7723c4f5a5..7db6884f4206 100644 --- a/fs/sysfs/sysfs.h +++ b/fs/sysfs/sysfs.h @@ -79,6 +79,7 @@ struct sysfs_dirent { #define SYSFS_KOBJ_BIN_ATTR 0x0004 #define SYSFS_KOBJ_LINK 0x0008 #define SYSFS_COPY_NAME (SYSFS_DIR | SYSFS_KOBJ_LINK) +#define SYSFS_ACTIVE_REF (SYSFS_KOBJ_ATTR | SYSFS_KOBJ_BIN_ATTR) #define SYSFS_FLAG_MASK ~SYSFS_TYPE_MASK #define SYSFS_FLAG_REMOVED 0x0200 -- cgit v1.2.3 From 6992f5334995af474c2b58d010d08bc597f0f2fe Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Thu, 11 Feb 2010 15:21:53 -0800 Subject: sysfs: Use one lockdep class per sysfs attribute. Acknowledge that the logical sysfs rwsem has one instance per sysfs attribute with different locking depencencies for different attributes. There is a sysfs idiom where writing to one sysfs file causes the addition or removal of other sysfs files. Lumping all of the sysfs attributes together in one lock class causes lockdep to generate lots of false positives. This introduces the requirement that non-static sysfs attributes need to be initialized with sysfs_attr_init or sysfs_bin_attr_init. Strictly speaking this requirement only exists when lockdep is enabled, and when lockdep is enabled we get a bit fat warning if this requirement is not met. Signed-off-by: Eric W. Biederman Acked-by: WANG Cong Cc: Tejun Heo Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/sysfs.h | 7 +++++-- include/linux/sysfs.h | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) (limited to 'fs/sysfs/sysfs.h') diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h index 7db6884f4206..37e0e086233c 100644 --- a/fs/sysfs/sysfs.h +++ b/fs/sysfs/sysfs.h @@ -92,9 +92,12 @@ static inline unsigned int sysfs_type(struct sysfs_dirent *sd) #ifdef CONFIG_DEBUG_LOCK_ALLOC #define sysfs_dirent_init_lockdep(sd) \ do { \ - static struct lock_class_key __key; \ + struct attribute *attr = sd->s_attr.attr; \ + struct lock_class_key *key = attr->key; \ + if (!key) \ + key = &attr->skey; \ \ - lockdep_init_map(&sd->dep_map, "s_active", &__key, 0); \ + lockdep_init_map(&sd->dep_map, "s_active", key, 0); \ } while(0) #else #define sysfs_dirent_init_lockdep(sd) do {} while(0) diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h index 3e8526582146..006c359e63c0 100644 --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -15,6 +15,7 @@ #include #include #include +#include #include struct kobject; @@ -29,8 +30,23 @@ struct attribute { const char *name; struct module *owner; mode_t mode; +#ifdef CONFIG_DEBUG_LOCK_ALLOC + struct lock_class_key *key; + struct lock_class_key skey; +#endif }; +#ifdef CONFIG_DEBUG_LOCK_ALLOC +#define sysfs_attr_init(attr) \ +do { \ + static struct lock_class_key __key; \ + \ + (attr)->key = &__key; \ +} while(0) +#else +#define sysfs_attr_init(attr) do {} while(0) +#endif + struct attribute_group { const char *name; mode_t (*is_visible)(struct kobject *, @@ -74,6 +90,8 @@ struct bin_attribute { struct vm_area_struct *vma); }; +#define sysfs_bin_attr_init(bin_attr) sysfs_attr_init(&bin_attr->attr) + struct sysfs_ops { ssize_t (*show)(struct kobject *, struct attribute *,char *); ssize_t (*store)(struct kobject *,struct attribute *,const char *, size_t); -- cgit v1.2.3 From 19c38b632d4554329986021fd66bdea468cf452c Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 12 Feb 2010 19:22:24 -0800 Subject: sysfs: Pack sysfs_dirent more tightly. Placing the 16bit s_mode between a pointer and a long doesn't pack well especailly on 64bit where we wast 48 bits. So move s_mode and declare it as a unsigned short. This is the sysfs backing store after all we don't need fields extra large just in case someday we want userspace to be able to use a larger value. Acked-by: Tejun Heo Signed-off-by: Eric W. Biederman Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/sysfs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/sysfs/sysfs.h') diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h index 37e0e086233c..5a3192a08442 100644 --- a/fs/sysfs/sysfs.h +++ b/fs/sysfs/sysfs.h @@ -66,8 +66,8 @@ struct sysfs_dirent { }; unsigned int s_flags; + unsigned short s_mode; ino_t s_ino; - umode_t s_mode; struct sysfs_inode_attrs *s_iattr; }; -- cgit v1.2.3 From fac2622bbad4d7e3a32c53e631e018b80ec631dc Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 12 Feb 2010 19:22:27 -0800 Subject: sysfs: Pass super_block to sysfs_get_inode Currently sysfs_get_inode magically returns an inode on sysfs_sb. Make the super_block parameter explicit and the code becomes clearer. Acked-by: Tejun Heo Acked-by: Serge Hallyn Signed-off-by: Eric W. Biederman Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/dir.c | 2 +- fs/sysfs/inode.c | 5 +++-- fs/sysfs/mount.c | 2 +- fs/sysfs/sysfs.h | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) (limited to 'fs/sysfs/sysfs.h') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index 481fdec09f4e..590717861c7a 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -645,7 +645,7 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry, } /* attach dentry and inode */ - inode = sysfs_get_inode(sd); + inode = sysfs_get_inode(dir->i_sb, sd); if (!inode) { ret = ERR_PTR(-ENOMEM); goto out_unlock; diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c index 0d09f6c6efab..082daaecac1b 100644 --- a/fs/sysfs/inode.c +++ b/fs/sysfs/inode.c @@ -283,6 +283,7 @@ static void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode) /** * sysfs_get_inode - get inode for sysfs_dirent + * @sb: super block * @sd: sysfs_dirent to allocate inode for * * Get inode for @sd. If such inode doesn't exist, a new inode @@ -295,11 +296,11 @@ static void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode) * RETURNS: * Pointer to allocated inode on success, NULL on failure. */ -struct inode * sysfs_get_inode(struct sysfs_dirent *sd) +struct inode * sysfs_get_inode(struct super_block *sb, struct sysfs_dirent *sd) { struct inode *inode; - inode = iget_locked(sysfs_sb, sd->s_ino); + inode = iget_locked(sb, sd->s_ino); if (inode && (inode->i_state & I_NEW)) sysfs_init_inode(sd, inode); diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c index 49749955ccaf..89db07e489b0 100644 --- a/fs/sysfs/mount.c +++ b/fs/sysfs/mount.c @@ -54,7 +54,7 @@ static int sysfs_fill_super(struct super_block *sb, void *data, int silent) /* get root inode, initialize and unlock it */ mutex_lock(&sysfs_mutex); - inode = sysfs_get_inode(&sysfs_root); + inode = sysfs_get_inode(sb, &sysfs_root); mutex_unlock(&sysfs_mutex); if (!inode) { pr_debug("sysfs: could not get root inode\n"); diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h index 5a3192a08442..7593d71d92f8 100644 --- a/fs/sysfs/sysfs.h +++ b/fs/sysfs/sysfs.h @@ -172,7 +172,7 @@ static inline void __sysfs_put(struct sysfs_dirent *sd) /* * inode.c */ -struct inode *sysfs_get_inode(struct sysfs_dirent *sd); +struct inode *sysfs_get_inode(struct super_block *sb, struct sysfs_dirent *sd); void sysfs_delete_inode(struct inode *inode); int sysfs_sd_setattr(struct sysfs_dirent *sd, struct iattr *iattr); int sysfs_permission(struct inode *inode, int mask); -- cgit v1.2.3 From 0f4288ec6fcc1a47d1fa0241ec1c6dacd5a09e96 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 12 Feb 2010 19:22:28 -0800 Subject: sysfs: Kill unused sysfs_sb variable. Now that there are no more users we can remove the sysfs_sb variable. Acked-by: Tejun Heo Acked-by: Serge Hallyn Signed-off-by: Eric W. Biederman Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/mount.c | 2 -- fs/sysfs/sysfs.h | 1 - 2 files changed, 3 deletions(-) (limited to 'fs/sysfs/sysfs.h') diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c index 89db07e489b0..0cb10884a2fc 100644 --- a/fs/sysfs/mount.c +++ b/fs/sysfs/mount.c @@ -23,7 +23,6 @@ static struct vfsmount *sysfs_mount; -struct super_block * sysfs_sb = NULL; struct kmem_cache *sysfs_dir_cachep; static const struct super_operations sysfs_ops = { @@ -50,7 +49,6 @@ static int sysfs_fill_super(struct super_block *sb, void *data, int silent) sb->s_magic = SYSFS_MAGIC; sb->s_op = &sysfs_ops; sb->s_time_gran = 1; - sysfs_sb = sb; /* get root inode, initialize and unlock it */ mutex_lock(&sysfs_mutex); diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h index 7593d71d92f8..30f5a44fb5d3 100644 --- a/fs/sysfs/sysfs.h +++ b/fs/sysfs/sysfs.h @@ -115,7 +115,6 @@ struct sysfs_addrm_cxt { * mount.c */ extern struct sysfs_dirent sysfs_root; -extern struct super_block *sysfs_sb; extern struct kmem_cache *sysfs_dir_cachep; /* -- cgit v1.2.3