2007-09-20 08:32:21

by Tejun Heo

[permalink] [raw]
Subject: [PATCHSET 4/4] sysfs: implement new features

Hello, all.

This is the fourth patchset of four sysfs update patchset series[1]
and to be applied on top of the third patchset[2].

This patchset implements the following new features.

* Notify pollers on file deactivation.

* Name-formatting for symlinks. e.g. symlink pointing to
/dira/dirb/leaf can be named as "symlink:%1-%0" and it will show up
as "symlink:dirb-leaf". This only applies when new interface is
used.

* Autoremoval of symlinks when target is removed. This only applies
when new interface is used.

* Autorenaming of symlinks according to the name format string when
target or one of its ancestors is renamed or moved. This only
applies when new interface is used.

* Plugged operations. Sysfs users can plug top node and build subtree
gradually without revealing the process to userland. When subtree
is fully constructed, the top node can be unplugged and userland
will see completely built subtree appearing at once. If subtree
creation fails in the process, the whole subtree can be removed by
simply removing the top node. There won't be any userland
noticeable event. This is to be combined with uevent_suppress
mechanism of driver model.

* Batch error handling. A plugged node accumulates any error
condition occurring below it and can return the first error when
asked. Also, all interface functions accepth ERR_PTR() value as
sysfs_dirent parameter. This means that constructs like the
following can be used to replace the current group interface.

<<-- code -->>
group = sysfs_add_dir(parent, "group_name", 0777 | SYSFS_PLUGGED, NULL);
sysfs_add_file(group, "file0", 0777, file0_ops, file0_data);
sysfs_add_file(group, "file1", 0777, file1_ops, file1_data);
...
sysfs_add_file(group, "fileN", 0777, fileN_ops, fileN_data);
rc = sysfs_check_batch_error(group);
if (rc) {
sysfs_remove(group);
return rc;
}
sysfs_unplug(group);
return 0;
<<-- end of code -->>

The above will create a subdirectory "group_name" which contains N
files and show them atomically to userland or remove them without
letting userland notice if any failure happens. This will simplify
sysfs users quite a bit (not only for groups, other stuff too).

This patchset contains the following 8 patches.

0001-sysfs-notify-file-on-deactivation.patch
0002-sysfs-add-name-formatting-support-for-symlinks.patch
0003-sysfs-chain-symlinks-to-their-targets.patch
0004-sysfs-implement-symlink-auto-removal.patch
0005-sysfs-implement-symlink-auto-rename.patch
0006-sysfs-implement-plugged-creation-of-sysfs-nodes.patch
0007-sysfs-implement-batch-error-handling.patch
0008-sysfs-add-copyrights.patch

0001 implements notification on file deactivation. 0002-0005
implement symlink name formatting, autoremoving and autorenaming.
0006 implements plugged creation. 0007 batch error handling, and 0008
adds copyright notices on top of all sysfs header and source files.

Thanks.

--
tejun

[1] http://thread.gmane.org/gmane.linux.kernel/582105
[2] http://thread.gmane.org/gmane.linux.kernel/582147



2007-09-20 08:31:57

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 4/8] sysfs: implement symlink auto-removal

When a sysfs_node is removed, automatically remove all symlinks
pointing to it together.

Note that as links created with kobject based sysfs_create_link()
aren't chained on its target, they aren't removed automatically. This
is for backward compatibility.

Signed-off-by: Tejun Heo <[email protected]>
---
fs/sysfs/dir.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index bb9e87e..4a04cb4 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -956,6 +956,12 @@ void __sysfs_remove(struct sysfs_dirent *sd, int recurse)
(sysfs_type(cur) == SYSFS_DIR || cur->s_parent != sd))
continue;

+ /* kill all symlinks pointing to @cur */
+ if (sysfs_type(cur) == SYSFS_DIR)
+ while (cur->s_dir.links)
+ sysfs_remove_one(&acxt, cur->s_dir.links);
+
+ /* kill @cur */
sysfs_remove_one(&acxt, cur);
} while ((cur = next));

--
1.5.0.3


2007-09-20 08:32:34

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 1/8] sysfs: notify file on deactivation

Notify file on deactivation so that the pollers get event when the
polled file dies, which is very easy to implement with sd based
interface.

Signed-off-by: Tejun Heo <[email protected]>
---
fs/sysfs/dir.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index a74ca4a..d50d3ac 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -245,6 +245,10 @@ static void sysfs_deactivate(struct sysfs_dirent *sd)
*/
v = atomic_add_return(SD_DEACTIVATED_BIAS, &sd->s_active);

+ /* file is dying, notify pollers */
+ if (sysfs_type(sd) == SYSFS_FILE)
+ sysfs_notify_file(sd);
+
if (v != SD_DEACTIVATED_BIAS)
wait_for_completion(&wait);

--
1.5.0.3


2007-09-20 08:32:51

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 2/8] sysfs: add name formatting support for symlinks

This patch implements sysfs name formatting. sysfs_add_link() is
passed @name_fmt instead of @fmt. In the format string only
"%[:alnum:]" and "%%" are handled specially. "%0" is substitued with
the name of @target, "%3" with the name of the third ancestor of
@target, "%[aA]" the 10th and "%[zZ]" with 35th. "%%" is substituted
with "%".

This formatting is mainly to make symlink renaming automatic such that
when the target or one of its ancestors gets renamed the symlink can
be renamed together automatically & atomically. It also simplifies
sysfs creation a bit by allowing callers to use static format string
instead of formatting symlink name itself.

@name to kobject based sysfs_create_link() is not interpreted as
format string to keep backward compatibility.

Signed-off-by: Tejun Heo <[email protected]>
---
fs/sysfs/dir.c | 3 +
fs/sysfs/kobject.c | 2 +-
fs/sysfs/symlink.c | 245 ++++++++++++++++++++++++++++++++++++++++++++++--
fs/sysfs/sysfs.h | 9 ++
include/linux/sysfs.h | 6 +-
5 files changed, 250 insertions(+), 15 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index d50d3ac..b042a2e 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -295,6 +295,9 @@ void release_sysfs_dirent(struct sysfs_dirent * sd)
sysfs_put(sd->s_link.target);
if (sd->s_flags & SYSFS_FLAG_NAME_COPIED)
kfree(sd->s_name);
+ if (sd->s_flags & SYSFS_FLAG_LINK_NAME_FMT_COPIED)
+ kfree(sd->s_link.name_fmt);
+
kfree(sd->s_iattr);
sysfs_free_ino(sd->s_ino);
kmem_cache_free(sysfs_dir_cachep, sd);
diff --git a/fs/sysfs/kobject.c b/fs/sysfs/kobject.c
index 16e10de..7ea9186 100644
--- a/fs/sysfs/kobject.c
+++ b/fs/sysfs/kobject.c
@@ -425,7 +425,7 @@ int sysfs_create_link(struct kobject *kobj, struct kobject *target,
if (!target_sd)
return -ENOENT;

- sd = sysfs_add_link(parent_sd, name, SYSFS_COPY_NAME, target_sd);
+ sd = __sysfs_add_link(parent_sd, name, SYSFS_COPY_NAME, target_sd, 0);

sysfs_put(target_sd);

diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 42ecb69..296fef5 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -7,6 +7,7 @@
#include <linux/module.h>
#include <linux/namei.h>
#include <linux/mutex.h>
+#include <linux/ctype.h>

#include "sysfs.h"

@@ -43,16 +44,162 @@ static void fill_object_path(struct sysfs_dirent *sd, char *buffer, int length)
}
}

+size_t sysfs_format_link_name(const char *fmt, struct sysfs_dirent *target,
+ char *buf, struct sysfs_dirent *renamed_sd,
+ struct sysfs_dirent *new_parent,
+ const char *new_name)
+{
+ char *dst = buf;
+ int cnt = 0;
+
+ while (*fmt != '\0') {
+ char ch = *fmt++, next = *fmt;
+ struct sysfs_dirent *tsd;
+ const char *tname;
+ int level;
+ size_t len;
+
+ if (ch != '%' || !isalnum(next)) {
+ if (dst)
+ *dst++ = ch;
+ cnt++;
+ /* %% is % */
+ if (ch== '%' && next == '%')
+ fmt++;
+ continue;
+ }
+
+ /* Seen %[:alnum:]. %0 is the target's name. %z is
+ * the name of the 35th ancestor of the target.
+ */
+ fmt++;
+ if (isdigit(next))
+ level = next - '0';
+ else
+ level = tolower(next) - 'a' + 10;
+
+ /* Work up level times and use its name. It's a bit
+ * complicated due to the renamed node handling.
+ */
+ tsd = target;
+ while (level && tsd->s_parent) {
+ if (tsd == renamed_sd)
+ tsd = new_parent;
+ else
+ tsd = tsd->s_parent;
+ level--;
+ }
+ WARN_ON(level);
+
+ tname = tsd->s_name;
+ if (tsd == renamed_sd)
+ tname = new_name;
+
+ /* got the name, copy it */
+ len = strlen(tname);
+ if (dst) {
+ memcpy(dst, tname, len);
+ dst += len;
+ }
+ cnt += len;
+ }
+
+ if (dst)
+ *dst = '\0';
+ return cnt;
+}
+
/**
- * sysfs_add_link - add a new sysfs symlink
+ * sysfs_link_name - format the name of symlink
+ * @fmt: format string
+ * @target: target sysfs_dirent the symlink points to
+ * @link_name: out parameter for the formatted string
+ * @renamed_sd: renamed sysfs_dirent (can be NULL)
+ * @new_parent: new parent of @renamed_sd (NULL if !@renamed_sd)
+ * @new_name: new name of @renamed_sd (NULL if !@renamed_sd)
+ *
+ * Format the name of the symlink according to @fmt and given
+ * parameters.
+ *
+ * %[:alnum:] has special meaning in the format string. %0 is
+ * substituted with the target's name. %1 is the parent, %2 the
+ * parent's parent, %9 the 9th ancestor, %a or %A the 10th
+ * ancestor and so on upto %z or %Z which is substitued with the
+ * name of the 35th ancestor. %% is substituted with %. All
+ * other sequences are copied verbatim.
+ *
+ * @renamed_sd is used to override the specified sd's parent and
+ * name. If a sd which matches @renamed_sd is in the path
+ * between sysfs_root and @sd, @new_parent is used instead of
+ * sd->s_parent and @new_name is used instead of sd->s_name.
+ * This is used to format new symlink name while preparing to
+ * rename the target or one of the ancestors of it.
+ *
+ * LOCKING:
+ * mutex_lock(sysfs_op_mutex)
+ *
+ * RETURNS:
+ * 1 if success and the buffer *@link_name points to needs to be
+ * freed later. 0 if success and *@link_name points to @fmt
+ * directly. -errno on failure.
+ */
+int sysfs_link_name(const char *fmt, struct sysfs_dirent *target,
+ const char **link_name, struct sysfs_dirent *renamed_sd,
+ struct sysfs_dirent *new_parent, const char *new_name)
+{
+ size_t len;
+ char *buf;
+
+ /* nothing to format? */
+ if (!strchr(fmt, '%')) {
+ *link_name = fmt;
+ return 0;
+ }
+
+ /* determine length and allocate space for the formatted string */
+ len = sysfs_format_link_name(fmt, target, NULL,
+ renamed_sd, new_parent, new_name);
+ buf = kmalloc(len + 1, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ /* format it */
+ sysfs_format_link_name(fmt, target, buf,
+ renamed_sd, new_parent, new_name);
+ *link_name = buf;
+ return 1;
+}
+
+/**
+ * __sysfs_add_link - add a new sysfs symlink
* @parent: sysfs_dirent to add symlink under
- * @name: name of the symlink
+ * @name_fmt: format string for the name of the symlink
* @mode: SYSFS_* flags for the new symlink
* @target: target of the symlink
+ * @format: whether format symlink name or not
*
* Add a new symlink which points to @target under @parent with
* the specified parameters.
*
+ * If @format is not zero, @name_fmt is interpreted as format
+ * string and the symlink name will be formatted accordingly.
+ * Also, the symlink will be chained into the links list of the
+ * @target and will be renamed automatically when the @target is
+ * renamed or moved.
+ *
+ * If @format is zero, @name_fmt is taken as verbatim name of the
+ * symlink and won't be renamed automatically no matter what
+ * happens to the @target.
+ *
+ * SYSFS_COPY_NAME always specifies that the string @name_fmt
+ * points to should be copied whether it's interpreted as format
+ * string or not.
+ *
+ * This is an internal function to be used to implement
+ * sysfs_add_link() and sysfs_create_link(). @format is
+ * necessary to support the original sysfs_create_link()
+ * semantics where the symlink name is specified verbatim.
+ *
* LOCKING:
* Kernel thread context (may sleep).
*
@@ -60,27 +207,103 @@ static void fill_object_path(struct sysfs_dirent *sd, char *buffer, int length)
* Pointer to the new sysfs_dirent on success, ERR_PTR() value on
* error.
*/
-struct sysfs_dirent *sysfs_add_link(struct sysfs_dirent *parent,
- const char *name, mode_t mode,
- struct sysfs_dirent *target)
+struct sysfs_dirent *__sysfs_add_link(struct sysfs_dirent *parent,
+ const char *name_fmt, mode_t mode,
+ struct sysfs_dirent *target, int format)
{
- struct sysfs_dirent *sd;
+ struct sysfs_dirent *sd = NULL;
+ unsigned int flags = 0;
+ const char *name;
+ struct sysfs_addrm_cxt acxt;
+ int rc;
+
+ /* acquire locks early for link name formatting */
+ sysfs_addrm_start(&acxt);

/* Only symlink to directories are allowed. This is an
- * artificial limitation. If ever needed, allowing symlinks
- * to point to other types of sysfs nodes isn't difficult.
+ * artificial limitation mainly to reduce the size of
+ * sysfs_dirent by putting the links head into s_dir union
+ * member. If ever needed, allowing symlinks to point to
+ * other types of sysfs nodes isn't difficult.
*/
+ rc = -EINVAL;
if (sysfs_type(target) != SYSFS_DIR)
- return ERR_PTR(-EINVAL);
+ goto err;
+
+ if (format) {
+ /* SYSFS_COPY_NAME means 'copy the format string' */
+ rc = -ENOMEM;
+ if (mode & SYSFS_COPY_NAME) {
+ name_fmt = kstrdup(name_fmt, GFP_KERNEL);
+ if (!name_fmt)
+ goto err;
+ mode &= ~SYSFS_COPY_NAME;
+ flags |= SYSFS_FLAG_LINK_NAME_FMT_COPIED;
+ }
+
+ /* format name */
+ rc = sysfs_link_name(name_fmt, target, &name, NULL, NULL, NULL);
+ if (rc < 0)
+ goto err;
+ /* set NAME_COPIED if name has been allocated and formatted */
+ if (rc)
+ flags |= SYSFS_FLAG_NAME_COPIED;
+ } else
+ name = name_fmt;

/* allocate & initialize */
+ rc = -ENOMEM;
sd = sysfs_new_dirent(name, mode | S_IRWXUGO, SYSFS_LINK);
if (!sd)
- return ERR_PTR(-ENOMEM);
+ goto err;

+ sd->s_flags |= flags;
+ sd->s_link.name_fmt = name_fmt;
sd->s_link.target = sysfs_get(target);

- return sysfs_insert_one(parent, sd);
+ /* add the new node */
+ rc = sysfs_add_one(&acxt, parent, sd);
+ if (rc) {
+ sysfs_put(sd); /* target is put when sd is released */
+ goto err;
+ }
+
+ sysfs_addrm_finish(&acxt);
+ return sd;
+
+ err:
+ sysfs_addrm_finish(&acxt);
+ if (flags & SYSFS_FLAG_LINK_NAME_FMT_COPIED)
+ kfree(name_fmt);
+ if (flags & SYSFS_FLAG_NAME_COPIED)
+ kfree(name);
+ return ERR_PTR(rc);
+}
+
+/**
+ * sysfs_add_link - add a new sysfs symlink
+ * @parent: sysfs_dirent to add symlink under
+ * @name_fmt: format string for the name of the symlink
+ * @mode: SYSFS_* flags for the new symlink
+ * @target: target of the symlink
+ *
+ * Add a new symlink which points to @target under @parent with
+ * the specified parameters. @name_fmt is always interpreted as
+ * format string for symlink name. See __sysfs_add_link() for
+ * details.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep).
+ *
+ * RETURNS:
+ * Pointer to the new sysfs_dirent on success, ERR_PTR() value on
+ * error.
+ */
+struct sysfs_dirent *sysfs_add_link(struct sysfs_dirent *parent,
+ const char *name, mode_t mode,
+ struct sysfs_dirent *target)
+{
+ return __sysfs_add_link(parent, name, mode, target, 1);
}
EXPORT_SYMBOL_GPL(sysfs_add_link);

diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index 732b292..9167032 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -9,6 +9,7 @@ struct sysfs_elem_dir {

struct sysfs_elem_link {
struct sysfs_dirent *target;
+ const char *name_fmt;
};

struct sysfs_elem_file {
@@ -62,6 +63,7 @@ struct sysfs_dirent {
#define SYSFS_FLAG_MASK ~SYSFS_TYPE_MASK
#define SYSFS_FLAG_REMOVED 0x0200
#define SYSFS_FLAG_NAME_COPIED 0x0400
+#define SYSFS_FLAG_LINK_NAME_FMT_COPIED 0x0800

static inline unsigned int sysfs_type(struct sysfs_dirent *sd)
{
@@ -152,4 +154,11 @@ extern const struct file_operations sysfs_bin_file_operations;
/*
* symlink.c
*/
+struct sysfs_dirent *__sysfs_add_link(struct sysfs_dirent *parent,
+ const char *name, mode_t mode,
+ struct sysfs_dirent *target, int format);
+int sysfs_link_name(const char *fmt, struct sysfs_dirent *target,
+ const char **link_name, struct sysfs_dirent *renamed_sd,
+ struct sysfs_dirent *new_parent, const char *new_name);
+
extern const struct inode_operations sysfs_link_inode_operations;
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index f0279a7..5afe3bd 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -29,7 +29,7 @@ struct vm_area_struct;
* specific flags.
*/
#define SYSFS_DIR_MODE (S_IRWXU | S_IRUGO | S_IXUGO)
-#define SYSFS_COPY_NAME 010000 /* copy passed @name */
+#define SYSFS_COPY_NAME 010000 /* copy passed @name[_fmt] */

/* collection of all flags for verification */
#define SYSFS_MODE_FLAGS SYSFS_COPY_NAME
@@ -61,7 +61,7 @@ struct sysfs_dirent *sysfs_add_bin(struct sysfs_dirent *parent,
const char *name, mode_t mode, size_t size,
const struct sysfs_bin_ops *bops, void *data);
struct sysfs_dirent *sysfs_add_link(struct sysfs_dirent *parent,
- const char *name, mode_t mode,
+ const char *name_fmt, mode_t mode,
struct sysfs_dirent *target);

struct sysfs_dirent *sysfs_find_child(struct sysfs_dirent *parent,
@@ -100,7 +100,7 @@ static inline struct sysfs_dirent *sysfs_add_bin(struct sysfs_dirent *parent,
}

static inline struct sysfs_dirent *sysfs_add_link(struct sysfs_dirent *parent,
- const char *name, mode_t mode,
+ const char *name_fmt, mode_t mode,
struct sysfs_dirent *target)
{
return NULL;
--
1.5.0.3


2007-09-20 08:33:16

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 3/8] sysfs: chain symlinks to their targets

Add sd->s_dir.links and sd->s_link.next and chain symlinks to their
targets. This will be used to implement auto-removal and auto-rename
of symlinks.

Symlinks created with kobject-based sysfs_create_symlink() won't be
chained to keep backward compatibility.

Signed-off-by: Tejun Heo <[email protected]>
---
fs/sysfs/dir.c | 26 ++++++++++++++++++++++++--
fs/sysfs/symlink.c | 3 +++
fs/sysfs/sysfs.h | 4 ++++
3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index b042a2e..bb9e87e 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -495,7 +495,8 @@ static struct inode *sysfs_addrm_get_parent_inode(struct sysfs_addrm_cxt *acxt,
*
* Get @parent and set sd->s_parent to it and increment nlink of
* parent inode if @sd is a directory and link into the children
- * list of the parent.
+ * list of the parent. If LINK_CHAINED flag is set, @sd is also
+ * chained into the parent's s_dir.links list.
*
* This function should be called between calls to
* sysfs_addrm_start() and sysfs_addrm_finish() and should be
@@ -522,6 +523,11 @@ int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *parent,

sysfs_link_sibling(sd);

+ if (sd->s_flags & SYSFS_FLAG_LINK_CHAINED) {
+ sd->s_link.next = sd->s_link.target->s_dir.links;
+ sd->s_link.target->s_dir.links = sd;
+ }
+
if (parent_inode) {
parent_inode->i_ctime = parent_inode->i_mtime = CURRENT_TIME;
if (sysfs_type(sd) == SYSFS_DIR)
@@ -537,7 +543,8 @@ int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *parent,
* @sd: sysfs_dirent to be added
*
* Mark @sd removed and drop nlink of parent inode if @sd is a
- * directory. @sd is unlinked from the children list.
+ * directory. @sd is unlinked from the children list and the
+ * s_dir.links list if applicable.
*
* This function should be called between calls to
* sysfs_addrm_start() and sysfs_addrm_finish() and should be
@@ -554,6 +561,21 @@ void sysfs_remove_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)

parent_inode = sysfs_addrm_get_parent_inode(acxt, sd->s_parent);

+ if (sd->s_flags & SYSFS_FLAG_LINK_CHAINED) {
+ struct sysfs_dirent *target = sd->s_link.target;
+ struct sysfs_dirent **p;
+ int removed = 0;
+
+ for (p = &target->s_dir.links; *p; p = &(*p)->s_link.next) {
+ if (*p == sd) {
+ *p = sd->s_link.next;
+ removed = 1;
+ break;
+ }
+ }
+ BUG_ON(!removed);
+ }
+
sysfs_unlink_sibling(sd);

sd->s_flags |= SYSFS_FLAG_REMOVED;
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 296fef5..53cc8a6 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -231,6 +231,9 @@ struct sysfs_dirent *__sysfs_add_link(struct sysfs_dirent *parent,
goto err;

if (format) {
+ /* chain into the target's links list */
+ flags |= SYSFS_FLAG_LINK_CHAINED;
+
/* SYSFS_COPY_NAME means 'copy the format string' */
rc = -ENOMEM;
if (mode & SYSFS_COPY_NAME) {
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index 9167032..f704279 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -5,10 +5,13 @@ struct sysfs_elem_dir {
void *data;
/* children list starts here and goes through sd->s_sibling */
struct sysfs_dirent *children;
+ /* symlinks list starts here and goes through sd->s_link.next */
+ struct sysfs_dirent *links;
};

struct sysfs_elem_link {
struct sysfs_dirent *target;
+ struct sysfs_dirent *next;
const char *name_fmt;
};

@@ -64,6 +67,7 @@ struct sysfs_dirent {
#define SYSFS_FLAG_REMOVED 0x0200
#define SYSFS_FLAG_NAME_COPIED 0x0400
#define SYSFS_FLAG_LINK_NAME_FMT_COPIED 0x0800
+#define SYSFS_FLAG_LINK_CHAINED 0x1000

static inline unsigned int sysfs_type(struct sysfs_dirent *sd)
{
--
1.5.0.3


2007-09-20 08:33:33

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 6/8] sysfs: implement plugged creation of sysfs nodes

This patch implements plugged creation of sysfs nodes. If
SYSFS_PLUGGED is specified to any of sysfs_add_*() functions, the
node, its children which get added under it and symlinks pointing to
it or its children won't be visible from userland - lookup and
directory listing won't show them, until the plugged node is
unplugged using sysfs_unplug().

This will be used to prevent userland from seeing sysfs hierarchy for
certain entity (device or whatever) appear piece-by-piece. Also,
initialization failure won't be visible on userland. Everything
including inode nlinks are handled properly.

This will be later combined with uevent_suppress.

Signed-off-by: Tejun Heo <[email protected]>
---
fs/sysfs/dir.c | 165 +++++++++++++++++++++++++++++++++++++++++--------
fs/sysfs/inode.c | 3 +-
fs/sysfs/sysfs.h | 2 +
include/linux/sysfs.h | 8 ++-
4 files changed, 149 insertions(+), 29 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index eac8fef..88ec749 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -118,8 +118,15 @@ struct dentry *sysfs_get_dentry(struct sysfs_dirent *sd)
/* look it up */
parent = dentry;
mutex_lock(&parent->d_inode->i_mutex);
+
dentry = lookup_one_len_kern(cur->s_name, parent,
strlen(cur->s_name));
+ /* negative dentry means @sd is plugged, return -ENOENT */
+ if (!dentry->d_inode) {
+ dput(dentry);
+ dentry = ERR_PTR(-ENOENT);
+ }
+
mutex_unlock(&parent->d_inode->i_mutex);
dput(parent);

@@ -322,12 +329,13 @@ static struct dentry_operations sysfs_dentry_ops = {
/**
* sysfs_new_dirent - allocate and initialize sysfs_dirent
* @name: name for the new sysfs_dirent
- * @mode: mask of bits from S_IRWXUGO | SYSFS_COPY_NAME
+ * @mode: mask of bits from S_IRWXUGO | SYSFS_COPY_NAME | SYSFS_PLUGGED
* @type: one of SYSFS_{DIR|FILE|BIN|LINK}
*
* Allocate and initialize a sysfs_dirent with the specified
* parameters. If SYSFS_COPY_NAME is specified in @mode, @name
- * is duplicated.
+ * is duplicated. If SYSFS_PLUGGED is set, the node is
+ * initialized into plugged state.
*
* LOCKING:
* Kernel thread context (may sleep).
@@ -351,6 +359,10 @@ struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type)
flags |= SYSFS_FLAG_NAME_COPIED;
}

+ /* start plugged? */
+ if (mode & SYSFS_PLUGGED)
+ flags |= SYSFS_FLAG_PLUGGED;
+
/* normalize mode */
mode &= S_IALLUGO;

@@ -512,12 +524,14 @@ static struct inode *sysfs_addrm_get_parent_inode(struct sysfs_addrm_cxt *acxt,
int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *parent,
struct sysfs_dirent *sd)
{
- struct inode *parent_inode;
+ struct inode *parent_inode = NULL;

if (sysfs_find_dirent(parent, sd->s_name))
return -EEXIST;

- parent_inode = sysfs_addrm_get_parent_inode(acxt, parent);
+ /* if plugged, don't update parent inode, will be updated on unplug */
+ if (!(sd->s_flags & SYSFS_FLAG_PLUGGED))
+ parent_inode = sysfs_addrm_get_parent_inode(acxt, parent);

sd->s_parent = sysfs_get(parent);

@@ -555,11 +569,13 @@ int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *parent,
*/
void sysfs_remove_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd)
{
- struct inode *parent_inode;
+ struct inode *parent_inode = NULL;

BUG_ON(sd->s_flags & SYSFS_FLAG_REMOVED);

- parent_inode = sysfs_addrm_get_parent_inode(acxt, sd->s_parent);
+ /* if plugged, don't update parent inode, will be updated on unplug */
+ if (!(sd->s_flags & SYSFS_FLAG_PLUGGED))
+ parent_inode = sysfs_addrm_get_parent_inode(acxt, sd->s_parent);

if (sd->s_flags & SYSFS_FLAG_LINK_CHAINED) {
struct sysfs_dirent *target = sd->s_link.target;
@@ -805,7 +821,9 @@ EXPORT_SYMBOL_GPL(sysfs_find_child);
*
* Add a new directory under @parent with the specified
* parameters. If SYSFS_COPY_NAME is set in @mode, @name is
- * copied before being used.
+ * copied before being used. If SYSFS_PLUGGED is set in @mode,
+ * the new directory will start plugged (won't be visible till
+ * unplugged).
*
* LOCKING:
* Kernel thread context (may sleep).
@@ -830,6 +848,84 @@ struct sysfs_dirent *sysfs_add_dir(struct sysfs_dirent *parent,
}
EXPORT_SYMBOL_GPL(sysfs_add_dir);

+/**
+ * sysfs_unplug - unplug a plugged sysfs_dirent
+ * @sd: sysfs_dirent to unplug
+ *
+ * A plugged sysfs_dirent isn't visible to userland. As are all
+ * its children and symlinks point to the sysfs_dirent or its
+ * children. This function unplugs @sd and makes it visible to
+ * userland.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep). Uses sysfs_mutex.
+ */
+void sysfs_unplug(struct sysfs_dirent *sd)
+{
+ struct sysfs_addrm_cxt acxt;
+
+ /* Unplugging performs parent inode update delayed from
+ * add/removal. Also, sysfs_op_mutex grabbed by addrm_start
+ * guarantees renaming to see consistent plugged status.
+ */
+ sysfs_addrm_start(&acxt);
+
+ if (sd->s_flags & SYSFS_FLAG_PLUGGED) {
+ struct inode *parent_inode =
+ sysfs_addrm_get_parent_inode(&acxt, sd->s_parent);
+
+ sd->s_flags &= ~SYSFS_FLAG_PLUGGED;
+
+ if (parent_inode) {
+ parent_inode->i_ctime =
+ parent_inode->i_mtime = CURRENT_TIME;
+
+ if (sysfs_type(sd) == SYSFS_DIR)
+ inc_nlink(parent_inode);
+ }
+ }
+
+ sysfs_addrm_finish(&acxt);
+}
+EXPORT_SYMBOL_GPL(sysfs_unplug);
+
+static int sysfs_plugged(struct sysfs_dirent *sd,
+ struct sysfs_dirent *new_parent)
+{
+ struct sysfs_dirent *cur;
+
+ /* @sd itself is plugged? */
+ if (sd->s_flags & SYSFS_FLAG_PLUGGED)
+ return 1;
+
+ /* Parent override for renaming. This is used to determine
+ * whether @sd will be plugged when moved under @new_parent.
+ */
+ if (!new_parent)
+ new_parent = sd->s_parent;
+
+ /* is one of @sd's acnestors plugged? */
+ for (cur = new_parent; cur; cur = cur->s_parent)
+ if (cur->s_flags & SYSFS_FLAG_PLUGGED)
+ return 1;
+
+ /* A symlink is plugged if its target is plugged. Check
+ * whether the target is plugged unless the symlink is known
+ * to be unplugged.
+ */
+ if (sysfs_type(sd) != SYSFS_LINK ||
+ (sd->s_flags & SYSFS_FLAG_LINK_UNPLUGGED))
+ return 0;
+
+ for (cur = sd->s_link.target; cur; cur = cur->s_parent)
+ if (cur->s_flags & SYSFS_FLAG_PLUGGED)
+ return 1;
+
+ /* if unplugged, mark it to accelerate future plugged test */
+ sd->s_flags |= SYSFS_FLAG_LINK_UNPLUGGED;
+ return 0;
+}
+
static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry,
struct nameidata *nd)
{
@@ -842,8 +938,8 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry,

sd = sysfs_find_dirent(parent_sd, dentry->d_name.name);

- /* no such entry */
- if (!sd)
+ /* no such entry or plugged */
+ if (unlikely(!sd || sysfs_plugged(sd, NULL)))
goto out_unlock;

/* attach dentry and inode */
@@ -1021,6 +1117,10 @@ static int sysfs_readdir(struct file * filp, void * dirent, filldir_t filldir)
const char * name;
int len;

+ /* skip plugged nodes */
+ if (unlikely(sysfs_plugged(pos, NULL)))
+ continue;
+
name = pos->s_name;
len = strlen(name);
filp->f_pos = ino = pos->s_ino;
@@ -1398,33 +1498,44 @@ EXPORT_SYMBOL_GPL(sysfs_rename);
*/
int sysfs_chmod(struct sysfs_dirent *sd, mode_t mode)
{
- struct dentry *dentry;
- struct inode *inode;
- struct iattr newattrs;
- int rc;
+ mode_t new_mode = (mode & S_IALLUGO) | (sd->s_mode & ~S_IALLUGO);
+ struct dentry *dentry = NULL;
+ struct inode *inode = NULL;
+ int rc = 0;

mutex_lock(&sysfs_op_mutex);
- dentry = sysfs_get_dentry(sd);
- mutex_unlock(&sysfs_op_mutex);
- if (IS_ERR(dentry))
- return PTR_ERR(dentry);

- inode = dentry->d_inode;
+ /* If @sd is plugged, dentry and inode aren't there and can't
+ * be looked up. Just update @sd->s_mode.
+ */
+ if (!sysfs_plugged(sd, NULL)) {
+ struct iattr newattrs;

- mutex_lock(&inode->i_mutex);
+ dentry = sysfs_get_dentry(sd);
+ if (IS_ERR(dentry)) {
+ rc = PTR_ERR(dentry);
+ goto out_unlock_op;
+ }
+ inode = dentry->d_inode;

- newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
- newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
- rc = notify_change(dentry, &newattrs);
+ mutex_lock(&inode->i_mutex);

- if (rc == 0) {
- mutex_lock(&sysfs_mutex);
- sd->s_mode = newattrs.ia_mode;
- mutex_unlock(&sysfs_mutex);
+ newattrs.ia_mode = new_mode;
+ newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
+ rc = notify_change(dentry, &newattrs);
+ if (rc)
+ goto out_unlock_inode;
}

- mutex_unlock(&inode->i_mutex);
+ mutex_lock(&sysfs_mutex);
+ sd->s_mode = new_mode;
+ mutex_unlock(&sysfs_mutex);

+ out_unlock_inode:
+ if (inode)
+ mutex_unlock(&inode->i_mutex);
+ out_unlock_op:
+ mutex_unlock(&sysfs_op_mutex);
dput(dentry);
return rc;
}
diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index 0aeea4b..bd61cca 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -128,7 +128,8 @@ static int sysfs_count_nlink(struct sysfs_dirent *sd)
int nr = 0;

for (child = sd->s_dir.children; child; child = child->s_sibling)
- if (sysfs_type(child) == SYSFS_DIR)
+ if (sysfs_type(child) == SYSFS_DIR &&
+ !(child->s_flags & SYSFS_FLAG_PLUGGED))
nr++;

return nr + 2;
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index f704279..51f346d 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -64,10 +64,12 @@ struct sysfs_dirent {
#define SYSFS_LINK 0x0008

#define SYSFS_FLAG_MASK ~SYSFS_TYPE_MASK
+#define SYSFS_FLAG_PLUGGED 0x0100
#define SYSFS_FLAG_REMOVED 0x0200
#define SYSFS_FLAG_NAME_COPIED 0x0400
#define SYSFS_FLAG_LINK_NAME_FMT_COPIED 0x0800
#define SYSFS_FLAG_LINK_CHAINED 0x1000
+#define SYSFS_FLAG_LINK_UNPLUGGED 0x2000

static inline unsigned int sysfs_type(struct sysfs_dirent *sd)
{
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 5afe3bd..5a8c9f1 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -30,9 +30,10 @@ struct vm_area_struct;
*/
#define SYSFS_DIR_MODE (S_IRWXU | S_IRUGO | S_IXUGO)
#define SYSFS_COPY_NAME 010000 /* copy passed @name[_fmt] */
+#define SYSFS_PLUGGED 020000 /* plug the node on creation */

/* collection of all flags for verification */
-#define SYSFS_MODE_FLAGS SYSFS_COPY_NAME
+#define SYSFS_MODE_FLAGS (SYSFS_COPY_NAME | SYSFS_PLUGGED)

struct sysfs_file_ops {
ssize_t (*show)(char *page, void *data, void *parent_data);
@@ -63,6 +64,7 @@ struct sysfs_dirent *sysfs_add_bin(struct sysfs_dirent *parent,
struct sysfs_dirent *sysfs_add_link(struct sysfs_dirent *parent,
const char *name_fmt, mode_t mode,
struct sysfs_dirent *target);
+void sysfs_unplug(struct sysfs_dirent *sd);

struct sysfs_dirent *sysfs_find_child(struct sysfs_dirent *parent,
const char *name);
@@ -106,6 +108,10 @@ static inline struct sysfs_dirent *sysfs_add_link(struct sysfs_dirent *parent,
return NULL;
}

+static inline void sysfs_unplug(struct sysfs_dirent *sd)
+{
+}
+
static inline struct sysfs_dirent *sysfs_find_child(struct sysfs_dirent *parent,
const char *name)
{
--
1.5.0.3


2007-09-20 08:33:52

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 7/8] sysfs: implement batch error handling

Batch error handling is to be used with plugged creation. It
accumulates any error condition which happens under a plugged node and
let the user handle error once right before unplugging. This easily
replaces attribute group creation.

Batch error handling works by the following two mechanisms.

* When an error occurs, tree is walked toward the top and, if there
are plugged nodes which don't have plugged error recorded yet,
sd->s_batch_error is set before returning the error code.

* All interface funtions allow ERR_PTR value to be passed as
sysfs_dirent arguments and return -EBADF if that happens.

* After all the operations are complete, the user calls
sysfs_check_batch_error() on the plugged node (usually the top one)
which returns 0 if there hasn't been any error or error code of the
first error.

* The user unplugs or removes the node accordingly.

Signed-off-by: Tejun Heo <[email protected]>
---
fs/sysfs/bin.c | 8 +++-
fs/sysfs/dir.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++--
fs/sysfs/file.c | 12 ++++-
fs/sysfs/symlink.c | 7 +++
fs/sysfs/sysfs.h | 12 ++++-
include/linux/sysfs.h | 6 ++
6 files changed, 173 insertions(+), 9 deletions(-)

diff --git a/fs/sysfs/bin.c b/fs/sysfs/bin.c
index d6cc7b3..1f705d2 100644
--- a/fs/sysfs/bin.c
+++ b/fs/sysfs/bin.c
@@ -231,10 +231,16 @@ struct sysfs_dirent *sysfs_add_bin(struct sysfs_dirent *parent,
{
struct sysfs_dirent *sd;

+ /* for batch error handling */
+ if (IS_ERR(parent))
+ return ERR_PTR(-EBADF);
+
/* allocate and initialize */
sd = sysfs_new_dirent(name, mode, SYSFS_BIN);
- if (!sd)
+ if (!sd) {
+ sysfs_set_batch_error(parent, -ENOMEM);
return ERR_PTR(-ENOMEM);
+ }

sd->s_bin.ops = bops;
sd->s_bin.size = size;
diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 88ec749..f18da1b 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -305,7 +305,12 @@ void release_sysfs_dirent(struct sysfs_dirent * sd)
if (sd->s_flags & SYSFS_FLAG_LINK_NAME_FMT_COPIED)
kfree(sd->s_link.name_fmt);

- kfree(sd->s_iattr);
+ /* s_batch_error shares room with s_iattr. If plugged, it's
+ * s_batch_error; otherwise, s_iattr.
+ */
+ if (!(sd->s_flags & SYSFS_FLAG_PLUGGED))
+ kfree(sd->s_iattr);
+
sysfs_free_ino(sd->s_ino);
kmem_cache_free(sysfs_dir_cachep, sd);

@@ -722,6 +727,7 @@ struct sysfs_dirent *sysfs_insert_one(struct sysfs_dirent *parent,

if (rc) {
sysfs_put(sd);
+ sysfs_set_batch_error(parent, rc);
return ERR_PTR(rc);
}

@@ -804,6 +810,10 @@ struct sysfs_dirent *sysfs_find_child(struct sysfs_dirent *parent,
{
struct sysfs_dirent *sd;

+ /* for batch error handling */
+ if (IS_ERR(parent))
+ return ERR_PTR(-EBADF);
+
mutex_lock(&sysfs_mutex);
sd = sysfs_find_dirent(parent, name);
mutex_unlock(&sysfs_mutex);
@@ -837,10 +847,16 @@ struct sysfs_dirent *sysfs_add_dir(struct sysfs_dirent *parent,
{
struct sysfs_dirent *sd;

+ /* for plugged error handling */
+ if (IS_ERR(parent))
+ return ERR_PTR(-EBADF);
+
/* allocate and initialize */
sd = sysfs_new_dirent(name, mode, SYSFS_DIR);
- if (!sd)
+ if (!sd) {
+ sysfs_set_batch_error(parent, -ENOMEM);
return ERR_PTR(-ENOMEM);
+ }

sd->s_dir.data = data;

@@ -849,6 +865,71 @@ struct sysfs_dirent *sysfs_add_dir(struct sysfs_dirent *parent,
EXPORT_SYMBOL_GPL(sysfs_add_dir);

/**
+ * sysfs_set_batch_error - record batch error
+ * @sd: sysfs_dirent which failed operation
+ * @error: error value
+ *
+ * If @error is an error value, find all plugging ancestors and
+ * record batch error status if this is the first error in the
+ * subtree. Note that only the first error behind a plugged node
+ * is recorded.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep). Uses sysfs_mutex.
+ */
+void sysfs_set_batch_error(struct sysfs_dirent *sd, int error)
+{
+ if (!IS_ERR_VALUE(error))
+ return;
+
+ mutex_lock(&sysfs_mutex);
+
+ for ( ; sd; sd = sd->s_parent) {
+ if (!(sd->s_flags & SYSFS_FLAG_PLUGGED) || sd->s_batch_error) {
+ BUG_ON(sd->s_batch_error &&
+ !IS_ERR_VALUE(sd->s_batch_error));
+ continue;
+ }
+
+ /* s_iattr must be unused at this point */
+ BUG_ON(sd->s_iattr);
+ sd->s_batch_error = error;
+ }
+
+ mutex_unlock(&sysfs_mutex);
+}
+
+/**
+ * sysfs_check_batch_error - check batch error
+ * @sd: sysfs_dirent to check batch error for
+ *
+ * Check whether any error has occurred on a plugged node @sd and
+ * the nodes behind it.
+ *
+ * LOCKING:
+ * None.
+ *
+ * RETURNS:
+ * 0 if there hasn't been any batch error, -errno if it had one
+ * or more batch errors. -errno is from the first batch error.
+ */
+int sysfs_check_batch_error(struct sysfs_dirent *sd)
+{
+ int error = 0;
+
+ if (IS_ERR(sd))
+ return PTR_ERR(sd);
+
+ if (sd->s_batch_error) {
+ error = sd->s_batch_error;
+ BUG_ON(!IS_ERR_VALUE(error));
+ }
+
+ return error;
+}
+EXPORT_SYMBOL_GPL(sysfs_check_batch_error);
+
+/**
* sysfs_unplug - unplug a plugged sysfs_dirent
* @sd: sysfs_dirent to unplug
*
@@ -858,12 +939,19 @@ EXPORT_SYMBOL_GPL(sysfs_add_dir);
* userland.
*
* LOCKING:
- * Kernel thread context (may sleep). Uses sysfs_mutex.
+ * Kernel thread context (may sleep). Uses sysfs_op_mutex and
+ * sysfs_mutex.
*/
void sysfs_unplug(struct sysfs_dirent *sd)
{
struct sysfs_addrm_cxt acxt;

+ /* This happens when creation of the top node failed but the
+ * user want to ignore it. Do nothing.
+ */
+ if (IS_ERR(sd))
+ return;
+
/* Unplugging performs parent inode update delayed from
* add/removal. Also, sysfs_op_mutex grabbed by addrm_start
* guarantees renaming to see consistent plugged status.
@@ -876,6 +964,12 @@ void sysfs_unplug(struct sysfs_dirent *sd)

sd->s_flags &= ~SYSFS_FLAG_PLUGGED;

+ /* s_batch_error shares room with s_iattr, make sure
+ * it's NULL.
+ */
+ BUG_ON(sd->s_batch_error && !IS_ERR_VALUE(sd->s_batch_error));
+ sd->s_iattr = NULL;
+
if (parent_inode) {
parent_inode->i_ctime =
parent_inode->i_mtime = CURRENT_TIME;
@@ -1036,8 +1130,8 @@ void __sysfs_remove(struct sysfs_dirent *sd, int recurse)
struct sysfs_addrm_cxt acxt;
struct sysfs_dirent *cur, *next;

- /* noop on NULL */
- if (!sd)
+ /* noop on NULL and ERR_PTR, the latter is for batch error handling */
+ if (!sd || IS_ERR(sd))
return;

sysfs_addrm_start(&acxt);
@@ -1223,6 +1317,18 @@ static int sysfs_rcxt_get_dentries(struct sysfs_rename_context *rcxt,
struct dentry *old_dentry, *new_parent_dentry, *new_dentry;
int rc;

+ /* If sd is plugged currently and will stay plugged under the
+ * new parent, dentries are guaranteed to not exist and stay
+ * that way till rename is complete (op mutex released) and
+ * there's nothing to do regarding dentries.
+ *
+ * Changing plugged status by moving isn't allowed and this
+ * function will fail later if the user tries to do that.
+ */
+ if (sysfs_plugged(rent->sd, NULL) &&
+ sysfs_plugged(rent->sd, rent->new_parent))
+ return 0;
+
/* get old dentry */
old_dentry = sysfs_get_dentry(rent->sd);
if (IS_ERR(old_dentry))
@@ -1424,6 +1530,10 @@ int sysfs_rename(struct sysfs_dirent *sd, struct sysfs_dirent *new_parent,
struct sysfs_rcxt_rename_ent *rent;
int error;

+ /* for batch error handling */
+ if (IS_ERR(sd) || IS_ERR(new_parent))
+ return -EBADF;
+
mutex_lock(&sysfs_op_mutex);

if (!new_parent)
@@ -1463,6 +1573,12 @@ int sysfs_rename(struct sysfs_dirent *sd, struct sysfs_dirent *new_parent,
sysfs_link_sibling(rent->sd);
}

+ /* no dentry, no inode - this happens for plugged rename */
+ if (!rent->old_dentry) {
+ WARN_ON(rent->new_dentry);
+ continue;
+ }
+
/* update dcache and inode accordingly */
if (sysfs_type(rent->sd) == SYSFS_DIR) {
drop_nlink(rent->old_dentry->d_parent->d_inode);
@@ -1479,6 +1595,8 @@ int sysfs_rename(struct sysfs_dirent *sd, struct sysfs_dirent *new_parent,
sysfs_post_rename(&rcxt, error);
out:
mutex_unlock(&sysfs_op_mutex);
+ sysfs_set_batch_error(sd, error);
+ sysfs_set_batch_error(new_parent, error);
return error;
}
EXPORT_SYMBOL_GPL(sysfs_rename);
@@ -1498,11 +1616,17 @@ EXPORT_SYMBOL_GPL(sysfs_rename);
*/
int sysfs_chmod(struct sysfs_dirent *sd, mode_t mode)
{
- mode_t new_mode = (mode & S_IALLUGO) | (sd->s_mode & ~S_IALLUGO);
struct dentry *dentry = NULL;
struct inode *inode = NULL;
+ mode_t new_mode;
int rc = 0;

+ /* for batch error handling */
+ if (IS_ERR(sd))
+ return -EBADF;
+
+ new_mode = (mode & S_IALLUGO) | (sd->s_mode & ~S_IALLUGO);
+
mutex_lock(&sysfs_op_mutex);

/* If @sd is plugged, dentry and inode aren't there and can't
@@ -1537,6 +1661,7 @@ int sysfs_chmod(struct sysfs_dirent *sd, mode_t mode)
out_unlock_op:
mutex_unlock(&sysfs_op_mutex);
dput(dentry);
+ sysfs_set_batch_error(sd, rc);
return rc;
}
EXPORT_SYMBOL_GPL(sysfs_chmod);
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 1d93940..3412732 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -498,6 +498,10 @@ void sysfs_notify_file(struct sysfs_dirent *sd)
{
struct sysfs_open_dirent *od;

+ /* for batch error handling */
+ if (IS_ERR(sd))
+ return;
+
BUG_ON(sysfs_type(sd) != SYSFS_FILE);

spin_lock(&sysfs_open_dirent_lock);
@@ -545,10 +549,16 @@ struct sysfs_dirent *sysfs_add_file(struct sysfs_dirent *parent,
{
struct sysfs_dirent *sd;

+ /* for batch error handling */
+ if (IS_ERR(parent))
+ return ERR_PTR(-EBADF);
+
/* allocate and initialize */
sd = sysfs_new_dirent(name, mode, SYSFS_FILE);
- if (!sd)
+ if (!sd) {
+ sysfs_set_batch_error(parent, -ENOMEM);
return ERR_PTR(-ENOMEM);
+ }

sd->s_file.ops = fops;
sd->s_file.data = data;
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 53cc8a6..7e0fd89 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -217,6 +217,10 @@ struct sysfs_dirent *__sysfs_add_link(struct sysfs_dirent *parent,
struct sysfs_addrm_cxt acxt;
int rc;

+ /* for batch error handling */
+ if (IS_ERR(parent) || IS_ERR(target))
+ return ERR_PTR(-EBADF);
+
/* acquire locks early for link name formatting */
sysfs_addrm_start(&acxt);

@@ -280,6 +284,9 @@ struct sysfs_dirent *__sysfs_add_link(struct sysfs_dirent *parent,
kfree(name_fmt);
if (flags & SYSFS_FLAG_NAME_COPIED)
kfree(name);
+
+ sysfs_set_batch_error(parent, rc);
+ sysfs_set_batch_error(target, rc);
return ERR_PTR(rc);
}

diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index 51f346d..ec0b308 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -52,7 +52,15 @@ struct sysfs_dirent {
unsigned int s_flags;
ino_t s_ino;
umode_t s_mode;
- struct iattr *s_iattr;
+
+ /* Make s_iattr and s_batch_error share the room. This is
+ * okay because s_iattr is used only after a node is unplugged
+ * while s_batch_error is used only while a node is plugged.
+ */
+ union {
+ struct iattr *s_iattr;
+ int s_batch_error;
+ };
};

#define SD_DEACTIVATED_BIAS INT_MIN
@@ -120,6 +128,8 @@ struct sysfs_dirent *sysfs_get_dirent(struct sysfs_dirent *parent_sd,
const unsigned char *name);
struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type);

+void sysfs_set_batch_error(struct sysfs_dirent *sd, int error);
+
void __sysfs_remove(struct sysfs_dirent *sd, int recurse);

void release_sysfs_dirent(struct sysfs_dirent *sd);
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 5a8c9f1..a8dfc6b 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -64,6 +64,7 @@ struct sysfs_dirent *sysfs_add_bin(struct sysfs_dirent *parent,
struct sysfs_dirent *sysfs_add_link(struct sysfs_dirent *parent,
const char *name_fmt, mode_t mode,
struct sysfs_dirent *target);
+int sysfs_check_batch_error(struct sysfs_dirent *sd);
void sysfs_unplug(struct sysfs_dirent *sd);

struct sysfs_dirent *sysfs_find_child(struct sysfs_dirent *parent,
@@ -108,6 +109,11 @@ static inline struct sysfs_dirent *sysfs_add_link(struct sysfs_dirent *parent,
return NULL;
}

+static inline int sysfs_check_batch_error(struct sysfs_dirent *sd)
+{
+ return 0;
+}
+
static inline void sysfs_unplug(struct sysfs_dirent *sd)
{
}
--
1.5.0.3


2007-09-20 08:34:21

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 8/8] sysfs: add copyrights

Sysfs has gone through considerable amount of reimplementation. Add
copyrights. Any objections? :-)

Signed-off-by: Tejun Heo <[email protected]>
---
fs/sysfs/bin.c | 8 +++++++-
fs/sysfs/dir.c | 10 +++++++++-
fs/sysfs/file.c | 10 +++++++++-
fs/sysfs/inode.c | 8 ++++++--
fs/sysfs/kobject.c | 2 ++
fs/sysfs/mount.c | 10 +++++++++-
fs/sysfs/symlink.c | 10 +++++++++-
fs/sysfs/sysfs.h | 10 ++++++++++
include/linux/sysfs-kobject.h | 2 ++
include/linux/sysfs.h | 4 ++++
10 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/fs/sysfs/bin.c b/fs/sysfs/bin.c
index 1f705d2..ae6986d 100644
--- a/fs/sysfs/bin.c
+++ b/fs/sysfs/bin.c
@@ -1,9 +1,15 @@
/*
- * bin.c - binary file operations for sysfs.
+ * fs/sysfs/bin.c - sysfs binary file implementation
*
* Copyright (c) 2003 Patrick Mochel
* Copyright (c) 2003 Matthew Wilcox
* Copyright (c) 2004 Silicon Graphics, Inc.
+ * Copyright (c) 2007 SUSE Linux Products GmbH
+ * Copyright (c) 2007 Tejun Heo <[email protected]>
+ *
+ * This file is released under the GPLv2.
+ *
+ * Please see Documentation/filesystems/sysfs.txt for more information.
*/

#undef DEBUG
diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index f18da1b..a8571f4 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -1,5 +1,13 @@
/*
- * dir.c - Operations for sysfs directories.
+ * fs/sysfs/dir.c - sysfs core and dir operation implementation
+ *
+ * Copyright (c) 2001-3 Patrick Mochel
+ * Copyright (c) 2007 SUSE Linux Products GmbH
+ * Copyright (c) 2007 Tejun Heo <[email protected]>
+ *
+ * This file is released under the GPLv2.
+ *
+ * Please see Documentation/filesystems/sysfs.txt for more information.
*/

#undef DEBUG
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 3412732..68ec732 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -1,5 +1,13 @@
/*
- * file.c - operations for regular (text) files.
+ * fs/sysfs/file.c - sysfs regular (text) file implementation
+ *
+ * Copyright (c) 2001-3 Patrick Mochel
+ * Copyright (c) 2007 SUSE Linux Products GmbH
+ * Copyright (c) 2007 Tejun Heo <[email protected]>
+ *
+ * This file is released under the GPLv2.
+ *
+ * Please see Documentation/filesystems/sysfs.txt for more information.
*/

#include <linux/module.h>
diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index bd61cca..6834a11 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -1,7 +1,11 @@
/*
- * inode.c - basic inode and dentry operations.
+ * fs/sysfs/inode.c - basic sysfs inode and dentry operations
*
- * sysfs is Copyright (c) 2001-3 Patrick Mochel
+ * Copyright (c) 2001-3 Patrick Mochel
+ * Copyright (c) 2007 SUSE Linux Products GmbH
+ * Copyright (c) 2007 Tejun Heo <[email protected]>
+ *
+ * This file is released under the GPLv2.
*
* Please see Documentation/filesystems/sysfs.txt for more information.
*/
diff --git a/fs/sysfs/kobject.c b/fs/sysfs/kobject.c
index 7ea9186..c0bc77d 100644
--- a/fs/sysfs/kobject.c
+++ b/fs/sysfs/kobject.c
@@ -3,6 +3,8 @@
*
* Copyright (c) 2001,2002 Patrick Mochel
* Copyright (c) 2004 Silicon Graphics, Inc.
+ * Copyright (c) 2007 SUSE Linux Products GmbH
+ * Copyright (c) 2007 Tejun Heo <[email protected]>
*
* This is compatibility interface which wraps the primary interface
* defined in linux/sysfs.h to remain compatible with the original
diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
index d61eb08..642988f 100644
--- a/fs/sysfs/mount.c
+++ b/fs/sysfs/mount.c
@@ -1,5 +1,13 @@
/*
- * mount.c - operations for initializing and mounting sysfs.
+ * fs/sysfs/symlink.c - operations for initializing and mounting sysfs
+ *
+ * Copyright (c) 2001-3 Patrick Mochel
+ * Copyright (c) 2007 SUSE Linux Products GmbH
+ * Copyright (c) 2007 Tejun Heo <[email protected]>
+ *
+ * This file is released under the GPLv2.
+ *
+ * Please see Documentation/filesystems/sysfs.txt for more information.
*/

#define DEBUG
diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
index 7e0fd89..ed2550c 100644
--- a/fs/sysfs/symlink.c
+++ b/fs/sysfs/symlink.c
@@ -1,5 +1,13 @@
/*
- * symlink.c - operations for sysfs symlinks.
+ * fs/sysfs/symlink.c - sysfs symlink implementation
+ *
+ * Copyright (c) 2001-3 Patrick Mochel
+ * Copyright (c) 2007 SUSE Linux Products GmbH
+ * Copyright (c) 2007 Tejun Heo <[email protected]>
+ *
+ * This file is released under the GPLv2.
+ *
+ * Please see Documentation/filesystems/sysfs.txt for more information.
*/

#include <linux/fs.h>
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index ec0b308..a92a95d 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -1,3 +1,13 @@
+/*
+ * fs/sysfs/sysfs.h - sysfs internal header file
+ *
+ * Copyright (c) 2001-3 Patrick Mochel
+ * Copyright (c) 2007 SUSE Linux Products GmbH
+ * Copyright (c) 2007 Tejun Heo <[email protected]>
+ *
+ * This file is released under the GPLv2.
+ */
+
struct sysfs_open_dirent;

/* type-specific structures for sysfs_dirent->s_* union members */
diff --git a/include/linux/sysfs-kobject.h b/include/linux/sysfs-kobject.h
index 4c821c2..dbbad99 100644
--- a/include/linux/sysfs-kobject.h
+++ b/include/linux/sysfs-kobject.h
@@ -3,6 +3,8 @@
*
* Copyright (c) 2001,2002 Patrick Mochel
* Copyright (c) 2004 Silicon Graphics, Inc.
+ * Copyright (c) 2007 SUSE Linux Products GmbH
+ * Copyright (c) 2007 Tejun Heo <[email protected]>
*
* This is compatibility interface which wraps the primary interface
* defined in linux/sysfs.h to remain compatible with the original
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index a8dfc6b..22cdbf4 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -1,6 +1,10 @@
/*
* sysfs.h - definitions for the device driver filesystem
*
+ * Copyright (c) 2001-3 Patrick Mochel
+ * Copyright (c) 2007 SUSE Linux Products GmbH
+ * Copyright (c) 2007 Tejun Heo <[email protected]>
+ *
* Primary sysfs interface based on sysfs_dirent
*
* If you're using sysfs directly instead of via driver model, please
--
1.5.0.3


2007-09-20 08:34:38

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 5/8] sysfs: implement symlink auto-rename

When a sysfs_node or one of its ancestors is renamed, automatically
rename symlinks pointing to it according to the name format together.

Note that as links created with kobject based sysfs_create_link()
aren't chained on its target, they aren't renamed automatically. This
is for backward compatibility.

Signed-off-by: Tejun Heo <[email protected]>
---
fs/sysfs/dir.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 4a04cb4..eac8fef 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -1196,6 +1196,7 @@ static int sysfs_prep_rename(struct sysfs_rename_context *rcxt,
{
struct sysfs_rcxt_rename_ent *rent;
struct sysfs_rcxt_mutex_ent *ment;
+ struct sysfs_dirent *cur;
int rc;

INIT_LIST_HEAD(&rcxt->mutexes);
@@ -1220,6 +1221,50 @@ static int sysfs_prep_rename(struct sysfs_rename_context *rcxt,
goto err;

/*
+ * prep links pointing to @sd and its children
+ */
+ cur = NULL;
+ while ((cur = sysfs_tree_walk_next(sd, cur))) {
+ struct sysfs_dirent *link;
+
+ if (sysfs_type(cur) != SYSFS_DIR)
+ continue;
+
+ for (link = cur->s_dir.links; link; link = link->s_link.next) {
+ const char *link_new_name;
+ int copied;
+
+ rc = sysfs_link_name(link->s_link.name_fmt,
+ link->s_link.target,
+ &link_new_name,
+ sd, new_parent, new_name);
+ if (rc < 0)
+ goto err;
+ copied = rc;
+
+ if (!strcmp(link->s_name, link_new_name)) {
+ if (copied)
+ kfree(link_new_name);
+ continue;
+ }
+
+ rc = -ENOMEM;
+ rent = sysfs_rcxt_add(rcxt, link, link->s_parent);
+ if (!rent) {
+ mutex_unlock(&sysfs_mutex);
+ goto err;
+ }
+
+ rent->new_name = link_new_name;
+ rent->new_name_copied = copied;
+
+ rc = sysfs_rcxt_get_dentries(rcxt, rent);
+ if (rc)
+ goto err;
+ }
+ }
+
+ /*
* lock all i_mutexes
*/
try_lock:
--
1.5.0.3


2007-09-25 22:50:33

by Greg KH

[permalink] [raw]
Subject: Re: [PATCHSET 4/4] sysfs: implement new features

On Thu, Sep 20, 2007 at 05:31:37PM +0900, Tejun Heo wrote:
> Hello, all.
>
> This is the fourth patchset of four sysfs update patchset series[1]
> and to be applied on top of the third patchset[2].
>
> This patchset implements the following new features.
>
> * Notify pollers on file deactivation.

This looks nice.

> * Name-formatting for symlinks. e.g. symlink pointing to
> /dira/dirb/leaf can be named as "symlink:%1-%0" and it will show up
> as "symlink:dirb-leaf". This only applies when new interface is
> used.

Is this really necessary? It looks like we are adding a "special" type
of parser here that no one uses.

> * Autoremoval of symlinks when target is removed. This only applies
> when new interface is used.

Nice.

> * Autorenaming of symlinks according to the name format string when
> target or one of its ancestors is renamed or moved. This only
> applies when new interface is used.

Nice.

> * Plugged operations. Sysfs users can plug top node and build subtree
> gradually without revealing the process to userland. When subtree
> is fully constructed, the top node can be unplugged and userland
> will see completely built subtree appearing at once. If subtree
> creation fails in the process, the whole subtree can be removed by
> simply removing the top node. There won't be any userland
> noticeable event. This is to be combined with uevent_suppress
> mechanism of driver model.

Hm, but why? Can't we do this today with the attribute groups?

> * Batch error handling. A plugged node accumulates any error
> condition occurring below it and can return the first error when
> asked. Also, all interface functions accepth ERR_PTR() value as
> sysfs_dirent parameter. This means that constructs like the
> following can be used to replace the current group interface.
>
> <<-- code -->>
> group = sysfs_add_dir(parent, "group_name", 0777 | SYSFS_PLUGGED, NULL);
> sysfs_add_file(group, "file0", 0777, file0_ops, file0_data);
> sysfs_add_file(group, "file1", 0777, file1_ops, file1_data);
> ...
> sysfs_add_file(group, "fileN", 0777, fileN_ops, fileN_data);
> rc = sysfs_check_batch_error(group);
> if (rc) {
> sysfs_remove(group);
> return rc;
> }
> sysfs_unplug(group);
> return 0;
> <<-- end of code -->>
>
> The above will create a subdirectory "group_name" which contains N
> files and show them atomically to userland or remove them without
> letting userland notice if any failure happens. This will simplify
> sysfs users quite a bit (not only for groups, other stuff too).

I'm still not sold on why this is needed. It looks like a lot of extra
work for something that we are already handling.

thanks,

greg k-h

2007-09-27 18:18:36

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCHSET 4/4] sysfs: implement new features

Hello, Greg.

Please read the other reply first. We need some consensus there first.

Greg KH wrote:
>> * Notify pollers on file deactivation.
>
> This looks nice.

Cool.

>> * Name-formatting for symlinks. e.g. symlink pointing to
>> /dira/dirb/leaf can be named as "symlink:%1-%0" and it will show up
>> as "symlink:dirb-leaf". This only applies when new interface is
>> used.
>
> Is this really necessary? It looks like we are adding a "special" type
> of parser here that no one uses.

This is a package deal with the autorenaming. More on this later.

>> * Autoremoval of symlinks when target is removed. This only applies
>> when new interface is used.
>
> Nice.
>
>> * Autorenaming of symlinks according to the name format string when
>> target or one of its ancestors is renamed or moved. This only
>> applies when new interface is used.
>
> Nice.

To rename symlink when its target or one of its ancestors is renamed or
moved, sysfs should be able to determine what should be the new name of
the symlink when it points to the new target, right? So, we need
symlink name formatting to do this.

I think the implemented formatting scheme covers all symlinks. Symlink
name formatting will also simplify callers. No need to allocate name,
format and free it. It can simply use constant string.

>> * Plugged operations. Sysfs users can plug top node and build subtree
>> gradually without revealing the process to userland. When subtree
>> is fully constructed, the top node can be unplugged and userland
>> will see completely built subtree appearing at once. If subtree
>> creation fails in the process, the whole subtree can be removed by
>> simply removing the top node. There won't be any userland
>> noticeable event. This is to be combined with uevent_suppress
>> mechanism of driver model.
>
> Hm, but why? Can't we do this today with the attribute groups?

Yes, we can. This is the way to do it with the new interface. More on
this below.

>> * Batch error handling. A plugged node accumulates any error
>> condition occurring below it and can return the first error when
>> asked. Also, all interface functions accepth ERR_PTR() value as
>> sysfs_dirent parameter. This means that constructs like the
>> following can be used to replace the current group interface.
>>
>> <<-- code -->>
>> group = sysfs_add_dir(parent, "group_name", 0777 | SYSFS_PLUGGED, NULL);
>> sysfs_add_file(group, "file0", 0777, file0_ops, file0_data);
>> sysfs_add_file(group, "file1", 0777, file1_ops, file1_data);
>> ...
>> sysfs_add_file(group, "fileN", 0777, fileN_ops, fileN_data);
>> rc = sysfs_check_batch_error(group);
>> if (rc) {
>> sysfs_remove(group);
>> return rc;
>> }
>> sysfs_unplug(group);
>> return 0;
>> <<-- end of code -->>
>>
>> The above will create a subdirectory "group_name" which contains N
>> files and show them atomically to userland or remove them without
>> letting userland notice if any failure happens. This will simplify
>> sysfs users quite a bit (not only for groups, other stuff too).
>
> I'm still not sold on why this is needed. It looks like a lot of extra
> work for something that we are already handling.

Plugged creation + batch error handling is a way to group operations and
commit them atomically under the new interface. The problem of visible
partial creation / removal is mostly solved using
device->uevent_suppress but I think things can be improved for both
userland and kernel.

After uevent is moved into sysfs, uevent generation can be coupled with
grouped commit. sysfs hierarchy and accompanying uevents will appear
atomically to userland and in-kernel users end up with simpler and less
buggy code - single error check at the end of all sysfs operations
instead of piece-by-piece construction coupled with the same amount of
unroll code which is usually buggy. Maybe in (long) time, sysfs access
can be simplified for embedded systems or whatnot.

I don't really see much downside other than the added complexity and as
the added complexity will reduce complexity in the users, I think it's a
good trade off.

Thanks.

--
tejun

2007-09-27 22:39:36

by Kyle Moffett

[permalink] [raw]
Subject: Re: [PATCHSET 4/4] sysfs: implement new features

On Sep 25, 2007, at 18:50:05, Greg KH wrote:
> On Thu, Sep 20, 2007 at 05:31:37PM +0900, Tejun Heo wrote:
>> * Name-formatting for symlinks. e.g. symlink pointing to /dira/
>> dirb/leaf can be named as "symlink:%1-%0" and it will show up as
>> "symlink:dirb-leaf". This only applies when new interface is used.
>
> Is this really necessary? It looks like we are adding a "special"
> type of parser here that no one uses.

IMHO this would be nicer if it could reuse existing sprintf code to
handle all the nice shiny sprintf format specifiers. The only
challenge would be how to dynamically build a varargs list from an
array of component names although perhaps there could be an internal
__csprintf function which took a callback for retrieving arguments.
Also since all of the path components are strings I don't know that
numeric specifiers could be made useful, so perhaps it's not the
greatest idea.

I think the primary importance for this functionality is:

>> * Autorenaming of symlinks according to the name format string
>> when target or one of its ancestors is renamed or moved. This
>> only applies when new interface is used.
>
> Nice.

Cheers,
Kyle Moffett