[PATCH] fs: namespace: Simplify function commit_tree()

[PATCH] fs: namespace: Simplify function commit_tree()

[Jiang Liu]

Simplify function commit_tree() in fs/namespace.c, it also improves code
readability.

Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
 fs/namespace.c |   11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index cd1e9681a0cf..c2b6f6f676ec 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -846,18 +846,11 @@ static void attach_shadowed(struct mount *mnt,
 static void commit_tree(struct mount *mnt, struct mount *shadows)
 {
 	struct mount *parent = mnt->mnt_parent;
-	struct mount *m;
-	LIST_HEAD(head);
 	struct mnt_namespace *n = parent->mnt_ns;
 
 	BUG_ON(parent == mnt);
-
-	list_add_tail(&head, &mnt->mnt_list);
-	list_for_each_entry(m, &head, mnt_list)
-		m->mnt_ns = n;
-
-	list_splice(&head, n->list.prev);
-
+	mnt->mnt_ns = n;
+	list_add_tail(&mnt->mnt_list, &n->list);
 	attach_shadowed(mnt, parent, shadows);
 	touch_mnt_namespace(n);
 }
-- 
1.7.10.4

Re: [PATCH] fs: namespace: Simplify function commit_tree()

[Al Viro]

On Tue, Jan 13, 2015 at 01:08:06PM +0800, Jiang Liu wrote:
> Simplify function commit_tree() in fs/namespace.c, it also improves code
> readability.

Huh?

>  static void commit_tree(struct mount *mnt, struct mount *shadows)
>  {
>  	struct mount *parent = mnt->mnt_parent;
> -	struct mount *m;
> -	LIST_HEAD(head);
>  	struct mnt_namespace *n = parent->mnt_ns;
>  
>  	BUG_ON(parent == mnt);
> -
> -	list_add_tail(&head, &mnt->mnt_list);
> -	list_for_each_entry(m, &head, mnt_list)
> -		m->mnt_ns = n;
> -
> -	list_splice(&head, n->list.prev);
> -
> +	mnt->mnt_ns = n;
> +	list_add_tail(&mnt->mnt_list, &n->list);

And who will set ->mnt_ns on other mounts reachable via ->mnt_list from mnt?
Not to mention putting them on n->list...

What you seem to have missed is that list_add_tail(&head, &mnt->mnt_list)
is not the same thing as list_add_tail(&mnt->mnt_list, &head).  We are
not ripping mnt->mnt_list from whatever list it was on and insert it into
a previously empty one in head.  We take an unanchored cyclic list (all
elements being some_mnt->mnt_list) and anchor it at head, then go through
all elements of the original list and move them all at once into the
end of n->list.

Re: [PATCH] fs: namespace: Simplify function commit_tree()

[Jiang Liu]

On 2015/1/13 13:28, Al Viro wrote:
> On Tue, Jan 13, 2015 at 01:08:06PM +0800, Jiang Liu wrote:
>> Simplify function commit_tree() in fs/namespace.c, it also improves code
>> readability.
> 
> Huh?
> 
>>  static void commit_tree(struct mount *mnt, struct mount *shadows)
>>  {
>>  	struct mount *parent = mnt->mnt_parent;
>> -	struct mount *m;
>> -	LIST_HEAD(head);
>>  	struct mnt_namespace *n = parent->mnt_ns;
>>  
>>  	BUG_ON(parent == mnt);
>> -
>> -	list_add_tail(&head, &mnt->mnt_list);
>> -	list_for_each_entry(m, &head, mnt_list)
>> -		m->mnt_ns = n;
>> -
>> -	list_splice(&head, n->list.prev);
>> -
>> +	mnt->mnt_ns = n;
>> +	list_add_tail(&mnt->mnt_list, &n->list);
> 
> And who will set ->mnt_ns on other mounts reachable via ->mnt_list from mnt?
> Not to mention putting them on n->list...
> 
> What you seem to have missed is that list_add_tail(&head, &mnt->mnt_list)
> is not the same thing as list_add_tail(&mnt->mnt_list, &head).  We are
> not ripping mnt->mnt_list from whatever list it was on and insert it into
> a previously empty one in head.  We take an unanchored cyclic list (all
> elements being some_mnt->mnt_list) and anchor it at head, then go through
> all elements of the original list and move them all at once into the
> end of n->list.
Hi Al,
	Sorry for the noise, didn't realized that mnt->mnt_list anchors
a whole list:(
Regards!
Gerry