| Message ID | 558126FA.8050608@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 17, 2015 at 03:51:22PM +0800, Kinglong Mee wrote: > v5, new patch I don't know this code at all. I'll try to give it a proper review. But could you help me by explaining in some detail what this is doing and why you're sure it's correct? --b. > > Signed-off-by: Kinglong Mee <kinglongmee@gmail.com> > --- > fs/namespace.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/fs/namespace.c b/fs/namespace.c > index 1b9e111..3f08a48 100644 > --- a/fs/namespace.c > +++ b/fs/namespace.c > @@ -1049,8 +1049,6 @@ static void cleanup_mnt(struct mount *mnt) > * so mnt_get_writers() below is safe. > */ > WARN_ON(mnt_get_writers(mnt)); > - if (unlikely(mnt->mnt_pins.first)) > - mnt_pin_kill(mnt); > fsnotify_vfsmount_delete(&mnt->mnt); > dput(mnt->mnt.mnt_root); > deactivate_super(mnt->mnt.mnt_sb); > @@ -1078,6 +1076,7 @@ static DECLARE_DELAYED_WORK(delayed_mntput_work, delayed_mntput); > > static void mntput_no_expire(struct mount *mnt) > { > +put_again: > rcu_read_lock(); > mnt_add_count(mnt, -1); > if (likely(mnt->mnt_ns)) { /* shouldn't be the last one */ > @@ -1090,6 +1089,13 @@ static void mntput_no_expire(struct mount *mnt) > unlock_mount_hash(); > return; > } > + if (unlikely(mnt->mnt_pins.first)) { > + mnt_add_count(mnt, 1); > + rcu_read_unlock(); > + unlock_mount_hash(); > + mnt_pin_kill(mnt); > + goto put_again; > + } > if (unlikely(mnt->mnt.mnt_flags & MNT_DOOMED)) { > rcu_read_unlock(); > unlock_mount_hash(); > -- > 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
On Wed, Jun 17, 2015 at 03:51:22PM +0800, Kinglong Mee wrote: > +++ b/fs/namespace.c > @@ -1049,8 +1049,6 @@ static void cleanup_mnt(struct mount *mnt) > * so mnt_get_writers() below is safe. > */ > WARN_ON(mnt_get_writers(mnt)); > - if (unlikely(mnt->mnt_pins.first)) > - mnt_pin_kill(mnt); > fsnotify_vfsmount_delete(&mnt->mnt); > dput(mnt->mnt.mnt_root); > deactivate_super(mnt->mnt.mnt_sb); > @@ -1078,6 +1076,7 @@ static DECLARE_DELAYED_WORK(delayed_mntput_work, delayed_mntput); > > static void mntput_no_expire(struct mount *mnt) > { > +put_again: > rcu_read_lock(); > mnt_add_count(mnt, -1); > if (likely(mnt->mnt_ns)) { /* shouldn't be the last one */ > @@ -1090,6 +1089,13 @@ static void mntput_no_expire(struct mount *mnt) > unlock_mount_hash(); > return; > } > + if (unlikely(mnt->mnt_pins.first)) { > + mnt_add_count(mnt, 1); > + rcu_read_unlock(); > + unlock_mount_hash(); > + mnt_pin_kill(mnt); > + goto put_again; > + } > if (unlikely(mnt->mnt.mnt_flags & MNT_DOOMED)) { > rcu_read_unlock(); > unlock_mount_hash(); This is absolutely wrong. For one thing, you are running those suckers on fairly deep stack now, which is a bloody bad idea for a lot reasons - final mntput() can come with a lot of stack space consumed. For another, I'm really not convinced that what you are doing won't bugger the ordering to hell and back - not without a detailed analysis I don't see in these patches. If you want to be able to grab references by those suckers, this is a very wrong way to go. Look at legitimize_mnt() for better approach, and yes, you need to be able to cope with "too late, it's already doomed". Or you'll trade those -EBUSY for race with umount(2) returning before the fs shutdown is complete. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
On 6/20/2015 4:29 AM, J. Bruce Fields wrote: > On Wed, Jun 17, 2015 at 03:51:22PM +0800, Kinglong Mee wrote: >> v5, new patch > > I don't know this code at all. I'll try to give it a proper review. > But could you help me by explaining in some detail what this is doing > and why you're sure it's correct? Sorry for my misunderstand of your means in version 4. I have make a new version as, When reference of cahce_head increase(>1), grab a reference of mnt once. and reference decrease to 1 (==1), drop the reference of mnt. So after that, When ref > 1, user cannot umount the filesystem with -EBUSY. when ref ==1, means cache only reference by nfsd cache, no other reference. So user can try umount, 1. before set MNT_UMOUNT (protected by mount_lock), nfsd cache is referenced (ref > 1, legitimize_mntget), umount will fail with -EBUSY. 2. after set MNT_UMOUNT, nfsd cache is referenced (ref == 2), legitimize_mntget will fail, and set cache to CACHE_NEGATIVE, and the reference will be dropped, re-back to 1. So, pin_kill can delete the cache and umount success. 3. when umountting, no reference to nfsd cache, pin_kill can delete the cache and umount success. thanks, Kinglong Mee > > --b. > >> >> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com> >> --- >> fs/namespace.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/fs/namespace.c b/fs/namespace.c >> index 1b9e111..3f08a48 100644 >> --- a/fs/namespace.c >> +++ b/fs/namespace.c >> @@ -1049,8 +1049,6 @@ static void cleanup_mnt(struct mount *mnt) >> * so mnt_get_writers() below is safe. >> */ >> WARN_ON(mnt_get_writers(mnt)); >> - if (unlikely(mnt->mnt_pins.first)) >> - mnt_pin_kill(mnt); >> fsnotify_vfsmount_delete(&mnt->mnt); >> dput(mnt->mnt.mnt_root); >> deactivate_super(mnt->mnt.mnt_sb); >> @@ -1078,6 +1076,7 @@ static DECLARE_DELAYED_WORK(delayed_mntput_work, delayed_mntput); >> >> static void mntput_no_expire(struct mount *mnt) >> { >> +put_again: >> rcu_read_lock(); >> mnt_add_count(mnt, -1); >> if (likely(mnt->mnt_ns)) { /* shouldn't be the last one */ >> @@ -1090,6 +1089,13 @@ static void mntput_no_expire(struct mount *mnt) >> unlock_mount_hash(); >> return; >> } >> + if (unlikely(mnt->mnt_pins.first)) { >> + mnt_add_count(mnt, 1); >> + rcu_read_unlock(); >> + unlock_mount_hash(); >> + mnt_pin_kill(mnt); >> + goto put_again; >> + } >> if (unlikely(mnt->mnt.mnt_flags & MNT_DOOMED)) { >> rcu_read_unlock(); >> unlock_mount_hash(); >> -- >> 2.4.3 > . > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/namespace.c b/fs/namespace.c index 1b9e111..3f08a48 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -1049,8 +1049,6 @@ static void cleanup_mnt(struct mount *mnt) * so mnt_get_writers() below is safe. */ WARN_ON(mnt_get_writers(mnt)); - if (unlikely(mnt->mnt_pins.first)) - mnt_pin_kill(mnt); fsnotify_vfsmount_delete(&mnt->mnt); dput(mnt->mnt.mnt_root); deactivate_super(mnt->mnt.mnt_sb); @@ -1078,6 +1076,7 @@ static DECLARE_DELAYED_WORK(delayed_mntput_work, delayed_mntput); static void mntput_no_expire(struct mount *mnt) { +put_again: rcu_read_lock(); mnt_add_count(mnt, -1); if (likely(mnt->mnt_ns)) { /* shouldn't be the last one */ @@ -1090,6 +1089,13 @@ static void mntput_no_expire(struct mount *mnt) unlock_mount_hash(); return; } + if (unlikely(mnt->mnt_pins.first)) { + mnt_add_count(mnt, 1); + rcu_read_unlock(); + unlock_mount_hash(); + mnt_pin_kill(mnt); + goto put_again; + } if (unlikely(mnt->mnt.mnt_flags & MNT_DOOMED)) { rcu_read_unlock(); unlock_mount_hash();
v5, new patch Signed-off-by: Kinglong Mee <kinglongmee@gmail.com> --- fs/namespace.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)