| Message ID | 165881740958.21666.5904057696047278505.stgit@noble.brown (mailing list archive) |
|---|---|
| Headers | show |
| Series | NFSD: clean up locking | expand |
On Tue, 2022-07-26 at 16:45 +1000, NeilBrown wrote: > This is the latest version of my series to clean up locking - > particularly of directories - in preparation for proposed patches which > change how directory locking works across the VFS. > > I've included Jeff's patches to validate the dentry after getting a > delegation. The second patch has been changed quite a bit to use > nfsd_lookup_dentry(). I've left Jeff's From: line in place - let me know > if you'd rather I change it. > That's fine. > Setting of ACLs and security labels has been moved from nfs4 code to > nfsd_setattr() which allows quite a lot of code cleanup. > > I think I've addressed all the concerns that have been raised, though > maybe not in the way that was suggested. > > I've tested this with cthon tests over v2, v3, v4.0, v4.1, and xfstests > on v3 and v4.1, and pynfs 4.0, 4.1. No problems appeared. > > Thanks, > NeilBrown > > > --- > > Jeff Layton (2): > NFSD: drop fh argument from alloc_init_deleg > NFSD: verify the opened dentry after setting a delegation > > NeilBrown (11): > NFSD: introduce struct nfsd_attrs > NFSD: set attributes when creating symlinks > NFSD: add security label to struct nfsd_attrs > NFSD: add posix ACLs to struct nfsd_attrs > NFSD: change nfsd_create()/nfsd_symlink() to unlock directory before returning. > NFSD: always drop directory lock in nfsd_unlink() > NFSD: only call fh_unlock() once in nfsd_link() > NFSD: reduce locking in nfsd_lookup() > NFSD: use explicit lock/unlock for directory ops > NFSD: use (un)lock_inode instead of fh_(un)lock for file operations > NFSD: discard fh_locked flag and fh_lock/fh_unlock > It looks like the last 5 patches are missing from the posting (everything after patch #8). > > fs/nfsd/acl.h | 6 +- > fs/nfsd/nfs2acl.c | 6 +- > fs/nfsd/nfs3acl.c | 4 +- > fs/nfsd/nfs3proc.c | 25 ++--- > fs/nfsd/nfs4acl.c | 46 ++------- > fs/nfsd/nfs4proc.c | 153 ++++++++++++----------------- > fs/nfsd/nfs4state.c | 71 +++++++++++--- > fs/nfsd/nfsfh.c | 22 ++++- > fs/nfsd/nfsfh.h | 58 +---------- > fs/nfsd/nfsproc.c | 19 ++-- > fs/nfsd/vfs.c | 230 +++++++++++++++++++++----------------------- > fs/nfsd/vfs.h | 31 ++++-- > fs/nfsd/xdr4.h | 1 + > 13 files changed, 314 insertions(+), 358 deletions(-) > > -- > Signature >
> On Jul 27, 2022, at 11:50 AM, Jeff Layton <jlayton@kernel.org> wrote: > > On Tue, 2022-07-26 at 16:45 +1000, NeilBrown wrote: >> This is the latest version of my series to clean up locking - >> particularly of directories - in preparation for proposed patches which >> change how directory locking works across the VFS. >> >> I've included Jeff's patches to validate the dentry after getting a >> delegation. The second patch has been changed quite a bit to use >> nfsd_lookup_dentry(). I've left Jeff's From: line in place - let me know >> if you'd rather I change it. >> > > That's fine. > >> Setting of ACLs and security labels has been moved from nfs4 code to >> nfsd_setattr() which allows quite a lot of code cleanup. >> >> I think I've addressed all the concerns that have been raised, though >> maybe not in the way that was suggested. >> >> I've tested this with cthon tests over v2, v3, v4.0, v4.1, and xfstests >> on v3 and v4.1, and pynfs 4.0, 4.1. No problems appeared. >> >> Thanks, >> NeilBrown >> >> >> --- >> >> Jeff Layton (2): >> NFSD: drop fh argument from alloc_init_deleg >> NFSD: verify the opened dentry after setting a delegation >> >> NeilBrown (11): >> NFSD: introduce struct nfsd_attrs >> NFSD: set attributes when creating symlinks >> NFSD: add security label to struct nfsd_attrs >> NFSD: add posix ACLs to struct nfsd_attrs >> NFSD: change nfsd_create()/nfsd_symlink() to unlock directory before returning. >> NFSD: always drop directory lock in nfsd_unlink() >> NFSD: only call fh_unlock() once in nfsd_link() >> NFSD: reduce locking in nfsd_lookup() >> NFSD: use explicit lock/unlock for directory ops >> NFSD: use (un)lock_inode instead of fh_(un)lock for file operations >> NFSD: discard fh_locked flag and fh_lock/fh_unlock >> > > It looks like the last 5 patches are missing from the posting > (everything after patch #8). Fwiw, I don't see them in my inbox either, and they don't appear on lore. >> fs/nfsd/acl.h | 6 +- >> fs/nfsd/nfs2acl.c | 6 +- >> fs/nfsd/nfs3acl.c | 4 +- >> fs/nfsd/nfs3proc.c | 25 ++--- >> fs/nfsd/nfs4acl.c | 46 ++------- >> fs/nfsd/nfs4proc.c | 153 ++++++++++++----------------- >> fs/nfsd/nfs4state.c | 71 +++++++++++--- >> fs/nfsd/nfsfh.c | 22 ++++- >> fs/nfsd/nfsfh.h | 58 +---------- >> fs/nfsd/nfsproc.c | 19 ++-- >> fs/nfsd/vfs.c | 230 +++++++++++++++++++++----------------------- >> fs/nfsd/vfs.h | 31 ++++-- >> fs/nfsd/xdr4.h | 1 + >> 13 files changed, 314 insertions(+), 358 deletions(-) >> >> -- >> Signature >> > > -- > Jeff Layton <jlayton@kernel.org> -- Chuck Lever
On Thu, 28 Jul 2022, Jeff Layton wrote: > On Tue, 2022-07-26 at 16:45 +1000, NeilBrown wrote: > > This is the latest version of my series to clean up locking - > > particularly of directories - in preparation for proposed patches which > > change how directory locking works across the VFS. > > > > I've included Jeff's patches to validate the dentry after getting a > > delegation. The second patch has been changed quite a bit to use > > nfsd_lookup_dentry(). I've left Jeff's From: line in place - let me know > > if you'd rather I change it. > > > > That's fine. > > > Setting of ACLs and security labels has been moved from nfs4 code to > > nfsd_setattr() which allows quite a lot of code cleanup. > > > > I think I've addressed all the concerns that have been raised, though > > maybe not in the way that was suggested. > > > > I've tested this with cthon tests over v2, v3, v4.0, v4.1, and xfstests > > on v3 and v4.1, and pynfs 4.0, 4.1. No problems appeared. > > > > Thanks, > > NeilBrown > > > > > > --- > > > > Jeff Layton (2): > > NFSD: drop fh argument from alloc_init_deleg > > NFSD: verify the opened dentry after setting a delegation > > > > NeilBrown (11): > > NFSD: introduce struct nfsd_attrs > > NFSD: set attributes when creating symlinks > > NFSD: add security label to struct nfsd_attrs > > NFSD: add posix ACLs to struct nfsd_attrs > > NFSD: change nfsd_create()/nfsd_symlink() to unlock directory before returning. > > NFSD: always drop directory lock in nfsd_unlink() > > NFSD: only call fh_unlock() once in nfsd_link() > > NFSD: reduce locking in nfsd_lookup() > > NFSD: use explicit lock/unlock for directory ops > > NFSD: use (un)lock_inode instead of fh_(un)lock for file operations > > NFSD: discard fh_locked flag and fh_lock/fh_unlock > > > > It looks like the last 5 patches are missing from the posting > (everything after patch #8). > Sorry. You should have them now. NeilBrown
> On Jul 26, 2022, at 2:45 AM, NeilBrown <neilb@suse.de> wrote: > > This is the latest version of my series to clean up locking - > particularly of directories - in preparation for proposed patches which > change how directory locking works across the VFS. > > I've included Jeff's patches to validate the dentry after getting a > delegation. The second patch has been changed quite a bit to use > nfsd_lookup_dentry(). I've left Jeff's From: line in place - let me know > if you'd rather I change it. > > Setting of ACLs and security labels has been moved from nfs4 code to > nfsd_setattr() which allows quite a lot of code cleanup. > > I think I've addressed all the concerns that have been raised, though > maybe not in the way that was suggested. > > I've tested this with cthon tests over v2, v3, v4.0, v4.1, and xfstests > on v3 and v4.1, and pynfs 4.0, 4.1. No problems appeared. > > Thanks, > NeilBrown Hi Neil- No objections to this round, looks like a very good set of clean-ups. I've also resurrected NFSv2 on my test systems, and captured a baseline without these patches applied. There are a number of cosmetic issues I found, posting those separately. If you trust me, I can take care of those here, or you can submit a v3 (v4?). > --- > > Jeff Layton (2): > NFSD: drop fh argument from alloc_init_deleg > NFSD: verify the opened dentry after setting a delegation > > NeilBrown (11): > NFSD: introduce struct nfsd_attrs > NFSD: set attributes when creating symlinks > NFSD: add security label to struct nfsd_attrs > NFSD: add posix ACLs to struct nfsd_attrs > NFSD: change nfsd_create()/nfsd_symlink() to unlock directory before returning. > NFSD: always drop directory lock in nfsd_unlink() > NFSD: only call fh_unlock() once in nfsd_link() > NFSD: reduce locking in nfsd_lookup() > NFSD: use explicit lock/unlock for directory ops > NFSD: use (un)lock_inode instead of fh_(un)lock for file operations > NFSD: discard fh_locked flag and fh_lock/fh_unlock > > > fs/nfsd/acl.h | 6 +- > fs/nfsd/nfs2acl.c | 6 +- > fs/nfsd/nfs3acl.c | 4 +- > fs/nfsd/nfs3proc.c | 25 ++--- > fs/nfsd/nfs4acl.c | 46 ++------- > fs/nfsd/nfs4proc.c | 153 ++++++++++++----------------- > fs/nfsd/nfs4state.c | 71 +++++++++++--- > fs/nfsd/nfsfh.c | 22 ++++- > fs/nfsd/nfsfh.h | 58 +---------- > fs/nfsd/nfsproc.c | 19 ++-- > fs/nfsd/vfs.c | 230 +++++++++++++++++++++----------------------- > fs/nfsd/vfs.h | 31 ++++-- > fs/nfsd/xdr4.h | 1 + > 13 files changed, 314 insertions(+), 358 deletions(-) > > -- > Signature > -- Chuck Lever
On Fri, 29 Jul 2022, Chuck Lever III wrote: > > > On Jul 26, 2022, at 2:45 AM, NeilBrown <neilb@suse.de> wrote: > > > > This is the latest version of my series to clean up locking - > > particularly of directories - in preparation for proposed patches which > > change how directory locking works across the VFS. > > > > I've included Jeff's patches to validate the dentry after getting a > > delegation. The second patch has been changed quite a bit to use > > nfsd_lookup_dentry(). I've left Jeff's From: line in place - let me know > > if you'd rather I change it. > > > > Setting of ACLs and security labels has been moved from nfs4 code to > > nfsd_setattr() which allows quite a lot of code cleanup. > > > > I think I've addressed all the concerns that have been raised, though > > maybe not in the way that was suggested. > > > > I've tested this with cthon tests over v2, v3, v4.0, v4.1, and xfstests > > on v3 and v4.1, and pynfs 4.0, 4.1. No problems appeared. > > > > Thanks, > > NeilBrown > > Hi Neil- > > No objections to this round, looks like a very good set of clean-ups. > > I've also resurrected NFSv2 on my test systems, and captured a baseline > without these patches applied. > > There are a number of cosmetic issues I found, posting those separately. > If you trust me, I can take care of those here, or you can submit a > v3 (v4?). I would love for you to make those changes yourself! As I noted separately there is a bug : nfserrno() needed where you suggested __be32. All others are cosmetic and I trust you to fix those up however seems best. Thanks, NeilBrown
This is the latest version of my series to clean up locking - particularly of directories - in preparation for proposed patches which change how directory locking works across the VFS. I've included Jeff's patches to validate the dentry after getting a delegation. The second patch has been changed quite a bit to use nfsd_lookup_dentry(). I've left Jeff's From: line in place - let me know if you'd rather I change it. Setting of ACLs and security labels has been moved from nfs4 code to nfsd_setattr() which allows quite a lot of code cleanup. I think I've addressed all the concerns that have been raised, though maybe not in the way that was suggested. I've tested this with cthon tests over v2, v3, v4.0, v4.1, and xfstests on v3 and v4.1, and pynfs 4.0, 4.1. No problems appeared. Thanks, NeilBrown --- Jeff Layton (2): NFSD: drop fh argument from alloc_init_deleg NFSD: verify the opened dentry after setting a delegation NeilBrown (11): NFSD: introduce struct nfsd_attrs NFSD: set attributes when creating symlinks NFSD: add security label to struct nfsd_attrs NFSD: add posix ACLs to struct nfsd_attrs NFSD: change nfsd_create()/nfsd_symlink() to unlock directory before returning. NFSD: always drop directory lock in nfsd_unlink() NFSD: only call fh_unlock() once in nfsd_link() NFSD: reduce locking in nfsd_lookup() NFSD: use explicit lock/unlock for directory ops NFSD: use (un)lock_inode instead of fh_(un)lock for file operations NFSD: discard fh_locked flag and fh_lock/fh_unlock fs/nfsd/acl.h | 6 +- fs/nfsd/nfs2acl.c | 6 +- fs/nfsd/nfs3acl.c | 4 +- fs/nfsd/nfs3proc.c | 25 ++--- fs/nfsd/nfs4acl.c | 46 ++------- fs/nfsd/nfs4proc.c | 153 ++++++++++++----------------- fs/nfsd/nfs4state.c | 71 +++++++++++--- fs/nfsd/nfsfh.c | 22 ++++- fs/nfsd/nfsfh.h | 58 +---------- fs/nfsd/nfsproc.c | 19 ++-- fs/nfsd/vfs.c | 230 +++++++++++++++++++++----------------------- fs/nfsd/vfs.h | 31 ++++-- fs/nfsd/xdr4.h | 1 + 13 files changed, 314 insertions(+), 358 deletions(-) -- Signature