Re: [PATCH 2/4] sysfs: Only take active references on attributes.

[AmÃrico Wang]

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

Why?

If I read your patch correctly, s_active will be useless
for non-attributes sysfs entries? For sysfs dir, maybe,
since it can only be removed by sysfs_remove_dir(),
but not sure about sysfs symlinks...


>
>Signed-off-by: Eric W. Biederman <ebiederm@xxxxxxxxxxxx>
>---
> fs/sysfs/dir.c   |    5 ++++-
> fs/sysfs/file.c  |    1 +
> fs/sysfs/sysfs.h |    1 +
> 3 files changed, 6 insertions(+), 1 deletions(-)
>
>diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
>index 1bdc42f..481fdec 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 8d6bd65..8eaae5d 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 bb7723c..7db6884 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
>-- 
>1.6.5.2.143.g8cc62
>

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