[PATCH -mm 0/3] configfs: Miscellaneous cleanups

[PATCH -mm 0/3] configfs: Miscellaneous cleanups

[Satyam Sharma]

From: Satyam Sharma <ssatyam@cse.iitk.ac.in>

[0/3] configfs: Miscellaneous cleanups

Simple cleanups for configfs (plus DLM and OCFS2, wherever applicable).
This is diffed against 2.6.22-rc6-mm1.

[1/3] configfs+dlm: Separate out __CONFIGFS_ATTR into configfs.h
[2/3] configfs+dlm+ocfs2: Convert subsystem semaphore to mutex
[3/3] configfs+dlm: Rename config_group_find_obj and state semantics clearly

Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>

Satyam

[PATCH -mm 1/3] configfs+dlm: Separate out __CONFIGFS_ATTR into configfs.h

[Satyam Sharma]

From: Satyam Sharma <ssatyam@cse.iitk.ac.in>

[1/3] configfs+dlm: Separate out __CONFIGFS_ATTR into configfs.h

fs/dlm/config.c contains a useful generic macro called __CONFIGFS_ATTR
that is similar to sysfs' __ATTR macro that makes defining attributes
easy for any user of configfs. Separate it out into configfs.h so that
other users (forthcoming in dynamic netconsole patchset) can use it too.

Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
Cc: Joel Becker <joel.becker@oracle.com>
Cc: David Teigland <teigland@redhat.com>

---

 fs/dlm/config.c          |    8 --------
 include/linux/configfs.h |   16 ++++++++++++++++
 2 files changed, 16 insertions(+), 8 deletions(-)

---

diff -ruNp a/fs/dlm/config.c b/fs/dlm/config.c
--- a/fs/dlm/config.c	2007-06-28 17:32:57.000000000 +0530
+++ b/fs/dlm/config.c	2007-07-03 16:56:35.000000000 +0530
@@ -133,14 +133,6 @@ static ssize_t cluster_set(struct cluste
 	return len;
 }
 
-#define __CONFIGFS_ATTR(_name,_mode,_read,_write) {                           \
-	.attr   = { .ca_name = __stringify(_name),                            \
-		    .ca_mode = _mode,                                         \
-		    .ca_owner = THIS_MODULE },                                \
-	.show   = _read,                                                      \
-	.store  = _write,                                                     \
-}
-
 #define CLUSTER_ATTR(name, check_zero)                                        \
 static ssize_t name##_write(struct cluster *cl, const char *buf, size_t len)  \
 {                                                                             \
diff -ruNp a/include/linux/configfs.h b/include/linux/configfs.h
--- a/include/linux/configfs.h	2007-06-28 17:33:19.000000000 +0530
+++ b/include/linux/configfs.h	2007-07-03 16:56:35.000000000 +0530
@@ -130,6 +130,22 @@ struct configfs_attribute {
 	mode_t			ca_mode;
 };
 
+/*
+ * Users often need to create attribute structures for their configurable
+ * attributes, containing a configfs_attribute member and function pointers
+ * for the show() and store() operations on that attribute. They can use
+ * this macro (similar to sysfs' __ATTR) to make defining attributes easier.
+ */
+#define __CONFIGFS_ATTR(_name, _mode, _show, _store)			\
+{									\
+	.attr	= {							\
+			.ca_name = __stringify(_name),			\
+			.ca_mode = _mode,				\
+			.ca_owner = THIS_MODULE,			\
+	},								\
+	.show	= _show,						\
+	.store	= _store,						\
+}
 
 /*
  * If allow_link() exists, the item can symlink(2) out to other

[PATCH -mm 2/3] configfs+dlm+ocfs2: Convert subsystem semaphore to mutex

[Satyam Sharma]

From: Satyam Sharma <ssatyam@cse.iitk.ac.in>

[2/3] configfs+dlm+ocfs2: Convert subsystem semaphore to mutex

Convert the su_sem member of struct configfs_subsystem to a struct mutex,
as that's what it is. Also convert all the users and update
Documentation/configfs.txt and Documentation/configfs_example.c accordingly.

[ This patch touches three different trees (configfs, dlm and ocfs2)
and I thought of splitting this up, but I guess there's little point in
splitting into smaller patches that won't even build without each other. ]

Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
Cc: Joel Becker <joel.becker@oracle.com>
Cc: Mark Fasheh <mark.fasheh@oracle.com>
Cc: David Teigland <teigland@redhat.com>

---

 Documentation/filesystems/configfs/configfs.txt       |   20 ++++++++----------
 Documentation/filesystems/configfs/configfs_example.c |    2 -
 fs/configfs/dir.c                                     |   16 +++++++-------
 fs/dlm/config.c                                       |   10 ++++-----
 fs/ocfs2/cluster/nodemanager.c                        |    2 -
 include/linux/configfs.h                              |    5 +---
 6 files changed, 26 insertions(+), 29 deletions(-)

---

diff -ruNp a/Documentation/filesystems/configfs/configfs_example.c b/Documentation/filesystems/configfs/configfs_example.c
--- a/Documentation/filesystems/configfs/configfs_example.c	2007-04-26 08:38:32.000000000 +0530
+++ b/Documentation/filesystems/configfs/configfs_example.c	2007-07-03 18:38:28.000000000 +0530
@@ -453,7 +453,7 @@ static int __init configfs_example_init(
 		subsys = example_subsys[i];
 
 		config_group_init(&subsys->su_group);
-		init_MUTEX(&subsys->su_sem);
+		mutex_init(&subsys->su_mtx);
 		ret = configfs_register_subsystem(subsys);
 		if (ret) {
 			printk(KERN_ERR "Error %d while registering subsystem %s\n",
diff -ruNp a/Documentation/filesystems/configfs/configfs.txt b/Documentation/filesystems/configfs/configfs.txt
--- a/Documentation/filesystems/configfs/configfs.txt	2007-06-28 17:31:57.000000000 +0530
+++ b/Documentation/filesystems/configfs/configfs.txt	2007-07-03 18:38:28.000000000 +0530
@@ -292,18 +292,18 @@ tells configfs to make the subsystem app
 
 	struct configfs_subsystem {
 		struct config_group	su_group;
-		struct semaphore	su_sem;
+		struct mutex		su_mtx;
 	};
 
 	int configfs_register_subsystem(struct configfs_subsystem *subsys);
 	void configfs_unregister_subsystem(struct configfs_subsystem *subsys);
 
-	A subsystem consists of a toplevel config_group and a semaphore.
+	A subsystem consists of a toplevel config_group and a mutex.
 The group is where child config_items are created.  For a subsystem,
 this group is usually defined statically.  Before calling
 configfs_register_subsystem(), the subsystem must have initialized the
 group via the usual group _init() functions, and it must also have
-initialized the semaphore.
+initialized the mutex.
 	When the register call returns, the subsystem is live, and it
 will be visible via configfs.  At that point, mkdir(2) can be called and
 the subsystem must be ready for it.
@@ -315,7 +315,7 @@ subsystem/group and the simple_child ite
 shows a trivial object displaying and storing an attribute, and a simple
 group creating and destroying these children.
 
-[Hierarchy Navigation and the Subsystem Semaphore]
+[Hierarchy Navigation and the Subsystem Mutex]
 
 There is an extra bonus that configfs provides.  The config_groups and
 config_items are arranged in a hierarchy due to the fact that they
@@ -326,19 +326,17 @@ and config_item->ci_parent structure mem
 
 A subsystem can navigate the cg_children list and the ci_parent pointer
 to see the tree created by the subsystem.  This can race with configfs'
-management of the hierarchy, so configfs uses the subsystem semaphore to
+management of the hierarchy, so configfs uses the subsystem mutex to
 protect modifications.  Whenever a subsystem wants to navigate the
-hierarchy, it must do so under the protection of the subsystem
-semaphore.
+hierarchy, it must do so under the protection of the subsystem mutex.
 
-A subsystem will be prevented from acquiring the semaphore while a newly
+A subsystem will be prevented from acquiring the mutex while a newly
 allocated item has not been linked into this hierarchy.   Similarly, it
-will not be able to acquire the semaphore while a dropping item has not
+will not be able to acquire the mutex while a dropping item has not
 yet been unlinked.  This means that an item's ci_parent pointer will
 never be NULL while the item is in configfs, and that an item will only
 be in its parent's cg_children list for the same duration.  This allows
-a subsystem to trust ci_parent and cg_children while they hold the
-semaphore.
+a subsystem to trust ci_parent and cg_children while they hold the mutex.
 
 [Item Aggregation Via symlink(2)]
 
diff -ruNp a/fs/configfs/dir.c b/fs/configfs/dir.c
--- a/fs/configfs/dir.c	2007-06-28 17:32:57.000000000 +0530
+++ b/fs/configfs/dir.c	2007-07-03 18:44:07.000000000 +0530
@@ -566,7 +566,7 @@ static int populate_groups(struct config
 
 /*
  * All of link_obj/unlink_obj/link_group/unlink_group require that
- * subsys->su_sem is held.
+ * subsys->su_mtx is held.
  */
 
 static void unlink_obj(struct config_item *item)
@@ -1042,7 +1042,7 @@ static int configfs_mkdir(struct inode *
 
 	snprintf(name, dentry->d_name.len + 1, "%s", dentry->d_name.name);
 
-	down(&subsys->su_sem);
+	mutex_lock(&subsys->su_mtx);
 	group = NULL;
 	item = NULL;
 	if (type->ct_group_ops->make_group) {
@@ -1056,7 +1056,7 @@ static int configfs_mkdir(struct inode *
 		if (item)
 			link_obj(parent_item, item);
 	}
-	up(&subsys->su_sem);
+	mutex_unlock(&subsys->su_mtx);
 
 	kfree(name);
 	if (!item) {
@@ -1100,7 +1100,7 @@ static int configfs_mkdir(struct inode *
 out_unlink:
 	if (ret) {
 		/* Tear down everything we built up */
-		down(&subsys->su_sem);
+		mutex_lock(&subsys->su_mtx);
 
 		client_disconnect_notify(parent_item, item);
 		if (group)
@@ -1109,7 +1109,7 @@ out_unlink:
 			unlink_obj(item);
 		client_drop_item(parent_item, item);
 
-		up(&subsys->su_sem);
+		mutex_unlock(&subsys->su_mtx);
 
 		if (module_got)
 			module_put(owner);
@@ -1179,19 +1179,19 @@ static int configfs_rmdir(struct inode *
 	if (sd->s_type & CONFIGFS_USET_DIR) {
 		configfs_detach_group(item);
 
-		down(&subsys->su_sem);
+		mutex_lock(&subsys->su_mtx);
 		client_disconnect_notify(parent_item, item);
 		unlink_group(to_config_group(item));
 	} else {
 		configfs_detach_item(item);
 
-		down(&subsys->su_sem);
+		mutex_lock(&subsys->su_mtx);
 		client_disconnect_notify(parent_item, item);
 		unlink_obj(item);
 	}
 
 	client_drop_item(parent_item, item);
-	up(&subsys->su_sem);
+	mutex_unlock(&subsys->su_mtx);
 
 	/* Drop our reference from above */
 	config_item_put(item);
diff -ruNp a/fs/dlm/config.c b/fs/dlm/config.c
--- a/fs/dlm/config.c	2007-07-03 18:35:03.000000000 +0530
+++ b/fs/dlm/config.c	2007-07-03 18:38:28.000000000 +0530
@@ -607,7 +607,7 @@ static struct clusters clusters_root = {
 int dlm_config_init(void)
 {
 	config_group_init(&clusters_root.subsys.su_group);
-	init_MUTEX(&clusters_root.subsys.su_sem);
+	mutex_init(&clusters_root.subsys.su_mtx);
 	return configfs_register_subsystem(&clusters_root.subsys);
 }
 
@@ -751,9 +751,9 @@ static struct space *get_space(char *nam
 	if (!space_list)
 		return NULL;
 
-	down(&space_list->cg_subsys->su_sem);
+	mutex_lock(&space_list->cg_subsys->su_mtx);
 	i = config_group_find_obj(space_list, name);
-	up(&space_list->cg_subsys->su_sem);
+	mutex_unlock(&space_list->cg_subsys->su_mtx);
 
 	return to_space(i);
 }
@@ -772,7 +772,7 @@ static struct comm *get_comm(int nodeid,
 	if (!comm_list)
 		return NULL;
 
-	down(&clusters_root.subsys.su_sem);
+	mutex_lock(&clusters_root.subsys.su_mtx);
 
 	list_for_each_entry(i, &comm_list->cg_children, ci_entry) {
 		cm = to_comm(i);
@@ -792,7 +792,7 @@ static struct comm *get_comm(int nodeid,
 			break;
 		}
 	}
-	up(&clusters_root.subsys.su_sem);
+	mutex_unlock(&clusters_root.subsys.su_mtx);
 
 	if (!found)
 		cm = NULL;
diff -ruNp a/fs/ocfs2/cluster/nodemanager.c b/fs/ocfs2/cluster/nodemanager.c
--- a/fs/ocfs2/cluster/nodemanager.c	2007-06-28 17:33:07.000000000 +0530
+++ b/fs/ocfs2/cluster/nodemanager.c	2007-07-03 18:38:28.000000000 +0530
@@ -974,7 +974,7 @@ static int __init init_o2nm(void)
 		goto out_sysctl;
 
 	config_group_init(&o2nm_cluster_group.cs_subsys.su_group);
-	init_MUTEX(&o2nm_cluster_group.cs_subsys.su_sem);
+	mutex_init(&o2nm_cluster_group.cs_subsys.su_mtx);
 	ret = configfs_register_subsystem(&o2nm_cluster_group.cs_subsys);
 	if (ret) {
 		printk(KERN_ERR "nodemanager: Registration returned %d\n", ret);
diff -ruNp a/include/linux/configfs.h b/include/linux/configfs.h
--- a/include/linux/configfs.h	2007-07-03 18:35:03.000000000 +0530
+++ b/include/linux/configfs.h	2007-07-03 18:38:28.000000000 +0530
@@ -40,9 +40,8 @@
 #include <linux/types.h>
 #include <linux/list.h>
 #include <linux/kref.h>
-
+#include <linux/mutex.h>
 #include <asm/atomic.h>
-#include <asm/semaphore.h>
 
 #define CONFIGFS_ITEM_NAME_LEN	20
 
@@ -178,7 +177,7 @@ struct configfs_group_operations {
 
 struct configfs_subsystem {
 	struct config_group	su_group;
-	struct semaphore	su_sem;
+	struct mutex		su_mtx;
 };
 
 static inline struct configfs_subsystem *to_configfs_subsystem(struct config_group *group)

[PATCH -mm 3/3] configfs+dlm: Rename config_group_find_obj and state semantics clearly

[Satyam Sharma]

From: Satyam Sharma <ssatyam@cse.iitk.ac.in>

[3/3] configfs+dlm: Rename config_group_find_obj and state semantics clearly

[ Ok, this might sound like a gratuitous patch, but I believe there are
good reasons for it. ]

Configfs being based upon sysfs code, config_group_find_obj() is probably
so named because of the similar kset_find_obj() in sysfs. However,
"kobject"s in sysfs become "config_item"s in configfs, so let's call it
config_group_find_item() instead, for sake of uniformity, and make
corresponding change in the users of this function.

BTW a crucial difference between kset_find_obj and config_group_find_item
is in locking expectations. kset_find_obj does its locking by itself, but
config_group_find_item expects the *caller* to do the locking. The reason
for this: kset's have their own locks, config_group's don't but instead
rely on the subsystem mutex. And, subsystem needn't necessarily be around
when config_group_find_item() is called.

So let's state these locking semantics explicitly, and rectify the comment,
otherwise bugs could continue to occur in future, as they did in the past
(refer commit d82b8191e238 in gfs2-2.6-fixes.git).

[ I also took the opportunity to fix some bad whitespace and
double-empty lines. ]

Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
Cc: Joel Becker <joel.becker@oracle.com>
Cc: David Teigland <teigland@redhat.com>

---

 fs/configfs/item.c       |   18 ++++++++----------
 fs/dlm/config.c          |    2 +-
 include/linux/configfs.h |    8 ++------
 3 files changed, 11 insertions(+), 17 deletions(-)

---

diff -ruNp a/fs/configfs/item.c b/fs/configfs/item.c
--- a/fs/configfs/item.c	2007-06-28 17:32:57.000000000 +0530
+++ b/fs/configfs/item.c	2007-07-03 19:42:00.000000000 +0530
@@ -183,27 +183,25 @@ void config_group_init(struct config_gro
 	INIT_LIST_HEAD(&group->cg_children);
 }
 
-
 /**
- *	config_group_find_obj - search for item in group.
+ *	config_group_find_item - search for item in group.
  *	@group:	group we're looking in.
  *	@name:	item's name.
  *
- *	Lock group via @group->cg_subsys, and iterate over @group->cg_list,
- *	looking for a matching config_item. If matching item is found
- *	take a reference and return the item.
+ *	Iterate over @group->cg_list, looking for a matching config_item.
+ *	If matching item is found take a reference and return the item.
+ *	Caller must have locked group via @group->cg_subsys->su_mtx.
  */
-struct config_item *config_group_find_obj(struct config_group *group,
-					  const char * name)
+struct config_item *config_group_find_item(struct config_group *group,
+					   const char *name)
 {
 	struct list_head * entry;
 	struct config_item * ret = NULL;
 
-        /* XXX LOCKING! */
 	list_for_each(entry,&group->cg_children) {
 		struct config_item * item = to_item(entry);
 		if (config_item_name(item) &&
-                    !strcmp(config_item_name(item), name)) {
+		    !strcmp(config_item_name(item), name)) {
 			ret = config_item_get(item);
 			break;
 		}
@@ -215,4 +213,4 @@ EXPORT_SYMBOL(config_item_init);
 EXPORT_SYMBOL(config_group_init);
 EXPORT_SYMBOL(config_item_get);
 EXPORT_SYMBOL(config_item_put);
-EXPORT_SYMBOL(config_group_find_obj);
+EXPORT_SYMBOL(config_group_find_item);
diff -ruNp a/fs/dlm/config.c b/fs/dlm/config.c
--- a/fs/dlm/config.c	2007-07-03 18:52:25.000000000 +0530
+++ b/fs/dlm/config.c	2007-07-03 19:43:57.000000000 +0530
@@ -752,7 +752,7 @@ static struct space *get_space(char *nam
 		return NULL;
 
 	mutex_lock(&space_list->cg_subsys->su_mtx);
-	i = config_group_find_obj(space_list, name);
+	i = config_group_find_item(space_list, name);
 	mutex_unlock(&space_list->cg_subsys->su_mtx);
 
 	return to_space(i);
diff -ruNp a/include/linux/configfs.h b/include/linux/configfs.h
--- a/include/linux/configfs.h	2007-07-03 18:52:25.000000000 +0530
+++ b/include/linux/configfs.h	2007-07-03 19:46:56.000000000 +0530
@@ -85,12 +85,10 @@ struct config_item_type {
 	struct configfs_attribute		**ct_attrs;
 };
 
-
 /**
  *	group - a group of config_items of a specific type, belonging
  *	to a specific subsystem.
  */
-
 struct config_group {
 	struct config_item		cg_item;
 	struct list_head		cg_children;
@@ -98,13 +96,11 @@ struct config_group {
 	struct config_group		**default_groups;
 };
 
-
 extern void config_group_init(struct config_group *group);
 extern void config_group_init_type_name(struct config_group *group,
 					const char *name,
 					struct config_item_type *type);
 
-
 static inline struct config_group *to_config_group(struct config_item *item)
 {
 	return item ? container_of(item,struct config_group,cg_item) : NULL;
@@ -120,8 +116,8 @@ static inline void config_group_put(stru
 	config_item_put(&group->cg_item);
 }
 
-extern struct config_item *config_group_find_obj(struct config_group *, const char *);
-
+extern struct config_item *config_group_find_item(struct config_group *,
+						  const char *);
 
 struct configfs_attribute {
 	const char		*ca_name;

Re: [PATCH -mm 0/3] configfs: Miscellaneous cleanups

[Satyam Sharma]

Argh, my sendpatchset script got David's address wrong. I've made up
for that by sending the patchset separately to him -- didn't want to
spam the entire list.

On Wed, 4 Jul 2007, Satyam Sharma wrote:
> From: Satyam Sharma <ssatyam@cse.iitk.ac.in>
> 
> [0/3] configfs: Miscellaneous cleanups
> 
> Simple cleanups for configfs (plus DLM and OCFS2, wherever applicable).
> This is diffed against 2.6.22-rc6-mm1.
> 
> [1/3] configfs+dlm: Separate out __CONFIGFS_ATTR into configfs.h
> [2/3] configfs+dlm+ocfs2: Convert subsystem semaphore to mutex
> [3/3] configfs+dlm: Rename config_group_find_obj and state semantics clearly
> 
> Signed-off-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
> 
> Satyam
> 

Re: [PATCH -mm 0/3] configfs: Miscellaneous cleanups

[Joel Becker]

On Wed, Jul 04, 2007 at 04:37:01PM +0530, Satyam Sharma wrote:
> From: Satyam Sharma <ssatyam@cse.iitk.ac.in>
> 
> [0/3] configfs: Miscellaneous cleanups
> 
> Simple cleanups for configfs (plus DLM and OCFS2, wherever applicable).
> This is diffed against 2.6.22-rc6-mm1.

	All three are pretty clean and useful.

> [1/3] configfs+dlm: Separate out __CONFIGFS_ATTR into configfs.h

	I thought I had this already.  If someone is using it (and
fs/dlm obviously is), we should provide it for the general case.

> [2/3] configfs+dlm+ocfs2: Convert subsystem semaphore to mutex

	I've just been lazy about this one.  Thank you.

> [3/3] configfs+dlm: Rename config_group_find_obj and state semantics clearly

	Makes sense as well.

	Any dissent from other subsystems?  If not, expect these to
show up in the ocfs2 tree soon-ish (I'm currently on vacation :-)

Joel

-- 

"Well-timed silence hath more eloquence than speech."  
         - Martin Fraquhar Tupper

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

Re: [PATCH -mm 0/3] configfs: Miscellaneous cleanups

[Satyam Sharma]

Hi Joel,

On Wed, 4 Jul 2007, Joel Becker wrote:
> On Wed, Jul 04, 2007 at 04:37:01PM +0530, Satyam Sharma wrote:
> > From: Satyam Sharma <ssatyam@cse.iitk.ac.in>
> > 
> > [0/3] configfs: Miscellaneous cleanups
> > 
> > Simple cleanups for configfs (plus DLM and OCFS2, wherever applicable).
> > This is diffed against 2.6.22-rc6-mm1.
> 
> 	All three are pretty clean and useful.
> 
> > [1/3] configfs+dlm: Separate out __CONFIGFS_ATTR into configfs.h
> 
> 	I thought I had this already.  If someone is using it (and
> fs/dlm obviously is), we should provide it for the general case.
> 
> > [2/3] configfs+dlm+ocfs2: Convert subsystem semaphore to mutex
> 
> 	I've just been lazy about this one.  Thank you.
> 
> > [3/3] configfs+dlm: Rename config_group_find_obj and state semantics clearly
> 
> 	Makes sense as well.
> 
> 	Any dissent from other subsystems?  If not, expect these to
> show up in the ocfs2 tree soon-ish (I'm currently on vacation :-)

Steven Whitehouse liked these too, and has given them his:

Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>

as well.

It was a wrong-email-id mistake by me in the initial post, and then
I made another mistake by doing the resend to only David and Steven
without copying in LKML and others (I thought I was doing something
good by not spamming the list / others). Sorry about the whole
episode -- I guess I'll live and learn :-)

Thanks,
Satyam

Re: [PATCH -mm 0/3] configfs: Miscellaneous cleanups

[David Teigland]

On Wed, Jul 04, 2007 at 11:30:08PM -0700, Joel Becker wrote:
> On Wed, Jul 04, 2007 at 04:37:01PM +0530, Satyam Sharma wrote:
> > From: Satyam Sharma <ssatyam@cse.iitk.ac.in>
> > 
> > [0/3] configfs: Miscellaneous cleanups
> > 
> > Simple cleanups for configfs (plus DLM and OCFS2, wherever applicable).
> > This is diffed against 2.6.22-rc6-mm1.
> 
> 	All three are pretty clean and useful.
> 
> > [1/3] configfs+dlm: Separate out __CONFIGFS_ATTR into configfs.h
> 
> 	I thought I had this already.  If someone is using it (and
> fs/dlm obviously is), we should provide it for the general case.
> 
> > [2/3] configfs+dlm+ocfs2: Convert subsystem semaphore to mutex
> 
> 	I've just been lazy about this one.  Thank you.
> 
> > [3/3] configfs+dlm: Rename config_group_find_obj and state semantics clearly
> 
> 	Makes sense as well.
> 
> 	Any dissent from other subsystems?  If not, expect these to
> show up in the ocfs2 tree soon-ish (I'm currently on vacation :-)

All look good from here.

Dave

Re: [PATCH -mm 0/3] configfs: Miscellaneous cleanups

[Joel Becker]

On Wed, Jul 04, 2007 at 11:30:08PM -0700, Joel Becker wrote:
> > [1/3] configfs+dlm: Separate out __CONFIGFS_ATTR into configfs.h
> 
> 	I thought I had this already.  If someone is using it (and
> fs/dlm obviously is), we should provide it for the general case.

	Ok, this is in my tree.

> > [2/3] configfs+dlm+ocfs2: Convert subsystem semaphore to mutex
> > [3/3] configfs+dlm: Rename config_group_find_obj and state semantics clearly

	These break against mainline.  I'm going to merge them against
mainline (shouldn't be hard), but I wonder if they are going to break
-mm, specifically conflicting against the DLM patches.  Andrew, do you
have a preference how we handle this?

Joel

-- 

"Always give your best, never get discouraged, never be petty; always
 remember, others may hate you.  Those who hate you don't win unless
 you hate them.  And then you destroy yourself."
	- Richard M. Nixon

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[PATCH] configfs: Convert subsystem semaphore to mutex

[Joel Becker]

[Rewritten so that it applies to mainline and names the filed su_mutex.
 This will appear in my tree.]

Convert the su_sem member of struct configfs_subsystem to a struct
mutex, as that's what it is. Also convert all the users and update
Documentation/configfs.txt and Documentation/configfs_example.c
accordingly.

Inspired-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 Documentation/filesystems/configfs/configfs.txt    |   18 +++++++++---------
 .../filesystems/configfs/configfs_example.c        |    2 +-
 fs/configfs/dir.c                                  |   16 ++++++++--------
 fs/dlm/config.c                                    |    6 +++---
 fs/ocfs2/cluster/nodemanager.c                     |    2 +-
 include/linux/configfs.h                           |    4 ++--
 6 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/Documentation/filesystems/configfs/configfs.txt b/Documentation/filesystems/configfs/configfs.txt
index b34cdb5..21f038e 100644
--- a/Documentation/filesystems/configfs/configfs.txt
+++ b/Documentation/filesystems/configfs/configfs.txt
@@ -280,18 +280,18 @@ tells configfs to make the subsystem app
 
 	struct configfs_subsystem {
 		struct config_group	su_group;
-		struct semaphore	su_sem;
+		struct mutex		su_mutex;
 	};
 
 	int configfs_register_subsystem(struct configfs_subsystem *subsys);
 	void configfs_unregister_subsystem(struct configfs_subsystem *subsys);
 
-	A subsystem consists of a toplevel config_group and a semaphore.
+	A subsystem consists of a toplevel config_group and a mutex.
 The group is where child config_items are created.  For a subsystem,
 this group is usually defined statically.  Before calling
 configfs_register_subsystem(), the subsystem must have initialized the
 group via the usual group _init() functions, and it must also have
-initialized the semaphore.
+initialized the mutex.
 	When the register call returns, the subsystem is live, and it
 will be visible via configfs.  At that point, mkdir(2) can be called and
 the subsystem must be ready for it.
@@ -303,7 +303,7 @@ subsystem/group and the simple_child ite
 shows a trivial object displaying and storing an attribute, and a simple
 group creating and destroying these children.
 
-[Hierarchy Navigation and the Subsystem Semaphore]
+[Hierarchy Navigation and the Subsystem Mutex]
 
 There is an extra bonus that configfs provides.  The config_groups and
 config_items are arranged in a hierarchy due to the fact that they
@@ -314,19 +314,19 @@ and config_item->ci_parent structure mem
 
 A subsystem can navigate the cg_children list and the ci_parent pointer
 to see the tree created by the subsystem.  This can race with configfs'
-management of the hierarchy, so configfs uses the subsystem semaphore to
+management of the hierarchy, so configfs uses the subsystem mutex to
 protect modifications.  Whenever a subsystem wants to navigate the
 hierarchy, it must do so under the protection of the subsystem
-semaphore.
+mutex.
 
-A subsystem will be prevented from acquiring the semaphore while a newly
+A subsystem will be prevented from acquiring the mutex while a newly
 allocated item has not been linked into this hierarchy.   Similarly, it
-will not be able to acquire the semaphore while a dropping item has not
+will not be able to acquire the mutex while a dropping item has not
 yet been unlinked.  This means that an item's ci_parent pointer will
 never be NULL while the item is in configfs, and that an item will only
 be in its parent's cg_children list for the same duration.  This allows
 a subsystem to trust ci_parent and cg_children while they hold the
-semaphore.
+mutex.
 
 [Item Aggregation Via symlink(2)]
 
diff --git a/Documentation/filesystems/configfs/configfs_example.c b/Documentation/filesystems/configfs/configfs_example.c
index 2d6a14a..e56d492 100644
--- a/Documentation/filesystems/configfs/configfs_example.c
+++ b/Documentation/filesystems/configfs/configfs_example.c
@@ -453,7 +453,7 @@ static int __init configfs_example_init(
 		subsys = example_subsys[i];
 
 		config_group_init(&subsys->su_group);
-		init_MUTEX(&subsys->su_sem);
+		mutex_init(&subsys->su_mutex);
 		ret = configfs_register_subsystem(subsys);
 		if (ret) {
 			printk(KERN_ERR "Error %d while registering subsystem %s\n",
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 5e6e37e..d3b1dbb 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -562,7 +562,7 @@ static int populate_groups(struct config
 
 /*
  * All of link_obj/unlink_obj/link_group/unlink_group require that
- * subsys->su_sem is held.
+ * subsys->su_mutex is held.
  */
 
 static void unlink_obj(struct config_item *item)
@@ -783,7 +783,7 @@ static int configfs_mkdir(struct inode *
 
 	snprintf(name, dentry->d_name.len + 1, "%s", dentry->d_name.name);
 
-	down(&subsys->su_sem);
+	mutex_lock(&subsys->su_mutex);
 	group = NULL;
 	item = NULL;
 	if (type->ct_group_ops->make_group) {
@@ -797,7 +797,7 @@ static int configfs_mkdir(struct inode *
 		if (item)
 			link_obj(parent_item, item);
 	}
-	up(&subsys->su_sem);
+	mutex_unlock(&subsys->su_mutex);
 
 	kfree(name);
 	if (!item) {
@@ -841,13 +841,13 @@ static int configfs_mkdir(struct inode *
 out_unlink:
 	if (ret) {
 		/* Tear down everything we built up */
-		down(&subsys->su_sem);
+		mutex_lock(&subsys->su_mutex);
 		if (group)
 			unlink_group(group);
 		else
 			unlink_obj(item);
 		client_drop_item(parent_item, item);
-		up(&subsys->su_sem);
+		mutex_unlock(&subsys->su_mutex);
 
 		if (module_got)
 			module_put(owner);
@@ -910,17 +910,17 @@ static int configfs_rmdir(struct inode *
 	if (sd->s_type & CONFIGFS_USET_DIR) {
 		configfs_detach_group(item);
 
-		down(&subsys->su_sem);
+		mutex_lock(&subsys->su_mutex);
 		unlink_group(to_config_group(item));
 	} else {
 		configfs_detach_item(item);
 
-		down(&subsys->su_sem);
+		mutex_lock(&subsys->su_mutex);
 		unlink_obj(item);
 	}
 
 	client_drop_item(parent_item, item);
-	up(&subsys->su_sem);
+	mutex_unlock(&subsys->su_mutex);
 
 	/* Drop our reference from above */
 	config_item_put(item);
diff --git a/fs/dlm/config.c b/fs/dlm/config.c
index 822abdc..bf93f97 100644
--- a/fs/dlm/config.c
+++ b/fs/dlm/config.c
@@ -609,7 +609,7 @@ static struct clusters clusters_root = {
 int dlm_config_init(void)
 {
 	config_group_init(&clusters_root.subsys.su_group);
-	init_MUTEX(&clusters_root.subsys.su_sem);
+	mutex_init(&clusters_root.subsys.su_mutex);
 	return configfs_register_subsystem(&clusters_root.subsys);
 }
 
@@ -767,7 +767,7 @@ static struct comm *get_comm(int nodeid,
 	if (!comm_list)
 		return NULL;
 
-	down(&clusters_root.subsys.su_sem);
+	mutex_lock(&clusters_root.subsys.su_mutex);
 
 	list_for_each_entry(i, &comm_list->cg_children, ci_entry) {
 		cm = to_comm(i);
@@ -785,7 +785,7 @@ static struct comm *get_comm(int nodeid,
 			break;
 		}
 	}
-	up(&clusters_root.subsys.su_sem);
+	mutex_unlock(&clusters_root.subsys.su_mutex);
 
 	if (found)
 		config_item_get(i);
diff --git a/fs/ocfs2/cluster/nodemanager.c b/fs/ocfs2/cluster/nodemanager.c
index 9f5ad0f..48b77d1 100644
--- a/fs/ocfs2/cluster/nodemanager.c
+++ b/fs/ocfs2/cluster/nodemanager.c
@@ -934,7 +934,7 @@ static int __init init_o2nm(void)
 		goto out_sysctl;
 
 	config_group_init(&o2nm_cluster_group.cs_subsys.su_group);
-	init_MUTEX(&o2nm_cluster_group.cs_subsys.su_sem);
+	mutex_init(&o2nm_cluster_group.cs_subsys.su_mutex);
 	ret = configfs_register_subsystem(&o2nm_cluster_group.cs_subsys);
 	if (ret) {
 		printk(KERN_ERR "nodemanager: Registration returned %d\n", ret);
diff --git a/include/linux/configfs.h b/include/linux/configfs.h
index fef6f3d..798c0c1 100644
--- a/include/linux/configfs.h
+++ b/include/linux/configfs.h
@@ -40,9 +40,9 @@ #ifdef __KERNEL__
 #include <linux/types.h>
 #include <linux/list.h>
 #include <linux/kref.h>
+#include <linux/mutex.h>
 
 #include <asm/atomic.h>
-#include <asm/semaphore.h>
 
 #define CONFIGFS_ITEM_NAME_LEN	20
 
@@ -162,7 +162,7 @@ struct configfs_group_operations {
 
 struct configfs_subsystem {
 	struct config_group	su_group;
-	struct semaphore	su_sem;
+	struct mutex		su_mutex;
 };
 
 static inline struct configfs_subsystem *to_configfs_subsystem(struct config_group *group)
-- 
1.4.2.3

-- 

"The first requisite of a good citizen in this republic of ours
 is that he shall be able and willing to pull his weight."
	- Theodore Roosevelt

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

Re: [PATCH] configfs: Convert subsystem semaphore to mutex

[Satyam Sharma]

Hi Joel,

On Fri, 6 Jul 2007, Joel Becker wrote:
> [Rewritten so that it applies to mainline and names the filed su_mutex.
>  This will appear in my tree.]

The one I'd sent was diffed against -mm, which includes gfs2-2.6-fixes.git
but apparently Linus' tree (I just checked -rc7, at least) doesn't seem
to have merged in gfs2-2.6-fixes.git as yet. There are references to
down() / up() / su_sem / config_group_find_obj() in there (which was why
I had diffed against -mm to cover all the uses of these), so those will
have to be updated accordingly as well now ...

> Convert the su_sem member of struct configfs_subsystem to a struct
> mutex, as that's what it is. Also convert all the users and update
> Documentation/configfs.txt and Documentation/configfs_example.c
> accordingly.
> 
> Inspired-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>
> Signed-off-by: Joel Becker <joel.becker@oracle.com>

With that caveat, Acked-by: Satyam Sharma <ssatyam@cse.iitk.ac.in>

Satyam