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@xxxxxxxxxx wrote:
> > > From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> > >
> > > 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@xxxxxxxxxxxxxxx>
> > > ---
> > > 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@xxxxxxxxxxxxxxx
