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