Re: [PATCH 3/4] sysfs: Use one lockdep class per sysfs attribute.

["Américo Wang" <xiyou.wangcong@gmail.com>]

On Thu, Feb 11, 2010 at 03:21:53PM -0800, Eric W. Biederman wrote:
>
>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.

Since there are fewer non-static attributes, this will not
waste much space.

>
>Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>


Acked-by: WANG Cong <xiyou.wangcong@gmail.com>

>---
> fs/sysfs/sysfs.h      |    7 +++++--
> include/linux/sysfs.h |   18 ++++++++++++++++++
> 2 files changed, 23 insertions(+), 2 deletions(-)
>
>diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
>index 7db6884..37e0e08 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 cfa8308..c3048de 100644
>--- a/include/linux/sysfs.h
>+++ b/include/linux/sysfs.h
>@@ -15,6 +15,7 @@
> #include <linux/compiler.h>
> #include <linux/errno.h>
> #include <linux/list.h>
>+#include <linux/lockdep.h>
> #include <asm/atomic.h>
> 
> 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);
>-- 
>1.6.5.2.143.g8cc62
>

-- 
Live like a child, think like the god.