loop cleanup in fs/namespace.c - repost

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.

Quoted 1 line

The patch given below replaces the goto-loop by a while-based one.

That certainly looks fine. I would also replace the 'return' with 'break', but I guess that's more of a question of personal preference.

Quoted 2 lines

Besides, it removes the export for the same routine, because there are no users for it outside of the core VFS code.

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

Quoted 2 lines

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.

Quoted 3 lines

Quoted 1 line

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.

:)

Quoted 2 lines

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.

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 пишет:

Quoted 10 lines

Quoted 3 lines

Quoted 1 line

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.

:)

Quoted 2 lines

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.

Quoted 5 lines

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.

Quoted 1 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

Quoted 3 lines

- z

Quoted 15 lines · On Thu, 22 Nov 2007 01:49:19 +0300 Dmitri Vorobiev <[email protected]>…

Zach Brown пишет:

Quoted 8 lines

Quoted 3 lines

Quoted 1 line

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.

:)

Quoted 2 lines

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.

Quoted 4 lines

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.

Quoted 10 lines

Quoted 1 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

Quoted 2 lines

- z

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

Signed-off-by Dmitri Vorobiev <[email protected]>

---

fs/namespace.c +2 -4

index 79883fe..b098b63 100644
@@ -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.

fs/namespace.c +2 -4

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