From: Miklos Szeredi <[email protected]>
Clean up mnt->mnt_slave_list being initialized too many times.
Move set_mnt_shared from pnode.h to pnode.c. Change
CLEAR_MNT_SHARED() to clear_mnt_shared() function, and move to
pnode.c.
Signed-off-by: Miklos Szeredi <[email protected]>
---
fs/namespace.c | 2 +-
fs/pnode.c | 23 ++++++++++++++++-------
fs/pnode.h | 9 ++-------
3 files changed, 19 insertions(+), 15 deletions(-)
Index: linux/fs/pnode.c
===================================================================
--- linux.orig/fs/pnode.c 2008-03-13 20:45:15.000000000 +0100
+++ linux/fs/pnode.c 2008-03-13 20:45:49.000000000 +0100
@@ -27,6 +27,17 @@ static inline struct vfsmount *next_slav
return list_entry(p->mnt_slave.next, struct vfsmount, mnt_slave);
}
+void set_mnt_shared(struct vfsmount *mnt)
+{
+ mnt->mnt_flags &= ~MNT_PNODE_MASK;
+ mnt->mnt_flags |= MNT_SHARED;
+}
+
+void clear_mnt_shared(struct vfsmount *mnt)
+{
+ mnt->mnt_flags &= ~MNT_SHARED;
+}
+
static int __peer_group_id(struct vfsmount *mnt)
{
struct vfsmount *m;
@@ -89,20 +100,18 @@ static int do_make_slave(struct vfsmount
list_for_each_entry(slave_mnt, &mnt->mnt_slave_list, mnt_slave)
slave_mnt->mnt_master = master;
list_move(&mnt->mnt_slave, &master->mnt_slave_list);
- list_splice(&mnt->mnt_slave_list, master->mnt_slave_list.prev);
- INIT_LIST_HEAD(&mnt->mnt_slave_list);
+ list_splice_init(&mnt->mnt_slave_list,
+ master->mnt_slave_list.prev);
} else {
- struct list_head *p = &mnt->mnt_slave_list;
- while (!list_empty(p)) {
- slave_mnt = list_first_entry(p,
+ while (!list_empty(&mnt->mnt_slave_list)) {
+ slave_mnt = list_first_entry(&mnt->mnt_slave_list,
struct vfsmount, mnt_slave);
list_del_init(&slave_mnt->mnt_slave);
slave_mnt->mnt_master = NULL;
}
}
mnt->mnt_master = master;
- CLEAR_MNT_SHARED(mnt);
- INIT_LIST_HEAD(&mnt->mnt_slave_list);
+ clear_mnt_shared(mnt);
return 0;
}
Index: linux/fs/namespace.c
===================================================================
--- linux.orig/fs/namespace.c 2008-03-13 20:45:49.000000000 +0100
+++ linux/fs/namespace.c 2008-03-13 20:45:49.000000000 +0100
@@ -537,7 +537,7 @@ static struct vfsmount *clone_mnt(struct
if (flag & CL_SLAVE) {
list_add(&mnt->mnt_slave, &old->mnt_slave_list);
mnt->mnt_master = old;
- CLEAR_MNT_SHARED(mnt);
+ clear_mnt_shared(mnt);
} else if (!(flag & CL_PRIVATE)) {
if ((flag & CL_PROPAGATION) || IS_MNT_SHARED(old))
list_add(&mnt->mnt_share, &old->mnt_share);
Index: linux/fs/pnode.h
===================================================================
--- linux.orig/fs/pnode.h 2008-03-13 20:45:15.000000000 +0100
+++ linux/fs/pnode.h 2008-03-13 20:45:49.000000000 +0100
@@ -14,7 +14,6 @@
#define IS_MNT_SHARED(mnt) (mnt->mnt_flags & MNT_SHARED)
#define IS_MNT_SLAVE(mnt) (mnt->mnt_master)
#define IS_MNT_NEW(mnt) (!mnt->mnt_ns)
-#define CLEAR_MNT_SHARED(mnt) (mnt->mnt_flags &= ~MNT_SHARED)
#define IS_MNT_UNBINDABLE(mnt) (mnt->mnt_flags & MNT_UNBINDABLE)
#define CL_EXPIRE 0x01
@@ -24,12 +23,8 @@
#define CL_PROPAGATION 0x10
#define CL_PRIVATE 0x20
-static inline void set_mnt_shared(struct vfsmount *mnt)
-{
- mnt->mnt_flags &= ~MNT_PNODE_MASK;
- mnt->mnt_flags |= MNT_SHARED;
-}
-
+void set_mnt_shared(struct vfsmount *);
+void clear_mnt_shared(struct vfsmount *);
void change_mnt_propagation(struct vfsmount *, int);
int propagate_mnt(struct vfsmount *, struct dentry *, struct vfsmount *,
struct list_head *);
--
On Thu, Mar 13, 2008 at 10:26:43PM +0100, Miklos Szeredi wrote:
> Move set_mnt_shared from pnode.h to pnode.c. Change
> CLEAR_MNT_SHARED() to clear_mnt_shared() function, and move to
> pnode.c.
I don't see why these two are a cleanup, actually.
> @@ -89,20 +100,18 @@ static int do_make_slave(struct vfsmount
> list_for_each_entry(slave_mnt, &mnt->mnt_slave_list, mnt_slave)
> slave_mnt->mnt_master = master;
> list_move(&mnt->mnt_slave, &master->mnt_slave_list);
> - list_splice(&mnt->mnt_slave_list, master->mnt_slave_list.prev);
> - INIT_LIST_HEAD(&mnt->mnt_slave_list);
> + list_splice_init(&mnt->mnt_slave_list,
> + master->mnt_slave_list.prev);
Umm... OK.
> } else {
> - struct list_head *p = &mnt->mnt_slave_list;
> - while (!list_empty(p)) {
> - slave_mnt = list_first_entry(p,
> + while (!list_empty(&mnt->mnt_slave_list)) {
> + slave_mnt = list_first_entry(&mnt->mnt_slave_list,
How is that better?
> - INIT_LIST_HEAD(&mnt->mnt_slave_list);
Fine by me.
> On Thu, Mar 13, 2008 at 10:26:43PM +0100, Miklos Szeredi wrote:
> > Move set_mnt_shared from pnode.h to pnode.c. Change
> > CLEAR_MNT_SHARED() to clear_mnt_shared() function, and move to
> > pnode.c.
>
> I don't see why these two are a cleanup, actually.
People don't like my cleanups :( sniff...
> > @@ -89,20 +100,18 @@ static int do_make_slave(struct vfsmount
> > list_for_each_entry(slave_mnt, &mnt->mnt_slave_list, mnt_slave)
> > slave_mnt->mnt_master = master;
> > list_move(&mnt->mnt_slave, &master->mnt_slave_list);
> > - list_splice(&mnt->mnt_slave_list, master->mnt_slave_list.prev);
> > - INIT_LIST_HEAD(&mnt->mnt_slave_list);
> > + list_splice_init(&mnt->mnt_slave_list,
> > + master->mnt_slave_list.prev);
>
> Umm... OK.
Yeah, not much of an improvement.
> > } else {
> > - struct list_head *p = &mnt->mnt_slave_list;
> > - while (!list_empty(p)) {
> > - slave_mnt = list_first_entry(p,
> > + while (!list_empty(&mnt->mnt_slave_list)) {
> > + slave_mnt = list_first_entry(&mnt->mnt_slave_list,
>
> How is that better?
I find it easier to read. Renaming "p" to "slaves" would also have
been better. Single letter variables are reserved for iterators in my
mind...
Miklos