Re: [PATCH] [2.6.24-rc3-mm1] loop cleanup in fs/namespace.c - repost

[Al Viro]

On Wed, Nov 21, 2007 at 03:24:33PM -0800, Andrew Morton wrote:
 -repeat:
 - if (atomic_dec_and_lock(mnt-mnt_count, vfsmount_lock)) {
 + while (atomic_dec_and_lock(mnt-mnt_count, vfsmount_lock)) {
   if (likely(!mnt-mnt_pinned)) {
   spin_unlock(vfsmount_lock);
   __mntput(mnt);
 - return;
 + break;
   }
   atomic_add(mnt-mnt_pinned + 1, mnt-mnt_count);
   mnt-mnt_pinned = 0;
   spin_unlock(vfsmount_lock);
   acct_auto_close_mnt(mnt);
   security_sb_umount_close(mnt);
 - goto repeat;
   }
  }
  
 This patch has no changelog which I can use.

BTW, I have a problem with that one.  The change is misleading; FWIW,
I'm very tempted to turn that into tail recursion, with

if (!atomic_dec_and_lock(mnt-mnt_count, vfsmount_lock))
return;

in the very beginning (i.e. invert the test and pull the body to top level).
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [2.6.24-rc3-mm1] loop cleanup in fs/namespace.c - repost

[Dmitri Vorobiev]

Zach Brown пишет:
 This doesn't look fine.  Did you test this?
 Oops, my fault. Of course, I tested the patch, but kernel modules are
 disabled in my test setup, so I missed the error.
 
 :)
 
 Enclosed to this message is a new patch, which replaces the goto-loop by
 the while-based one, but leaves the EXPORT_SYMBOL macro intact.
 
 It certainly looks OK to me now, for whatever that's worth. 

Zach, thank you for the code review and suggestions.

 
 You probably want to wait 'till the next merge window to get it in,
 though.  It's just a cleanup and so shouldn't go in this late in the -rc
 line.
 
 Maybe Andrew will be willing to queue it until that time in -mm.

I am enclosing the patch against current -mm tree and adding Andrew to the Cc: 
list.

Thanks,

Dmitri

 
 - z
 

Signed-off-by: Dmitri Vorobiev [EMAIL PROTECTED]
---
diff --git a/fs/namespace.c b/fs/namespace.c
index 79883fe..b098b63 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -606,19 +606,17 @@ static inline void __mntput(struct vfsmo
 
 void mntput_no_expire(struct vfsmount *mnt)
 {
-repeat:
-	if (atomic_dec_and_lock(mnt-mnt_count, vfsmount_lock)) {
+	while (atomic_dec_and_lock(mnt-mnt_count, vfsmount_lock)) {
 		if (likely(!mnt-mnt_pinned)) {
 			spin_unlock(vfsmount_lock);
 			__mntput(mnt);
-			return;
+			break;
 		}
 		atomic_add(mnt-mnt_pinned + 1, mnt-mnt_count);
 		mnt-mnt_pinned = 0;
 		spin_unlock(vfsmount_lock);
 		acct_auto_close_mnt(mnt);
 		security_sb_umount_close(mnt);
-		goto repeat;
 	}
 }