* [PATCH 1/2] nfsd: Avoid /* Fallthrough */ @ 2020-12-01 4:14 trondmy 2020-12-01 4:14 ` [PATCH 2/2] nfsd: Record NFSv4 pre/post-op attributes as non-atomic trondmy 2020-12-01 15:24 ` [PATCH 1/2] nfsd: Avoid /* Fallthrough */ Chuck Lever 0 siblings, 2 replies; 10+ messages in thread From: trondmy @ 2020-12-01 4:14 UTC (permalink / raw) To: J. Bruce Fields, Chuck Lever; +Cc: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> The '/* Fallthrough */' comment is already deprecated. Avoid it. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfsd/nfsfh.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c index 46c86f7bc429..e80a7525561d 100644 --- a/fs/nfsd/nfsfh.c +++ b/fs/nfsd/nfsfh.c @@ -302,9 +302,9 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp) switch (rqstp->rq_vers) { case 3: - if (!(dentry->d_sb->s_export_op->flags & EXPORT_OP_NOWCC)) - break; - /* Fallthrough */ + if (dentry->d_sb->s_export_op->flags & EXPORT_OP_NOWCC) + fhp->fh_no_wcc = true; + break; case 2: fhp->fh_no_wcc = true; } -- 2.28.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] nfsd: Record NFSv4 pre/post-op attributes as non-atomic 2020-12-01 4:14 [PATCH 1/2] nfsd: Avoid /* Fallthrough */ trondmy @ 2020-12-01 4:14 ` trondmy 2020-12-01 16:10 ` Chuck Lever 2020-12-01 19:35 ` J. Bruce Fields 2020-12-01 15:24 ` [PATCH 1/2] nfsd: Avoid /* Fallthrough */ Chuck Lever 1 sibling, 2 replies; 10+ messages in thread From: trondmy @ 2020-12-01 4:14 UTC (permalink / raw) To: J. Bruce Fields, Chuck Lever; +Cc: linux-nfs From: Trond Myklebust <trond.myklebust@hammerspace.com> For the case of NFSv4, specify to the client that the the pre/post-op attributes were not recorded atomically with the main operation. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- fs/nfs/export.c | 3 ++- fs/nfsd/nfsfh.c | 4 ++++ fs/nfsd/nfsfh.h | 5 +++++ fs/nfsd/xdr4.h | 2 +- include/linux/exportfs.h | 3 +++ 5 files changed, 15 insertions(+), 2 deletions(-) diff --git a/fs/nfs/export.c b/fs/nfs/export.c index 48b879cfe6e3..7412bb164fa7 100644 --- a/fs/nfs/export.c +++ b/fs/nfs/export.c @@ -172,5 +172,6 @@ const struct export_operations nfs_export_ops = { .fh_to_dentry = nfs_fh_to_dentry, .get_parent = nfs_get_parent, .flags = EXPORT_OP_NOWCC|EXPORT_OP_NOSUBTREECHK| - EXPORT_OP_CLOSE_BEFORE_UNLINK|EXPORT_OP_REMOTE_FS, + EXPORT_OP_CLOSE_BEFORE_UNLINK|EXPORT_OP_REMOTE_FS| + EXPORT_OP_NOATOMIC_ATTR, }; diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c index e80a7525561d..66f2ef67792a 100644 --- a/fs/nfsd/nfsfh.c +++ b/fs/nfsd/nfsfh.c @@ -301,6 +301,10 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp) fhp->fh_export = exp; switch (rqstp->rq_vers) { + case 4: + if (dentry->d_sb->s_export_op->flags & EXPORT_OP_NOATOMIC_ATTR) + fhp->fh_no_atomic_attr = true; + break; case 3: if (dentry->d_sb->s_export_op->flags & EXPORT_OP_NOWCC) fhp->fh_no_wcc = true; diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h index 347d10aa6265..cb20c2cd3469 100644 --- a/fs/nfsd/nfsfh.h +++ b/fs/nfsd/nfsfh.h @@ -36,6 +36,11 @@ typedef struct svc_fh { bool fh_locked; /* inode locked by us */ bool fh_want_write; /* remount protection taken */ bool fh_no_wcc; /* no wcc data needed */ + bool fh_no_atomic_attr; + /* + * wcc data is not atomic with + * operation + */ int fh_flags; /* FH flags */ #ifdef CONFIG_NFSD_V3 bool fh_post_saved; /* post-op attrs saved */ diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h index b4556e86e97c..a60ff5ce1a37 100644 --- a/fs/nfsd/xdr4.h +++ b/fs/nfsd/xdr4.h @@ -748,7 +748,7 @@ static inline void set_change_info(struct nfsd4_change_info *cinfo, struct svc_fh *fhp) { BUG_ON(!fhp->fh_pre_saved); - cinfo->atomic = (u32)fhp->fh_post_saved; + cinfo->atomic = (u32)(fhp->fh_post_saved && !fhp->fh_no_atomic_attr); cinfo->before_change = fhp->fh_pre_change; cinfo->after_change = fhp->fh_post_change; diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h index d93e8a6737bb..9f4d4bcbf251 100644 --- a/include/linux/exportfs.h +++ b/include/linux/exportfs.h @@ -217,6 +217,9 @@ struct export_operations { #define EXPORT_OP_NOSUBTREECHK (0x2) /* no subtree checking */ #define EXPORT_OP_CLOSE_BEFORE_UNLINK (0x4) /* close files before unlink */ #define EXPORT_OP_REMOTE_FS (0x8) /* Filesystem is remote */ +#define EXPORT_OP_NOATOMIC_ATTR (0x10) /* Filesystem cannot supply + atomic attribute updates + */ unsigned long flags; }; -- 2.28.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] nfsd: Record NFSv4 pre/post-op attributes as non-atomic 2020-12-01 4:14 ` [PATCH 2/2] nfsd: Record NFSv4 pre/post-op attributes as non-atomic trondmy @ 2020-12-01 16:10 ` Chuck Lever 2020-12-01 19:35 ` J. Bruce Fields 1 sibling, 0 replies; 10+ messages in thread From: Chuck Lever @ 2020-12-01 16:10 UTC (permalink / raw) To: trondmy; +Cc: Bruce Fields, Linux NFS Mailing List Hi Trond- > On Nov 30, 2020, at 11:14 PM, trondmy@kernel.org wrote: > > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > For the case of NFSv4, specify to the client that the the pre/post-op > attributes were not recorded atomically with the main operation. > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> I've applied this one provisionally to my tree for the next merge window. > --- > fs/nfs/export.c | 3 ++- > fs/nfsd/nfsfh.c | 4 ++++ > fs/nfsd/nfsfh.h | 5 +++++ > fs/nfsd/xdr4.h | 2 +- > include/linux/exportfs.h | 3 +++ > 5 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/fs/nfs/export.c b/fs/nfs/export.c > index 48b879cfe6e3..7412bb164fa7 100644 > --- a/fs/nfs/export.c > +++ b/fs/nfs/export.c > @@ -172,5 +172,6 @@ const struct export_operations nfs_export_ops = { > .fh_to_dentry = nfs_fh_to_dentry, > .get_parent = nfs_get_parent, > .flags = EXPORT_OP_NOWCC|EXPORT_OP_NOSUBTREECHK| > - EXPORT_OP_CLOSE_BEFORE_UNLINK|EXPORT_OP_REMOTE_FS, > + EXPORT_OP_CLOSE_BEFORE_UNLINK|EXPORT_OP_REMOTE_FS| > + EXPORT_OP_NOATOMIC_ATTR, > }; > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c > index e80a7525561d..66f2ef67792a 100644 > --- a/fs/nfsd/nfsfh.c > +++ b/fs/nfsd/nfsfh.c > @@ -301,6 +301,10 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp) > fhp->fh_export = exp; > > switch (rqstp->rq_vers) { > + case 4: > + if (dentry->d_sb->s_export_op->flags & EXPORT_OP_NOATOMIC_ATTR) > + fhp->fh_no_atomic_attr = true; > + break; > case 3: > if (dentry->d_sb->s_export_op->flags & EXPORT_OP_NOWCC) > fhp->fh_no_wcc = true; > diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h > index 347d10aa6265..cb20c2cd3469 100644 > --- a/fs/nfsd/nfsfh.h > +++ b/fs/nfsd/nfsfh.h > @@ -36,6 +36,11 @@ typedef struct svc_fh { > bool fh_locked; /* inode locked by us */ > bool fh_want_write; /* remount protection taken */ > bool fh_no_wcc; /* no wcc data needed */ > + bool fh_no_atomic_attr; > + /* > + * wcc data is not atomic with > + * operation > + */ > int fh_flags; /* FH flags */ > #ifdef CONFIG_NFSD_V3 > bool fh_post_saved; /* post-op attrs saved */ > diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h > index b4556e86e97c..a60ff5ce1a37 100644 > --- a/fs/nfsd/xdr4.h > +++ b/fs/nfsd/xdr4.h > @@ -748,7 +748,7 @@ static inline void > set_change_info(struct nfsd4_change_info *cinfo, struct svc_fh *fhp) > { > BUG_ON(!fhp->fh_pre_saved); > - cinfo->atomic = (u32)fhp->fh_post_saved; > + cinfo->atomic = (u32)(fhp->fh_post_saved && !fhp->fh_no_atomic_attr); > > cinfo->before_change = fhp->fh_pre_change; > cinfo->after_change = fhp->fh_post_change; > diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h > index d93e8a6737bb..9f4d4bcbf251 100644 > --- a/include/linux/exportfs.h > +++ b/include/linux/exportfs.h > @@ -217,6 +217,9 @@ struct export_operations { > #define EXPORT_OP_NOSUBTREECHK (0x2) /* no subtree checking */ > #define EXPORT_OP_CLOSE_BEFORE_UNLINK (0x4) /* close files before unlink */ > #define EXPORT_OP_REMOTE_FS (0x8) /* Filesystem is remote */ > +#define EXPORT_OP_NOATOMIC_ATTR (0x10) /* Filesystem cannot supply > + atomic attribute updates > + */ > unsigned long flags; > }; > > -- > 2.28.0 > -- Chuck Lever ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] nfsd: Record NFSv4 pre/post-op attributes as non-atomic 2020-12-01 4:14 ` [PATCH 2/2] nfsd: Record NFSv4 pre/post-op attributes as non-atomic trondmy 2020-12-01 16:10 ` Chuck Lever @ 2020-12-01 19:35 ` J. Bruce Fields 2020-12-01 20:18 ` Trond Myklebust 1 sibling, 1 reply; 10+ messages in thread From: J. Bruce Fields @ 2020-12-01 19:35 UTC (permalink / raw) To: trondmy; +Cc: J. Bruce Fields, Chuck Lever, linux-nfs On Mon, Nov 30, 2020 at 11:14:27PM -0500, trondmy@kernel.org wrote: > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > For the case of NFSv4, specify to the client that the the pre/post-op > attributes were not recorded atomically with the main operation. > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > --- > fs/nfs/export.c | 3 ++- > fs/nfsd/nfsfh.c | 4 ++++ > fs/nfsd/nfsfh.h | 5 +++++ > fs/nfsd/xdr4.h | 2 +- > include/linux/exportfs.h | 3 +++ > 5 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/fs/nfs/export.c b/fs/nfs/export.c > index 48b879cfe6e3..7412bb164fa7 100644 > --- a/fs/nfs/export.c > +++ b/fs/nfs/export.c > @@ -172,5 +172,6 @@ const struct export_operations nfs_export_ops = { > .fh_to_dentry = nfs_fh_to_dentry, > .get_parent = nfs_get_parent, > .flags = EXPORT_OP_NOWCC|EXPORT_OP_NOSUBTREECHK| > - EXPORT_OP_CLOSE_BEFORE_UNLINK|EXPORT_OP_REMOTE_FS, > + EXPORT_OP_CLOSE_BEFORE_UNLINK|EXPORT_OP_REMOTE_FS| > + EXPORT_OP_NOATOMIC_ATTR, So I still don't understand why we need a new flag for this. Before you said it was because a filesystem might want to turn off the v4 atomic flag but still return v3 post-wcc attributes. But it seems that a) we have no example of a filesystem that wants to do that, b) it would violate the protocol anyway. Is that right? If so, then let's just stick to one flag for both. --b. > }; > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c > index e80a7525561d..66f2ef67792a 100644 > --- a/fs/nfsd/nfsfh.c > +++ b/fs/nfsd/nfsfh.c > @@ -301,6 +301,10 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp) > fhp->fh_export = exp; > > switch (rqstp->rq_vers) { > + case 4: > + if (dentry->d_sb->s_export_op->flags & EXPORT_OP_NOATOMIC_ATTR) > + fhp->fh_no_atomic_attr = true; > + break; > case 3: > if (dentry->d_sb->s_export_op->flags & EXPORT_OP_NOWCC) > fhp->fh_no_wcc = true; > diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h > index 347d10aa6265..cb20c2cd3469 100644 > --- a/fs/nfsd/nfsfh.h > +++ b/fs/nfsd/nfsfh.h > @@ -36,6 +36,11 @@ typedef struct svc_fh { > bool fh_locked; /* inode locked by us */ > bool fh_want_write; /* remount protection taken */ > bool fh_no_wcc; /* no wcc data needed */ > + bool fh_no_atomic_attr; > + /* > + * wcc data is not atomic with > + * operation > + */ > int fh_flags; /* FH flags */ > #ifdef CONFIG_NFSD_V3 > bool fh_post_saved; /* post-op attrs saved */ > diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h > index b4556e86e97c..a60ff5ce1a37 100644 > --- a/fs/nfsd/xdr4.h > +++ b/fs/nfsd/xdr4.h > @@ -748,7 +748,7 @@ static inline void > set_change_info(struct nfsd4_change_info *cinfo, struct svc_fh *fhp) > { > BUG_ON(!fhp->fh_pre_saved); > - cinfo->atomic = (u32)fhp->fh_post_saved; > + cinfo->atomic = (u32)(fhp->fh_post_saved && !fhp->fh_no_atomic_attr); > > cinfo->before_change = fhp->fh_pre_change; > cinfo->after_change = fhp->fh_post_change; > diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h > index d93e8a6737bb..9f4d4bcbf251 100644 > --- a/include/linux/exportfs.h > +++ b/include/linux/exportfs.h > @@ -217,6 +217,9 @@ struct export_operations { > #define EXPORT_OP_NOSUBTREECHK (0x2) /* no subtree checking */ > #define EXPORT_OP_CLOSE_BEFORE_UNLINK (0x4) /* close files before unlink */ > #define EXPORT_OP_REMOTE_FS (0x8) /* Filesystem is remote */ > +#define EXPORT_OP_NOATOMIC_ATTR (0x10) /* Filesystem cannot supply > + atomic attribute updates > + */ > unsigned long flags; > }; > > -- > 2.28.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] nfsd: Record NFSv4 pre/post-op attributes as non-atomic 2020-12-01 19:35 ` J. Bruce Fields @ 2020-12-01 20:18 ` Trond Myklebust 2020-12-01 20:27 ` Trond Myklebust 0 siblings, 1 reply; 10+ messages in thread From: Trond Myklebust @ 2020-12-01 20:18 UTC (permalink / raw) To: bfields@fieldses.org, trondmy@kernel.org Cc: linux-nfs@vger.kernel.org, bfields@redhat.com, chuck.lever@oracle.com On Tue, 2020-12-01 at 14:35 -0500, J. Bruce Fields wrote: > On Mon, Nov 30, 2020 at 11:14:27PM -0500, trondmy@kernel.org wrote: > > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > > > For the case of NFSv4, specify to the client that the the pre/post- > > op > > attributes were not recorded atomically with the main operation. > > > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > > --- > > fs/nfs/export.c | 3 ++- > > fs/nfsd/nfsfh.c | 4 ++++ > > fs/nfsd/nfsfh.h | 5 +++++ > > fs/nfsd/xdr4.h | 2 +- > > include/linux/exportfs.h | 3 +++ > > 5 files changed, 15 insertions(+), 2 deletions(-) > > > > diff --git a/fs/nfs/export.c b/fs/nfs/export.c > > index 48b879cfe6e3..7412bb164fa7 100644 > > --- a/fs/nfs/export.c > > +++ b/fs/nfs/export.c > > @@ -172,5 +172,6 @@ const struct export_operations nfs_export_ops = > > { > > .fh_to_dentry = nfs_fh_to_dentry, > > .get_parent = nfs_get_parent, > > .flags = EXPORT_OP_NOWCC|EXPORT_OP_NOSUBTREECHK| > > - EXPORT_OP_CLOSE_BEFORE_UNLINK|EXPORT_OP_REMOTE_FS, > > + EXPORT_OP_CLOSE_BEFORE_UNLINK|EXPORT_OP_REMOTE_FS| > > + EXPORT_OP_NOATOMIC_ATTR, > > So I still don't understand why we need a new flag for this. > > Before you said it was because a filesystem might want to turn off > the > v4 atomic flag but still return v3 post-wcc attributes. > > But it seems that a) we have no example of a filesystem that wants to > do > that, b) it would violate the protocol anyway. > > Is that right? > > If so, then let's just stick to one flag for both. The "atomic" flag in NFSv4 is very specifically tied to a single attribute (the change attribute) and how it is collected in a very limited set of operations. The truth is that we probably _can_ eventually set it when re-exporting NFSv4 as NFSv4, assuming that the server actually supplies us with an atomic update. The same is not true of WCC. We might be able to supply knfsd with atomic updates for some operations, but certainly not for something like READ or WRITE. So I do think this needs to be separate flags. It is definitely describing completely different functionality. -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@hammerspace.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] nfsd: Record NFSv4 pre/post-op attributes as non-atomic 2020-12-01 20:18 ` Trond Myklebust @ 2020-12-01 20:27 ` Trond Myklebust 2020-12-01 20:53 ` bfields 0 siblings, 1 reply; 10+ messages in thread From: Trond Myklebust @ 2020-12-01 20:27 UTC (permalink / raw) To: bfields@fieldses.org Cc: linux-nfs@vger.kernel.org, bfields@redhat.com, chuck.lever@oracle.com On Tue, 2020-12-01 at 15:18 -0500, Trond Myklebust wrote: > On Tue, 2020-12-01 at 14:35 -0500, J. Bruce Fields wrote: > > On Mon, Nov 30, 2020 at 11:14:27PM -0500, trondmy@kernel.org wrote: > > > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > > > > > For the case of NFSv4, specify to the client that the the > > > pre/post- > > > op > > > attributes were not recorded atomically with the main operation. > > > > > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > > > --- > > > fs/nfs/export.c | 3 ++- > > > fs/nfsd/nfsfh.c | 4 ++++ > > > fs/nfsd/nfsfh.h | 5 +++++ > > > fs/nfsd/xdr4.h | 2 +- > > > include/linux/exportfs.h | 3 +++ > > > 5 files changed, 15 insertions(+), 2 deletions(-) > > > > > > diff --git a/fs/nfs/export.c b/fs/nfs/export.c > > > index 48b879cfe6e3..7412bb164fa7 100644 > > > --- a/fs/nfs/export.c > > > +++ b/fs/nfs/export.c > > > @@ -172,5 +172,6 @@ const struct export_operations nfs_export_ops > > > = > > > { > > > .fh_to_dentry = nfs_fh_to_dentry, > > > .get_parent = nfs_get_parent, > > > .flags = EXPORT_OP_NOWCC|EXPORT_OP_NOSUBTREECHK| > > > - > > > EXPORT_OP_CLOSE_BEFORE_UNLINK|EXPORT_OP_REMOTE_FS, > > > + EXPORT_OP_CLOSE_BEFORE_UNLINK|EXPORT_OP_REMOTE_FS > > > | > > > + EXPORT_OP_NOATOMIC_ATTR, > > > > So I still don't understand why we need a new flag for this. > > > > Before you said it was because a filesystem might want to turn off > > the > > v4 atomic flag but still return v3 post-wcc attributes. > > > > But it seems that a) we have no example of a filesystem that wants > > to > > do > > that, b) it would violate the protocol anyway. > > > > Is that right? > > > > If so, then let's just stick to one flag for both. > > The "atomic" flag in NFSv4 is very specifically tied to a single > attribute (the change attribute) and how it is collected in a very > limited set of operations. The truth is that we probably _can_ > eventually set it when re-exporting NFSv4 as NFSv4, assuming that the > server actually supplies us with an atomic update. > > The same is not true of WCC. We might be able to supply knfsd with > atomic updates for some operations, but certainly not for something > like READ or WRITE. > > So I do think this needs to be separate flags. It is definitely > describing completely different functionality. > Actually. The above is a good reason to just drop the EXPORT_OP_NOATOMIC_ATTR altogether. As far as I can tell, there is no need for it at all, since both the NFSv3 and NFSv4 client can supply atomic struct change_info4 in the cases where it is relevant (those cases being recording the changes to the parent directory/ies when doing CREATE, OPEN(O_CREAT), LINK, REMOVE and RENAME). -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@hammerspace.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] nfsd: Record NFSv4 pre/post-op attributes as non-atomic 2020-12-01 20:27 ` Trond Myklebust @ 2020-12-01 20:53 ` bfields 2020-12-01 20:59 ` Trond Myklebust 0 siblings, 1 reply; 10+ messages in thread From: bfields @ 2020-12-01 20:53 UTC (permalink / raw) To: Trond Myklebust Cc: linux-nfs@vger.kernel.org, bfields@redhat.com, chuck.lever@oracle.com On Tue, Dec 01, 2020 at 08:27:38PM +0000, Trond Myklebust wrote: > As far as I can tell, there is no need for it at all, since both the > NFSv3 and NFSv4 client can supply atomic struct change_info4 in the > cases where it is relevant (those cases being recording the changes to > the parent directory/ies when doing CREATE, OPEN(O_CREAT), LINK, REMOVE > and RENAME). I was wondering about that. We'd need some additional interface to allow nfs to supply that stuff to nfsd, right? --b. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] nfsd: Record NFSv4 pre/post-op attributes as non-atomic 2020-12-01 20:53 ` bfields @ 2020-12-01 20:59 ` Trond Myklebust 2020-12-01 21:14 ` bfields 0 siblings, 1 reply; 10+ messages in thread From: Trond Myklebust @ 2020-12-01 20:59 UTC (permalink / raw) To: bfields@fieldses.org Cc: linux-nfs@vger.kernel.org, bfields@redhat.com, chuck.lever@oracle.com On Tue, 2020-12-01 at 15:53 -0500, bfields@fieldses.org wrote: > On Tue, Dec 01, 2020 at 08:27:38PM +0000, Trond Myklebust wrote: > > As far as I can tell, there is no need for it at all, since both > > the > > NFSv3 and NFSv4 client can supply atomic struct change_info4 in the > > cases where it is relevant (those cases being recording the changes > > to > > the parent directory/ies when doing CREATE, OPEN(O_CREAT), LINK, > > REMOVE > > and RENAME). > > I was wondering about that. We'd need some additional interface to > allow nfs to supply that stuff to nfsd, right? The only problem is the pre-op attributes, since those are supplied by the server but never recorded anywhere. Otherwise, all you really need is to hold the inode lock on the directory to prevent the client from making updates after the operation is done. -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@hammerspace.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] nfsd: Record NFSv4 pre/post-op attributes as non-atomic 2020-12-01 20:59 ` Trond Myklebust @ 2020-12-01 21:14 ` bfields 0 siblings, 0 replies; 10+ messages in thread From: bfields @ 2020-12-01 21:14 UTC (permalink / raw) To: Trond Myklebust Cc: linux-nfs@vger.kernel.org, bfields@redhat.com, chuck.lever@oracle.com On Tue, Dec 01, 2020 at 08:59:00PM +0000, Trond Myklebust wrote: > On Tue, 2020-12-01 at 15:53 -0500, bfields@fieldses.org wrote: > > On Tue, Dec 01, 2020 at 08:27:38PM +0000, Trond Myklebust wrote: > > > As far as I can tell, there is no need for it at all, since both > > > the > > > NFSv3 and NFSv4 client can supply atomic struct change_info4 in the > > > cases where it is relevant (those cases being recording the changes > > > to > > > the parent directory/ies when doing CREATE, OPEN(O_CREAT), LINK, > > > REMOVE > > > and RENAME). > > > > I was wondering about that. We'd need some additional interface to > > allow nfs to supply that stuff to nfsd, right? > > The only problem is the pre-op attributes, since those are supplied by > the server but never recorded anywhere. Otherwise, all you really need > is to hold the inode lock on the directory to prevent the client from > making updates after the operation is done. We already have the lock over all those operations. I was worried that wouldn't prevent a getattr in another thread from updating the change attribute. Is that not possible? My only idea for returning extra information was a hack using kthread_data() as we're doing in nfsd_breaker_owns_lease(); nfs would call something like: void nfsd_set_cinfo(struct nfs4_change_info *cinfo) { struct svc_rqst *rqst; if (!i_am_nfsd()) return; rqst = kthread_data(current); rqst->cinfo = cinfo; } Then nfsd fetches it afterwards. Maybe there's a better way. --b. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] nfsd: Avoid /* Fallthrough */ 2020-12-01 4:14 [PATCH 1/2] nfsd: Avoid /* Fallthrough */ trondmy 2020-12-01 4:14 ` [PATCH 2/2] nfsd: Record NFSv4 pre/post-op attributes as non-atomic trondmy @ 2020-12-01 15:24 ` Chuck Lever 1 sibling, 0 replies; 10+ messages in thread From: Chuck Lever @ 2020-12-01 15:24 UTC (permalink / raw) To: trondmy; +Cc: Bruce Fields, Linux NFS Mailing List Hi - > On Nov 30, 2020, at 11:14 PM, trondmy@kernel.org wrote: > > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > The '/* Fallthrough */' comment is already deprecated. Avoid it. > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> I've squashed this into "nfsd: add a new EXPORT_OP_NOWCC flag to struct export_operations", in addition to addressing Jeff's comment about line in the patch description that states support for re-exporting NFS filesystems may never be supported. These changes will appear next time I push into the cel-next branch. > --- > fs/nfsd/nfsfh.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c > index 46c86f7bc429..e80a7525561d 100644 > --- a/fs/nfsd/nfsfh.c > +++ b/fs/nfsd/nfsfh.c > @@ -302,9 +302,9 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp) > > switch (rqstp->rq_vers) { > case 3: > - if (!(dentry->d_sb->s_export_op->flags & EXPORT_OP_NOWCC)) > - break; > - /* Fallthrough */ > + if (dentry->d_sb->s_export_op->flags & EXPORT_OP_NOWCC) > + fhp->fh_no_wcc = true; > + break; > case 2: > fhp->fh_no_wcc = true; > } > -- > 2.28.0 > -- Chuck Lever ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-12-01 21:14 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-12-01 4:14 [PATCH 1/2] nfsd: Avoid /* Fallthrough */ trondmy 2020-12-01 4:14 ` [PATCH 2/2] nfsd: Record NFSv4 pre/post-op attributes as non-atomic trondmy 2020-12-01 16:10 ` Chuck Lever 2020-12-01 19:35 ` J. Bruce Fields 2020-12-01 20:18 ` Trond Myklebust 2020-12-01 20:27 ` Trond Myklebust 2020-12-01 20:53 ` bfields 2020-12-01 20:59 ` Trond Myklebust 2020-12-01 21:14 ` bfields 2020-12-01 15:24 ` [PATCH 1/2] nfsd: Avoid /* Fallthrough */ Chuck Lever
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).