2008-03-13 21:28:08

by Miklos Szeredi

Subject: [patch 2/6] vfs: pnode cleanup

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 *);

--

2008-03-19 21:20:55

by Al Viro

Subject: Re: [patch 2/6] vfs: pnode cleanup

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.

2008-03-20 00:24:29

by Miklos Szeredi

Subject: Re: [patch 2/6] vfs: pnode cleanup

> 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