Hi list,
Apparently, the following two C language constructs
while (condition) {

	// DO HARD WORK

}
and
repeat:

	if (condition) {

		// DO HARD WORK

		goto repeat;

	}
are equivalent, but the latter one looks quite cumbersome and is not idiomatic. However, the mntput_no_expire() routine in fs/namespace.c is implemented using the goto-based approach.
The patch given below replaces the goto-loop by a while-based one. Besides, it removes the export for the same routine, because there are no users for it outside of the core VFS code.
Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>

---

diff --git a/fs/namespace.c b/fs/namespace.c

index 0608388..38b4b4a 100644

--- a/fs/namespace.c

+++ b/fs/namespace.c

@@ -278,8 +278,7 @@ 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);

@@ -290,12 +289,9 @@ repeat:

 		spin_unlock(&vfsmount_lock);

 		acct_auto_close_mnt(mnt);

 		security_sb_umount_close(mnt);

-		goto repeat;

 	}

 }
-EXPORT_SYMBOL(mntput_no_expire);

-

 void mnt_pin(struct vfsmount *mnt)

 {

 	spin_lock(&vfsmount_lock);

-

That certainly looks fine.  I would also replace the 'return' with
This doesn't look fine.  Did you test this?
mntput_no_expire() is called from mntput() which is an inline function

in mount.h.  So lots of callers of mntput() in modules will end up

trying to call mntput_no_expire() from modules.
$ nm fs/fuse/fuse.ko | grep mntput_no_expire

                 U mntput_no_expire
- z

-

Oops, my fault. Of course, I tested the patch, but kernel modules are disabled in my test setup, so I missed the error. Thanks for pointing that out, Zach.
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.
Signed-off-by Dmitri Vorobiev <dmitri.vorobiev@gmail.com>

---

diff --git a/fs/namespace.c b/fs/namespace.c

index 0608388..410e766 100644

--- a/fs/namespace.c

+++ b/fs/namespace.c

@@ -278,19 +278,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;

 	}

 }
-

It certainly looks OK to me now, for whatever that's worth.
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.
- z

-

Zach Brown

On Thu, 22 Nov 2007 01:49:19 +0300

[loop-cleanup-fs-namespace-mm.diff  text/x-patch (742B)]

Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>

---

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;

 	}

 }
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).

-

Andrew Morton