* [PATCH 1/6] nfsd: drop inode parameter from nfsd4_change_attribute()
2024-10-14 19:26 [PATCH 0/6] nfsd: update the delstid patches for latest draft changes Jeff Layton
@ 2024-10-14 19:26 ` Jeff Layton
2024-10-15 18:29 ` Chuck Lever
2024-10-14 19:26 ` [PATCH 2/6] nfsd: switch to autogenerated definitions for open_delegation_type4 Jeff Layton
` (6 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Jeff Layton @ 2024-10-14 19:26 UTC (permalink / raw)
To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Jonathan Corbet, Trond Myklebust, Anna Schumaker, Thomas Haynes
Cc: linux-nfs, linux-kernel, linux-doc, Jeff Layton
Fix up nfs4_delegation_stat() to fetch STATX_MODE, and then drop the
inode parameter from nfsd4_change_attribute(), since it's no longer
needed.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
fs/nfsd/nfs4state.c | 5 ++---
fs/nfsd/nfs4xdr.c | 2 +-
fs/nfsd/nfsfh.c | 11 ++++-------
fs/nfsd/nfsfh.h | 3 +--
4 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d753926db09eedf629fc3e0938f10b1a6fdb0245..2961a277a79c1f4cdb8c29a7c19abcb3305b61a1 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5953,7 +5953,7 @@ nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
path.dentry = file_dentry(nf->nf_file);
rc = vfs_getattr(&path, stat,
- (STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
+ (STATX_MODE | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
AT_STATX_SYNC_AS_STAT);
nfsd_file_put(nf);
@@ -6037,8 +6037,7 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
}
open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
- dp->dl_cb_fattr.ncf_initial_cinfo =
- nfsd4_change_attribute(&stat, d_inode(currentfh->fh_dentry));
+ dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
} else {
open->op_delegate_type = NFS4_OPEN_DELEGATE_READ;
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 6286ad2afa069f5274ffa352209b7d3c8c577dac..da7ec663da7326ad5c68a9c738b12d09cfcdc65a 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3621,7 +3621,7 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
args.change_attr = ncf->ncf_initial_cinfo;
nfs4_put_stid(&dp->dl_stid);
} else {
- args.change_attr = nfsd4_change_attribute(&args.stat, d_inode(dentry));
+ args.change_attr = nfsd4_change_attribute(&args.stat);
}
if (err)
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index 4c5deea0e9535f2b197aa6ca1786d61730d53c44..453b7b52317d538971ce41f7e0492e5ab28b236d 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -670,20 +670,18 @@ fh_update(struct svc_fh *fhp)
__be32 __must_check fh_fill_pre_attrs(struct svc_fh *fhp)
{
bool v4 = (fhp->fh_maxsize == NFS4_FHSIZE);
- struct inode *inode;
struct kstat stat;
__be32 err;
if (fhp->fh_no_wcc || fhp->fh_pre_saved)
return nfs_ok;
- inode = d_inode(fhp->fh_dentry);
err = fh_getattr(fhp, &stat);
if (err)
return err;
if (v4)
- fhp->fh_pre_change = nfsd4_change_attribute(&stat, inode);
+ fhp->fh_pre_change = nfsd4_change_attribute(&stat);
fhp->fh_pre_mtime = stat.mtime;
fhp->fh_pre_ctime = stat.ctime;
@@ -700,7 +698,6 @@ __be32 __must_check fh_fill_pre_attrs(struct svc_fh *fhp)
__be32 fh_fill_post_attrs(struct svc_fh *fhp)
{
bool v4 = (fhp->fh_maxsize == NFS4_FHSIZE);
- struct inode *inode = d_inode(fhp->fh_dentry);
__be32 err;
if (fhp->fh_no_wcc)
@@ -716,7 +713,7 @@ __be32 fh_fill_post_attrs(struct svc_fh *fhp)
fhp->fh_post_saved = true;
if (v4)
fhp->fh_post_change =
- nfsd4_change_attribute(&fhp->fh_post_attr, inode);
+ nfsd4_change_attribute(&fhp->fh_post_attr);
return nfs_ok;
}
@@ -824,13 +821,13 @@ enum fsid_source fsid_source(const struct svc_fh *fhp)
* assume that the new change attr is always logged to stable storage in some
* fashion before the results can be seen.
*/
-u64 nfsd4_change_attribute(const struct kstat *stat, const struct inode *inode)
+u64 nfsd4_change_attribute(const struct kstat *stat)
{
u64 chattr;
if (stat->result_mask & STATX_CHANGE_COOKIE) {
chattr = stat->change_cookie;
- if (S_ISREG(inode->i_mode) &&
+ if (S_ISREG(stat->mode) &&
!(stat->attributes & STATX_ATTR_CHANGE_MONOTONIC)) {
chattr += (u64)stat->ctime.tv_sec << 30;
chattr += stat->ctime.tv_nsec;
diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
index 5b7394801dc4270dbd5236f3e2f2237130c73dad..876152a91f122f83fb94ffdfb0eedf8fca56a20c 100644
--- a/fs/nfsd/nfsfh.h
+++ b/fs/nfsd/nfsfh.h
@@ -297,8 +297,7 @@ static inline void fh_clear_pre_post_attrs(struct svc_fh *fhp)
fhp->fh_pre_saved = false;
}
-u64 nfsd4_change_attribute(const struct kstat *stat,
- const struct inode *inode);
+u64 nfsd4_change_attribute(const struct kstat *stat);
__be32 __must_check fh_fill_pre_attrs(struct svc_fh *fhp);
__be32 fh_fill_post_attrs(struct svc_fh *fhp);
__be32 __must_check fh_fill_both_attrs(struct svc_fh *fhp);
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 1/6] nfsd: drop inode parameter from nfsd4_change_attribute()
2024-10-14 19:26 ` [PATCH 1/6] nfsd: drop inode parameter from nfsd4_change_attribute() Jeff Layton
@ 2024-10-15 18:29 ` Chuck Lever
2024-10-15 18:42 ` Jeff Layton
0 siblings, 1 reply; 18+ messages in thread
From: Chuck Lever @ 2024-10-15 18:29 UTC (permalink / raw)
To: Jeff Layton
Cc: Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Jonathan Corbet, Trond Myklebust, Anna Schumaker, Thomas Haynes,
linux-nfs, linux-kernel, linux-doc
Hey Jeff -
On Mon, Oct 14, 2024 at 03:26:49PM -0400, Jeff Layton wrote:
> Fix up nfs4_delegation_stat() to fetch STATX_MODE,
The patch description isn't clear about why this change is needed.
After reading through the other patches, I'm not sure I'm any more
enlightened about it ;-)
> and then drop the
> inode parameter from nfsd4_change_attribute(), since it's no longer
> needed.
Since nfsd4_change_attribute() expects @stat to be filled in by the
caller, it needs a kdoc-style comment that documents that part of
the API contract.
I can add one when applying this patch, unless you would like to
resend this one or send me something to squash into this change.
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> fs/nfsd/nfs4state.c | 5 ++---
> fs/nfsd/nfs4xdr.c | 2 +-
> fs/nfsd/nfsfh.c | 11 ++++-------
> fs/nfsd/nfsfh.h | 3 +--
> 4 files changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index d753926db09eedf629fc3e0938f10b1a6fdb0245..2961a277a79c1f4cdb8c29a7c19abcb3305b61a1 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5953,7 +5953,7 @@ nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
> path.dentry = file_dentry(nf->nf_file);
>
> rc = vfs_getattr(&path, stat,
> - (STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> + (STATX_MODE | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> AT_STATX_SYNC_AS_STAT);
>
> nfsd_file_put(nf);
> @@ -6037,8 +6037,7 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> }
> open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
> dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
> - dp->dl_cb_fattr.ncf_initial_cinfo =
> - nfsd4_change_attribute(&stat, d_inode(currentfh->fh_dentry));
> + dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
> trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> } else {
> open->op_delegate_type = NFS4_OPEN_DELEGATE_READ;
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 6286ad2afa069f5274ffa352209b7d3c8c577dac..da7ec663da7326ad5c68a9c738b12d09cfcdc65a 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -3621,7 +3621,7 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
> args.change_attr = ncf->ncf_initial_cinfo;
> nfs4_put_stid(&dp->dl_stid);
> } else {
> - args.change_attr = nfsd4_change_attribute(&args.stat, d_inode(dentry));
> + args.change_attr = nfsd4_change_attribute(&args.stat);
> }
>
> if (err)
> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> index 4c5deea0e9535f2b197aa6ca1786d61730d53c44..453b7b52317d538971ce41f7e0492e5ab28b236d 100644
> --- a/fs/nfsd/nfsfh.c
> +++ b/fs/nfsd/nfsfh.c
> @@ -670,20 +670,18 @@ fh_update(struct svc_fh *fhp)
> __be32 __must_check fh_fill_pre_attrs(struct svc_fh *fhp)
> {
> bool v4 = (fhp->fh_maxsize == NFS4_FHSIZE);
> - struct inode *inode;
> struct kstat stat;
> __be32 err;
>
> if (fhp->fh_no_wcc || fhp->fh_pre_saved)
> return nfs_ok;
>
> - inode = d_inode(fhp->fh_dentry);
> err = fh_getattr(fhp, &stat);
> if (err)
> return err;
>
> if (v4)
> - fhp->fh_pre_change = nfsd4_change_attribute(&stat, inode);
> + fhp->fh_pre_change = nfsd4_change_attribute(&stat);
>
> fhp->fh_pre_mtime = stat.mtime;
> fhp->fh_pre_ctime = stat.ctime;
> @@ -700,7 +698,6 @@ __be32 __must_check fh_fill_pre_attrs(struct svc_fh *fhp)
> __be32 fh_fill_post_attrs(struct svc_fh *fhp)
> {
> bool v4 = (fhp->fh_maxsize == NFS4_FHSIZE);
> - struct inode *inode = d_inode(fhp->fh_dentry);
> __be32 err;
>
> if (fhp->fh_no_wcc)
> @@ -716,7 +713,7 @@ __be32 fh_fill_post_attrs(struct svc_fh *fhp)
> fhp->fh_post_saved = true;
> if (v4)
> fhp->fh_post_change =
> - nfsd4_change_attribute(&fhp->fh_post_attr, inode);
> + nfsd4_change_attribute(&fhp->fh_post_attr);
> return nfs_ok;
> }
>
> @@ -824,13 +821,13 @@ enum fsid_source fsid_source(const struct svc_fh *fhp)
> * assume that the new change attr is always logged to stable storage in some
> * fashion before the results can be seen.
> */
> -u64 nfsd4_change_attribute(const struct kstat *stat, const struct inode *inode)
> +u64 nfsd4_change_attribute(const struct kstat *stat)
> {
> u64 chattr;
>
> if (stat->result_mask & STATX_CHANGE_COOKIE) {
> chattr = stat->change_cookie;
> - if (S_ISREG(inode->i_mode) &&
> + if (S_ISREG(stat->mode) &&
> !(stat->attributes & STATX_ATTR_CHANGE_MONOTONIC)) {
> chattr += (u64)stat->ctime.tv_sec << 30;
> chattr += stat->ctime.tv_nsec;
> diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
> index 5b7394801dc4270dbd5236f3e2f2237130c73dad..876152a91f122f83fb94ffdfb0eedf8fca56a20c 100644
> --- a/fs/nfsd/nfsfh.h
> +++ b/fs/nfsd/nfsfh.h
> @@ -297,8 +297,7 @@ static inline void fh_clear_pre_post_attrs(struct svc_fh *fhp)
> fhp->fh_pre_saved = false;
> }
>
> -u64 nfsd4_change_attribute(const struct kstat *stat,
> - const struct inode *inode);
> +u64 nfsd4_change_attribute(const struct kstat *stat);
> __be32 __must_check fh_fill_pre_attrs(struct svc_fh *fhp);
> __be32 fh_fill_post_attrs(struct svc_fh *fhp);
> __be32 __must_check fh_fill_both_attrs(struct svc_fh *fhp);
>
> --
> 2.47.0
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 1/6] nfsd: drop inode parameter from nfsd4_change_attribute()
2024-10-15 18:29 ` Chuck Lever
@ 2024-10-15 18:42 ` Jeff Layton
2024-10-15 18:50 ` Chuck Lever
0 siblings, 1 reply; 18+ messages in thread
From: Jeff Layton @ 2024-10-15 18:42 UTC (permalink / raw)
To: Chuck Lever
Cc: Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Jonathan Corbet, Trond Myklebust, Anna Schumaker, Thomas Haynes,
linux-nfs, linux-kernel, linux-doc
On Tue, 2024-10-15 at 14:29 -0400, Chuck Lever wrote:
> Hey Jeff -
>
> On Mon, Oct 14, 2024 at 03:26:49PM -0400, Jeff Layton wrote:
> > Fix up nfs4_delegation_stat() to fetch STATX_MODE,
>
> The patch description isn't clear about why this change is needed.
> After reading through the other patches, I'm not sure I'm any more
> enlightened about it ;-)
>
My original thinking was that this patch was just a cleanup and
simplification, but now that I look, I think the inode we were passing
to this function in nfs4_open_delegation was wrong and that could throw
off the result in some cases.
Maybe we should add a Fixes: tag for this:
bf92e5008b17 nfsd: fix initial getattr on write delegation
...since that should have fixed this call as well.
>
> > and then drop the
> > inode parameter from nfsd4_change_attribute(), since it's no longer
> > needed.
>
> Since nfsd4_change_attribute() expects @stat to be filled in by the
> caller, it needs a kdoc-style comment that documents that part of
> the API contract.
>
Agreed. It needs STATX_MODE and STATX_CTIME. It can also make use of
STATX_CHANGE_COOKIE if present.
> I can add one when applying this patch, unless you would like to
> resend this one or send me something to squash into this change.
>
That sounds great. A kdoc comment over that is a good idea.
>
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > fs/nfsd/nfs4state.c | 5 ++---
> > fs/nfsd/nfs4xdr.c | 2 +-
> > fs/nfsd/nfsfh.c | 11 ++++-------
> > fs/nfsd/nfsfh.h | 3 +--
> > 4 files changed, 8 insertions(+), 13 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index d753926db09eedf629fc3e0938f10b1a6fdb0245..2961a277a79c1f4cdb8c29a7c19abcb3305b61a1 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -5953,7 +5953,7 @@ nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
> > path.dentry = file_dentry(nf->nf_file);
> >
> > rc = vfs_getattr(&path, stat,
> > - (STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> > + (STATX_MODE | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> > AT_STATX_SYNC_AS_STAT);
> >
> > nfsd_file_put(nf);
> > @@ -6037,8 +6037,7 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > }
> > open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
> > dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
> > - dp->dl_cb_fattr.ncf_initial_cinfo =
> > - nfsd4_change_attribute(&stat, d_inode(currentfh->fh_dentry));
> > + dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
> > trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> > } else {
> > open->op_delegate_type = NFS4_OPEN_DELEGATE_READ;
> > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > index 6286ad2afa069f5274ffa352209b7d3c8c577dac..da7ec663da7326ad5c68a9c738b12d09cfcdc65a 100644
> > --- a/fs/nfsd/nfs4xdr.c
> > +++ b/fs/nfsd/nfs4xdr.c
> > @@ -3621,7 +3621,7 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
> > args.change_attr = ncf->ncf_initial_cinfo;
> > nfs4_put_stid(&dp->dl_stid);
> > } else {
> > - args.change_attr = nfsd4_change_attribute(&args.stat, d_inode(dentry));
> > + args.change_attr = nfsd4_change_attribute(&args.stat);
> > }
> >
> > if (err)
> > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > index 4c5deea0e9535f2b197aa6ca1786d61730d53c44..453b7b52317d538971ce41f7e0492e5ab28b236d 100644
> > --- a/fs/nfsd/nfsfh.c
> > +++ b/fs/nfsd/nfsfh.c
> > @@ -670,20 +670,18 @@ fh_update(struct svc_fh *fhp)
> > __be32 __must_check fh_fill_pre_attrs(struct svc_fh *fhp)
> > {
> > bool v4 = (fhp->fh_maxsize == NFS4_FHSIZE);
> > - struct inode *inode;
> > struct kstat stat;
> > __be32 err;
> >
> > if (fhp->fh_no_wcc || fhp->fh_pre_saved)
> > return nfs_ok;
> >
> > - inode = d_inode(fhp->fh_dentry);
> > err = fh_getattr(fhp, &stat);
> > if (err)
> > return err;
> >
> > if (v4)
> > - fhp->fh_pre_change = nfsd4_change_attribute(&stat, inode);
> > + fhp->fh_pre_change = nfsd4_change_attribute(&stat);
> >
> > fhp->fh_pre_mtime = stat.mtime;
> > fhp->fh_pre_ctime = stat.ctime;
> > @@ -700,7 +698,6 @@ __be32 __must_check fh_fill_pre_attrs(struct svc_fh *fhp)
> > __be32 fh_fill_post_attrs(struct svc_fh *fhp)
> > {
> > bool v4 = (fhp->fh_maxsize == NFS4_FHSIZE);
> > - struct inode *inode = d_inode(fhp->fh_dentry);
> > __be32 err;
> >
> > if (fhp->fh_no_wcc)
> > @@ -716,7 +713,7 @@ __be32 fh_fill_post_attrs(struct svc_fh *fhp)
> > fhp->fh_post_saved = true;
> > if (v4)
> > fhp->fh_post_change =
> > - nfsd4_change_attribute(&fhp->fh_post_attr, inode);
> > + nfsd4_change_attribute(&fhp->fh_post_attr);
> > return nfs_ok;
> > }
> >
> > @@ -824,13 +821,13 @@ enum fsid_source fsid_source(const struct svc_fh *fhp)
> > * assume that the new change attr is always logged to stable storage in some
> > * fashion before the results can be seen.
> > */
> > -u64 nfsd4_change_attribute(const struct kstat *stat, const struct inode *inode)
> > +u64 nfsd4_change_attribute(const struct kstat *stat)
> > {
> > u64 chattr;
> >
> > if (stat->result_mask & STATX_CHANGE_COOKIE) {
> > chattr = stat->change_cookie;
> > - if (S_ISREG(inode->i_mode) &&
> > + if (S_ISREG(stat->mode) &&
> > !(stat->attributes & STATX_ATTR_CHANGE_MONOTONIC)) {
> > chattr += (u64)stat->ctime.tv_sec << 30;
> > chattr += stat->ctime.tv_nsec;
> > diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
> > index 5b7394801dc4270dbd5236f3e2f2237130c73dad..876152a91f122f83fb94ffdfb0eedf8fca56a20c 100644
> > --- a/fs/nfsd/nfsfh.h
> > +++ b/fs/nfsd/nfsfh.h
> > @@ -297,8 +297,7 @@ static inline void fh_clear_pre_post_attrs(struct svc_fh *fhp)
> > fhp->fh_pre_saved = false;
> > }
> >
> > -u64 nfsd4_change_attribute(const struct kstat *stat,
> > - const struct inode *inode);
> > +u64 nfsd4_change_attribute(const struct kstat *stat);
> > __be32 __must_check fh_fill_pre_attrs(struct svc_fh *fhp);
> > __be32 fh_fill_post_attrs(struct svc_fh *fhp);
> > __be32 __must_check fh_fill_both_attrs(struct svc_fh *fhp);
> >
> > --
> > 2.47.0
> >
>
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 1/6] nfsd: drop inode parameter from nfsd4_change_attribute()
2024-10-15 18:42 ` Jeff Layton
@ 2024-10-15 18:50 ` Chuck Lever
0 siblings, 0 replies; 18+ messages in thread
From: Chuck Lever @ 2024-10-15 18:50 UTC (permalink / raw)
To: Jeff Layton
Cc: Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Jonathan Corbet, Trond Myklebust, Anna Schumaker, Thomas Haynes,
linux-nfs, linux-kernel, linux-doc
On Tue, Oct 15, 2024 at 02:42:10PM -0400, Jeff Layton wrote:
> On Tue, 2024-10-15 at 14:29 -0400, Chuck Lever wrote:
> > Hey Jeff -
> >
> > On Mon, Oct 14, 2024 at 03:26:49PM -0400, Jeff Layton wrote:
> > > Fix up nfs4_delegation_stat() to fetch STATX_MODE,
> >
> > The patch description isn't clear about why this change is needed.
> > After reading through the other patches, I'm not sure I'm any more
> > enlightened about it ;-)
> >
>
> My original thinking was that this patch was just a cleanup and
> simplification, but now that I look, I think the inode we were passing
> to this function in nfs4_open_delegation was wrong and that could throw
> off the result in some cases.
>
> Maybe we should add a Fixes: tag for this:
>
> bf92e5008b17 nfsd: fix initial getattr on write delegation
>
> ...since that should have fixed this call as well.
>
> >
> > > and then drop the
> > > inode parameter from nfsd4_change_attribute(), since it's no longer
> > > needed.
> >
> > Since nfsd4_change_attribute() expects @stat to be filled in by the
> > caller, it needs a kdoc-style comment that documents that part of
> > the API contract.
> >
>
> Agreed. It needs STATX_MODE and STATX_CTIME. It can also make use of
> STATX_CHANGE_COOKIE if present.
>
> > I can add one when applying this patch, unless you would like to
> > resend this one or send me something to squash into this change.
> >
>
> That sounds great. A kdoc comment over that is a good idea.
I'll make adjustments before applying.
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > ---
> > > fs/nfsd/nfs4state.c | 5 ++---
> > > fs/nfsd/nfs4xdr.c | 2 +-
> > > fs/nfsd/nfsfh.c | 11 ++++-------
> > > fs/nfsd/nfsfh.h | 3 +--
> > > 4 files changed, 8 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > > index d753926db09eedf629fc3e0938f10b1a6fdb0245..2961a277a79c1f4cdb8c29a7c19abcb3305b61a1 100644
> > > --- a/fs/nfsd/nfs4state.c
> > > +++ b/fs/nfsd/nfs4state.c
> > > @@ -5953,7 +5953,7 @@ nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
> > > path.dentry = file_dentry(nf->nf_file);
> > >
> > > rc = vfs_getattr(&path, stat,
> > > - (STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> > > + (STATX_MODE | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE),
> > > AT_STATX_SYNC_AS_STAT);
> > >
> > > nfsd_file_put(nf);
> > > @@ -6037,8 +6037,7 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > > }
> > > open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
> > > dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
> > > - dp->dl_cb_fattr.ncf_initial_cinfo =
> > > - nfsd4_change_attribute(&stat, d_inode(currentfh->fh_dentry));
> > > + dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
> > > trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> > > } else {
> > > open->op_delegate_type = NFS4_OPEN_DELEGATE_READ;
> > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > > index 6286ad2afa069f5274ffa352209b7d3c8c577dac..da7ec663da7326ad5c68a9c738b12d09cfcdc65a 100644
> > > --- a/fs/nfsd/nfs4xdr.c
> > > +++ b/fs/nfsd/nfs4xdr.c
> > > @@ -3621,7 +3621,7 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
> > > args.change_attr = ncf->ncf_initial_cinfo;
> > > nfs4_put_stid(&dp->dl_stid);
> > > } else {
> > > - args.change_attr = nfsd4_change_attribute(&args.stat, d_inode(dentry));
> > > + args.change_attr = nfsd4_change_attribute(&args.stat);
> > > }
> > >
> > > if (err)
> > > diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> > > index 4c5deea0e9535f2b197aa6ca1786d61730d53c44..453b7b52317d538971ce41f7e0492e5ab28b236d 100644
> > > --- a/fs/nfsd/nfsfh.c
> > > +++ b/fs/nfsd/nfsfh.c
> > > @@ -670,20 +670,18 @@ fh_update(struct svc_fh *fhp)
> > > __be32 __must_check fh_fill_pre_attrs(struct svc_fh *fhp)
> > > {
> > > bool v4 = (fhp->fh_maxsize == NFS4_FHSIZE);
> > > - struct inode *inode;
> > > struct kstat stat;
> > > __be32 err;
> > >
> > > if (fhp->fh_no_wcc || fhp->fh_pre_saved)
> > > return nfs_ok;
> > >
> > > - inode = d_inode(fhp->fh_dentry);
> > > err = fh_getattr(fhp, &stat);
> > > if (err)
> > > return err;
> > >
> > > if (v4)
> > > - fhp->fh_pre_change = nfsd4_change_attribute(&stat, inode);
> > > + fhp->fh_pre_change = nfsd4_change_attribute(&stat);
> > >
> > > fhp->fh_pre_mtime = stat.mtime;
> > > fhp->fh_pre_ctime = stat.ctime;
> > > @@ -700,7 +698,6 @@ __be32 __must_check fh_fill_pre_attrs(struct svc_fh *fhp)
> > > __be32 fh_fill_post_attrs(struct svc_fh *fhp)
> > > {
> > > bool v4 = (fhp->fh_maxsize == NFS4_FHSIZE);
> > > - struct inode *inode = d_inode(fhp->fh_dentry);
> > > __be32 err;
> > >
> > > if (fhp->fh_no_wcc)
> > > @@ -716,7 +713,7 @@ __be32 fh_fill_post_attrs(struct svc_fh *fhp)
> > > fhp->fh_post_saved = true;
> > > if (v4)
> > > fhp->fh_post_change =
> > > - nfsd4_change_attribute(&fhp->fh_post_attr, inode);
> > > + nfsd4_change_attribute(&fhp->fh_post_attr);
> > > return nfs_ok;
> > > }
> > >
> > > @@ -824,13 +821,13 @@ enum fsid_source fsid_source(const struct svc_fh *fhp)
> > > * assume that the new change attr is always logged to stable storage in some
> > > * fashion before the results can be seen.
> > > */
> > > -u64 nfsd4_change_attribute(const struct kstat *stat, const struct inode *inode)
> > > +u64 nfsd4_change_attribute(const struct kstat *stat)
> > > {
> > > u64 chattr;
> > >
> > > if (stat->result_mask & STATX_CHANGE_COOKIE) {
> > > chattr = stat->change_cookie;
> > > - if (S_ISREG(inode->i_mode) &&
> > > + if (S_ISREG(stat->mode) &&
> > > !(stat->attributes & STATX_ATTR_CHANGE_MONOTONIC)) {
> > > chattr += (u64)stat->ctime.tv_sec << 30;
> > > chattr += stat->ctime.tv_nsec;
> > > diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
> > > index 5b7394801dc4270dbd5236f3e2f2237130c73dad..876152a91f122f83fb94ffdfb0eedf8fca56a20c 100644
> > > --- a/fs/nfsd/nfsfh.h
> > > +++ b/fs/nfsd/nfsfh.h
> > > @@ -297,8 +297,7 @@ static inline void fh_clear_pre_post_attrs(struct svc_fh *fhp)
> > > fhp->fh_pre_saved = false;
> > > }
> > >
> > > -u64 nfsd4_change_attribute(const struct kstat *stat,
> > > - const struct inode *inode);
> > > +u64 nfsd4_change_attribute(const struct kstat *stat);
> > > __be32 __must_check fh_fill_pre_attrs(struct svc_fh *fhp);
> > > __be32 fh_fill_post_attrs(struct svc_fh *fhp);
> > > __be32 __must_check fh_fill_both_attrs(struct svc_fh *fhp);
> > >
> > > --
> > > 2.47.0
> > >
> >
>
> --
> Jeff Layton <jlayton@kernel.org>
--
Chuck Lever
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/6] nfsd: switch to autogenerated definitions for open_delegation_type4
2024-10-14 19:26 [PATCH 0/6] nfsd: update the delstid patches for latest draft changes Jeff Layton
2024-10-14 19:26 ` [PATCH 1/6] nfsd: drop inode parameter from nfsd4_change_attribute() Jeff Layton
@ 2024-10-14 19:26 ` Jeff Layton
2024-10-14 19:26 ` [PATCH 3/6] nfsd: rename NFS4_SHARE_WANT_* constants to OPEN4_SHARE_ACCESS_WANT_* Jeff Layton
` (5 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Jeff Layton @ 2024-10-14 19:26 UTC (permalink / raw)
To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Jonathan Corbet, Trond Myklebust, Anna Schumaker, Thomas Haynes
Cc: linux-nfs, linux-kernel, linux-doc, Jeff Layton
Rename the enum with the same name in include/linux/nfs4.h, add the
proper enum to nfs4_1.x and regenerate the headers and source files. Do
a mass rename of all NFS4_OPEN_DELEGATE_* to OPEN_DELEGATE_* in the nfsd
directory.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Documentation/sunrpc/xdr/nfs4_1.x | 9 ++++++++-
fs/nfsd/nfs4state.c | 38 ++++++++++++++++++------------------
fs/nfsd/nfs4xdr.c | 8 ++++----
fs/nfsd/nfs4xdr_gen.c | 19 +++++++++++++++++-
fs/nfsd/nfs4xdr_gen.h | 2 +-
include/linux/nfs4.h | 2 +-
include/linux/sunrpc/xdrgen/nfs4_1.h | 13 +++++++++++-
7 files changed, 63 insertions(+), 28 deletions(-)
diff --git a/Documentation/sunrpc/xdr/nfs4_1.x b/Documentation/sunrpc/xdr/nfs4_1.x
index fc37d1ecba0f40e46c6986df90d07a0e6e6ae9b2..ee9f8f249f1e71dbfc383007a6950ebc4104ed67 100644
--- a/Documentation/sunrpc/xdr/nfs4_1.x
+++ b/Documentation/sunrpc/xdr/nfs4_1.x
@@ -161,6 +161,13 @@ pragma public fattr4_time_deleg_modify;
const FATTR4_TIME_DELEG_ACCESS = 84;
const FATTR4_TIME_DELEG_MODIFY = 85;
-
const OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS = 0x100000;
+enum open_delegation_type4 {
+ OPEN_DELEGATE_NONE = 0,
+ OPEN_DELEGATE_READ = 1,
+ OPEN_DELEGATE_WRITE = 2,
+ OPEN_DELEGATE_NONE_EXT = 3, /* new to v4.1 */
+ OPEN_DELEGATE_READ_ATTRS_DELEG = 4,
+ OPEN_DELEGATE_WRITE_ATTRS_DELEG = 5
+};
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 2961a277a79c1f4cdb8c29a7c19abcb3305b61a1..c8b4c2b9135128f98800bc525563503bff2d2ed2 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2852,7 +2852,7 @@ static int nfs4_show_deleg(struct seq_file *s, struct nfs4_stid *st)
seq_puts(s, ": { type: deleg, ");
seq_printf(s, "access: %s",
- ds->dl_type == NFS4_OPEN_DELEGATE_READ ? "r" : "w");
+ ds->dl_type == OPEN_DELEGATE_READ ? "r" : "w");
/* XXX: lease time, whether it's being recalled. */
@@ -5433,7 +5433,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
static inline __be32
nfs4_check_delegmode(struct nfs4_delegation *dp, int flags)
{
- if ((flags & WR_STATE) && (dp->dl_type == NFS4_OPEN_DELEGATE_READ))
+ if ((flags & WR_STATE) && (dp->dl_type == OPEN_DELEGATE_READ))
return nfserr_openmode;
else
return nfs_ok;
@@ -5675,7 +5675,7 @@ static struct file_lease *nfs4_alloc_init_lease(struct nfs4_delegation *dp,
return NULL;
fl->fl_lmops = &nfsd_lease_mng_ops;
fl->c.flc_flags = FL_DELEG;
- fl->c.flc_type = flag == NFS4_OPEN_DELEGATE_READ? F_RDLCK: F_WRLCK;
+ fl->c.flc_type = flag == OPEN_DELEGATE_READ ? F_RDLCK : F_WRLCK;
fl->c.flc_owner = (fl_owner_t)dp;
fl->c.flc_pid = current->tgid;
fl->c.flc_file = dp->dl_stid.sc_file->fi_deleg_file->nf_file;
@@ -5821,7 +5821,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
*/
if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_BOTH) {
nf = find_rw_file(fp);
- dl_type = NFS4_OPEN_DELEGATE_WRITE;
+ dl_type = OPEN_DELEGATE_WRITE;
}
/*
@@ -5830,7 +5830,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
*/
if (!nf && (open->op_share_access & NFS4_SHARE_ACCESS_READ)) {
nf = find_readable_file(fp);
- dl_type = NFS4_OPEN_DELEGATE_READ;
+ dl_type = OPEN_DELEGATE_READ;
}
if (!nf)
@@ -5919,7 +5919,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
{
- open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
+ open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
if (status == -EAGAIN)
open->op_why_no_deleg = WND4_CONTENTION;
else {
@@ -6035,20 +6035,20 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
destroy_delegation(dp);
goto out_no_deleg;
}
- open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE;
+ open->op_delegate_type = OPEN_DELEGATE_WRITE;
dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
} else {
- open->op_delegate_type = NFS4_OPEN_DELEGATE_READ;
+ open->op_delegate_type = OPEN_DELEGATE_READ;
trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
}
nfs4_put_stid(&dp->dl_stid);
return;
out_no_deleg:
- open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE;
+ open->op_delegate_type = OPEN_DELEGATE_NONE;
if (open->op_claim_type == NFS4_OPEN_CLAIM_PREVIOUS &&
- open->op_delegate_type != NFS4_OPEN_DELEGATE_NONE) {
+ open->op_delegate_type != OPEN_DELEGATE_NONE) {
dprintk("NFSD: WARNING: refusing delegation reclaim\n");
open->op_recall = true;
}
@@ -6063,17 +6063,17 @@ static void nfsd4_deleg_xgrade_none_ext(struct nfsd4_open *open,
struct nfs4_delegation *dp)
{
if (open->op_deleg_want == NFS4_SHARE_WANT_READ_DELEG &&
- dp->dl_type == NFS4_OPEN_DELEGATE_WRITE) {
- open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
+ dp->dl_type == OPEN_DELEGATE_WRITE) {
+ open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
open->op_why_no_deleg = WND4_NOT_SUPP_DOWNGRADE;
} else if (open->op_deleg_want == NFS4_SHARE_WANT_WRITE_DELEG &&
- dp->dl_type == NFS4_OPEN_DELEGATE_WRITE) {
- open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
+ dp->dl_type == OPEN_DELEGATE_WRITE) {
+ open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
open->op_why_no_deleg = WND4_NOT_SUPP_UPGRADE;
}
/* Otherwise the client must be confused wanting a delegation
* it already has, therefore we don't return
- * NFS4_OPEN_DELEGATE_NONE_EXT and reason.
+ * OPEN_DELEGATE_NONE_EXT and reason.
*/
}
@@ -6082,8 +6082,8 @@ static bool open_xor_delegation(struct nfsd4_open *open)
{
if (!(open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION))
return false;
- if (open->op_delegate_type != NFS4_OPEN_DELEGATE_READ &&
- open->op_delegate_type != NFS4_OPEN_DELEGATE_WRITE)
+ if (open->op_delegate_type != OPEN_DELEGATE_READ &&
+ open->op_delegate_type != OPEN_DELEGATE_WRITE)
return false;
return true;
}
@@ -6169,7 +6169,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
if (nfsd4_has_session(&resp->cstate)) {
if (open->op_deleg_want & NFS4_SHARE_WANT_NO_DELEG) {
- open->op_delegate_type = NFS4_OPEN_DELEGATE_NONE_EXT;
+ open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
open->op_why_no_deleg = WND4_NOT_WANTED;
goto nodeleg;
}
@@ -6196,7 +6196,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
trace_nfsd_open(&stp->st_stid.sc_stateid);
out:
/* 4.1 client trying to upgrade/downgrade delegation? */
- if (open->op_delegate_type == NFS4_OPEN_DELEGATE_NONE && dp &&
+ if (open->op_delegate_type == OPEN_DELEGATE_NONE && dp &&
open->op_deleg_want)
nfsd4_deleg_xgrade_none_ext(open, dp);
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index da7ec663da7326ad5c68a9c738b12d09cfcdc65a..682fc6a32c8562bbd8531458da9b7ff1de69bcd1 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4292,18 +4292,18 @@ nfsd4_encode_open_delegation4(struct xdr_stream *xdr, struct nfsd4_open *open)
if (xdr_stream_encode_u32(xdr, open->op_delegate_type) != XDR_UNIT)
return nfserr_resource;
switch (open->op_delegate_type) {
- case NFS4_OPEN_DELEGATE_NONE:
+ case OPEN_DELEGATE_NONE:
status = nfs_ok;
break;
- case NFS4_OPEN_DELEGATE_READ:
+ case OPEN_DELEGATE_READ:
/* read */
status = nfsd4_encode_open_read_delegation4(xdr, open);
break;
- case NFS4_OPEN_DELEGATE_WRITE:
+ case OPEN_DELEGATE_WRITE:
/* write */
status = nfsd4_encode_open_write_delegation4(xdr, open);
break;
- case NFS4_OPEN_DELEGATE_NONE_EXT:
+ case OPEN_DELEGATE_NONE_EXT:
/* od_whynone */
status = nfsd4_encode_open_none_delegation4(xdr, open);
break;
diff --git a/fs/nfsd/nfs4xdr_gen.c b/fs/nfsd/nfs4xdr_gen.c
index e5d34f9a3147d9d51fb3b9db4c29b048b1083cbf..a0e01f50a28d7f6828f3e6ef02f90b84bf180841 100644
--- a/fs/nfsd/nfs4xdr_gen.c
+++ b/fs/nfsd/nfs4xdr_gen.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
// Generated by xdrgen. Manual edits will be lost.
// XDR specification file: ../../Documentation/sunrpc/xdr/nfs4_1.x
-// XDR specification modification time: Thu Oct 3 11:30:59 2024
+// XDR specification modification time: Sat Oct 12 08:10:54 2024
#include <linux/sunrpc/svc.h>
@@ -135,6 +135,17 @@ xdrgen_decode_fattr4_time_deleg_modify(struct xdr_stream *xdr, fattr4_time_deleg
return xdrgen_decode_nfstime4(xdr, ptr);
};
+static bool __maybe_unused
+xdrgen_decode_open_delegation_type4(struct xdr_stream *xdr, open_delegation_type4 *ptr)
+{
+ u32 val;
+
+ if (xdr_stream_decode_u32(xdr, &val) < 0)
+ return false;
+ *ptr = val;
+ return true;
+}
+
static bool __maybe_unused
xdrgen_encode_int64_t(struct xdr_stream *xdr, const int64_t value)
{
@@ -237,3 +248,9 @@ xdrgen_encode_fattr4_time_deleg_modify(struct xdr_stream *xdr, const fattr4_time
{
return xdrgen_encode_nfstime4(xdr, value);
};
+
+static bool __maybe_unused
+xdrgen_encode_open_delegation_type4(struct xdr_stream *xdr, open_delegation_type4 value)
+{
+ return xdr_stream_encode_u32(xdr, value) == XDR_UNIT;
+}
diff --git a/fs/nfsd/nfs4xdr_gen.h b/fs/nfsd/nfs4xdr_gen.h
index c4c6a5075b17be3f931e2a20e282e33dc6e10ef1..3fc8bde2b3b5db6f80f17b41e7f5991487cfa959 100644
--- a/fs/nfsd/nfs4xdr_gen.h
+++ b/fs/nfsd/nfs4xdr_gen.h
@@ -1,7 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0 */
/* Generated by xdrgen. Manual edits will be lost. */
/* XDR specification file: ../../Documentation/sunrpc/xdr/nfs4_1.x */
-/* XDR specification modification time: Thu Oct 3 11:30:59 2024 */
+/* XDR specification modification time: Sat Oct 12 08:10:54 2024 */
#ifndef _LINUX_XDRGEN_NFS4_1_DECL_H
#define _LINUX_XDRGEN_NFS4_1_DECL_H
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index b907192447755a614289554a01928c1ebb61c3dc..71fbebfa43c7e2bd27708814c7300c506ce64c1b 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -366,7 +366,7 @@ enum limit_by4 {
NFS4_LIMIT_BLOCKS = 2
};
-enum open_delegation_type4 {
+enum nfs4_open_delegation_type4 {
NFS4_OPEN_DELEGATE_NONE = 0,
NFS4_OPEN_DELEGATE_READ = 1,
NFS4_OPEN_DELEGATE_WRITE = 2,
diff --git a/include/linux/sunrpc/xdrgen/nfs4_1.h b/include/linux/sunrpc/xdrgen/nfs4_1.h
index 6025ab6b739833aad33567102e216c162003f408..9ca83a4a04cff8ebb5aafa08a24a2db771d6c1ef 100644
--- a/include/linux/sunrpc/xdrgen/nfs4_1.h
+++ b/include/linux/sunrpc/xdrgen/nfs4_1.h
@@ -1,7 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0 */
/* Generated by xdrgen. Manual edits will be lost. */
/* XDR specification file: ../../Documentation/sunrpc/xdr/nfs4_1.x */
-/* XDR specification modification time: Thu Oct 3 11:30:59 2024 */
+/* XDR specification modification time: Sat Oct 12 08:10:54 2024 */
#ifndef _LINUX_XDRGEN_NFS4_1_DEF_H
#define _LINUX_XDRGEN_NFS4_1_DEF_H
@@ -98,6 +98,16 @@ enum { FATTR4_TIME_DELEG_MODIFY = 85 };
enum { OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS = 0x100000 };
+enum open_delegation_type4 {
+ OPEN_DELEGATE_NONE = 0,
+ OPEN_DELEGATE_READ = 1,
+ OPEN_DELEGATE_WRITE = 2,
+ OPEN_DELEGATE_NONE_EXT = 3,
+ OPEN_DELEGATE_READ_ATTRS_DELEG = 4,
+ OPEN_DELEGATE_WRITE_ATTRS_DELEG = 5,
+};
+typedef enum open_delegation_type4 open_delegation_type4;
+
#define NFS4_int64_t_sz \
(XDR_hyper)
#define NFS4_uint32_t_sz \
@@ -120,5 +130,6 @@ enum { OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS = 0x100000 };
(NFS4_nfstime4_sz)
#define NFS4_fattr4_time_deleg_modify_sz \
(NFS4_nfstime4_sz)
+#define NFS4_open_delegation_type4_sz (XDR_int)
#endif /* _LINUX_XDRGEN_NFS4_1_DEF_H */
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 3/6] nfsd: rename NFS4_SHARE_WANT_* constants to OPEN4_SHARE_ACCESS_WANT_*
2024-10-14 19:26 [PATCH 0/6] nfsd: update the delstid patches for latest draft changes Jeff Layton
2024-10-14 19:26 ` [PATCH 1/6] nfsd: drop inode parameter from nfsd4_change_attribute() Jeff Layton
2024-10-14 19:26 ` [PATCH 2/6] nfsd: switch to autogenerated definitions for open_delegation_type4 Jeff Layton
@ 2024-10-14 19:26 ` Jeff Layton
2024-10-14 19:26 ` [PATCH 4/6] nfsd: prepare delegation code for handing out *_ATTRS_DELEG delegations Jeff Layton
` (4 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Jeff Layton @ 2024-10-14 19:26 UTC (permalink / raw)
To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Jonathan Corbet, Trond Myklebust, Anna Schumaker, Thomas Haynes
Cc: linux-nfs, linux-kernel, linux-doc, Jeff Layton
Add the OPEN4_SHARE_ACCESS_WANT constants from the nfs4.1 and delstid
draft into the nfs4_1.x file, and regenerate the headers and source
files. Do a mass renaming of NFS4_SHARE_WANT_* to
OPEN4_SHARE_ACCESS_WANT_* in the nfsd directory.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Documentation/sunrpc/xdr/nfs4_1.x | 15 ++++++++++++++-
fs/nfsd/nfs4state.c | 16 ++++++++--------
fs/nfsd/nfs4xdr.c | 12 ++++++------
fs/nfsd/nfs4xdr_gen.c | 2 +-
fs/nfsd/nfs4xdr_gen.h | 2 +-
include/linux/sunrpc/xdrgen/nfs4_1.h | 24 +++++++++++++++++++++---
6 files changed, 51 insertions(+), 20 deletions(-)
diff --git a/Documentation/sunrpc/xdr/nfs4_1.x b/Documentation/sunrpc/xdr/nfs4_1.x
index ee9f8f249f1e71dbfc383007a6950ebc4104ed67..ca95150a3a29fc5418991bf2395326bd73645ea8 100644
--- a/Documentation/sunrpc/xdr/nfs4_1.x
+++ b/Documentation/sunrpc/xdr/nfs4_1.x
@@ -138,7 +138,6 @@ pragma public fattr4_open_arguments;
const FATTR4_OPEN_ARGUMENTS = 86;
-const OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION = 0x200000;
const OPEN4_RESULT_NO_OPEN_STATEID = 0x00000010;
@@ -161,7 +160,21 @@ pragma public fattr4_time_deleg_modify;
const FATTR4_TIME_DELEG_ACCESS = 84;
const FATTR4_TIME_DELEG_MODIFY = 85;
+
+
+/* new flags for share_access field of OPEN4args */
+const OPEN4_SHARE_ACCESS_WANT_DELEG_MASK = 0xFF00;
+const OPEN4_SHARE_ACCESS_WANT_NO_PREFERENCE = 0x0000;
+const OPEN4_SHARE_ACCESS_WANT_READ_DELEG = 0x0100;
+const OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG = 0x0200;
+const OPEN4_SHARE_ACCESS_WANT_ANY_DELEG = 0x0300;
+const OPEN4_SHARE_ACCESS_WANT_NO_DELEG = 0x0400;
+const OPEN4_SHARE_ACCESS_WANT_CANCEL = 0x0500;
+
+const OPEN4_SHARE_ACCESS_WANT_SIGNAL_DELEG_WHEN_RESRC_AVAIL = 0x10000;
+const OPEN4_SHARE_ACCESS_WANT_PUSH_DELEG_WHEN_UNCONTENDED = 0x20000;
const OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS = 0x100000;
+const OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION = 0x200000;
enum open_delegation_type4 {
OPEN_DELEGATE_NONE = 0,
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c8b4c2b9135128f98800bc525563503bff2d2ed2..fe74d8c0c61e76f635a3133a4c71b7fb7b622a48 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5925,14 +5925,14 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
else {
open->op_why_no_deleg = WND4_RESOURCE;
switch (open->op_deleg_want) {
- case NFS4_SHARE_WANT_READ_DELEG:
- case NFS4_SHARE_WANT_WRITE_DELEG:
- case NFS4_SHARE_WANT_ANY_DELEG:
+ case OPEN4_SHARE_ACCESS_WANT_READ_DELEG:
+ case OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG:
+ case OPEN4_SHARE_ACCESS_WANT_ANY_DELEG:
break;
- case NFS4_SHARE_WANT_CANCEL:
+ case OPEN4_SHARE_ACCESS_WANT_CANCEL:
open->op_why_no_deleg = WND4_CANCELLED;
break;
- case NFS4_SHARE_WANT_NO_DELEG:
+ case OPEN4_SHARE_ACCESS_WANT_NO_DELEG:
WARN_ON_ONCE(1);
}
}
@@ -6062,11 +6062,11 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
static void nfsd4_deleg_xgrade_none_ext(struct nfsd4_open *open,
struct nfs4_delegation *dp)
{
- if (open->op_deleg_want == NFS4_SHARE_WANT_READ_DELEG &&
+ if (open->op_deleg_want == OPEN4_SHARE_ACCESS_WANT_READ_DELEG &&
dp->dl_type == OPEN_DELEGATE_WRITE) {
open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
open->op_why_no_deleg = WND4_NOT_SUPP_DOWNGRADE;
- } else if (open->op_deleg_want == NFS4_SHARE_WANT_WRITE_DELEG &&
+ } else if (open->op_deleg_want == OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG &&
dp->dl_type == OPEN_DELEGATE_WRITE) {
open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
open->op_why_no_deleg = WND4_NOT_SUPP_UPGRADE;
@@ -6168,7 +6168,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
mutex_unlock(&stp->st_mutex);
if (nfsd4_has_session(&resp->cstate)) {
- if (open->op_deleg_want & NFS4_SHARE_WANT_NO_DELEG) {
+ if (open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_NO_DELEG) {
open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
open->op_why_no_deleg = WND4_NOT_WANTED;
goto nodeleg;
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 682fc6a32c8562bbd8531458da9b7ff1de69bcd1..e95f6ba5cc65611b47d5d297584ff6e478d80a1f 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -1068,12 +1068,12 @@ static __be32 nfsd4_decode_share_access(struct nfsd4_compoundargs *argp, u32 *sh
if (!argp->minorversion)
return nfserr_bad_xdr;
switch (w & NFS4_SHARE_WANT_TYPE_MASK) {
- case NFS4_SHARE_WANT_NO_PREFERENCE:
- case NFS4_SHARE_WANT_READ_DELEG:
- case NFS4_SHARE_WANT_WRITE_DELEG:
- case NFS4_SHARE_WANT_ANY_DELEG:
- case NFS4_SHARE_WANT_NO_DELEG:
- case NFS4_SHARE_WANT_CANCEL:
+ case OPEN4_SHARE_ACCESS_WANT_NO_PREFERENCE:
+ case OPEN4_SHARE_ACCESS_WANT_READ_DELEG:
+ case OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG:
+ case OPEN4_SHARE_ACCESS_WANT_ANY_DELEG:
+ case OPEN4_SHARE_ACCESS_WANT_NO_DELEG:
+ case OPEN4_SHARE_ACCESS_WANT_CANCEL:
break;
default:
return nfserr_bad_xdr;
diff --git a/fs/nfsd/nfs4xdr_gen.c b/fs/nfsd/nfs4xdr_gen.c
index a0e01f50a28d7f6828f3e6ef02f90b84bf180841..a17b5d8e60b3579caa2e2a8b40ed757070e1a622 100644
--- a/fs/nfsd/nfs4xdr_gen.c
+++ b/fs/nfsd/nfs4xdr_gen.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
// Generated by xdrgen. Manual edits will be lost.
// XDR specification file: ../../Documentation/sunrpc/xdr/nfs4_1.x
-// XDR specification modification time: Sat Oct 12 08:10:54 2024
+// XDR specification modification time: Mon Oct 14 09:10:13 2024
#include <linux/sunrpc/svc.h>
diff --git a/fs/nfsd/nfs4xdr_gen.h b/fs/nfsd/nfs4xdr_gen.h
index 3fc8bde2b3b5db6f80f17b41e7f5991487cfa959..41a0033b72562ee3c1fcdcd4a887ce635385b22b 100644
--- a/fs/nfsd/nfs4xdr_gen.h
+++ b/fs/nfsd/nfs4xdr_gen.h
@@ -1,7 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0 */
/* Generated by xdrgen. Manual edits will be lost. */
/* XDR specification file: ../../Documentation/sunrpc/xdr/nfs4_1.x */
-/* XDR specification modification time: Sat Oct 12 08:10:54 2024 */
+/* XDR specification modification time: Mon Oct 14 09:10:13 2024 */
#ifndef _LINUX_XDRGEN_NFS4_1_DECL_H
#define _LINUX_XDRGEN_NFS4_1_DECL_H
diff --git a/include/linux/sunrpc/xdrgen/nfs4_1.h b/include/linux/sunrpc/xdrgen/nfs4_1.h
index 9ca83a4a04cff8ebb5aafa08a24a2db771d6c1ef..cf21a14aa8850f4b21cd365cb7bc22a02c6097ce 100644
--- a/include/linux/sunrpc/xdrgen/nfs4_1.h
+++ b/include/linux/sunrpc/xdrgen/nfs4_1.h
@@ -1,7 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0 */
/* Generated by xdrgen. Manual edits will be lost. */
/* XDR specification file: ../../Documentation/sunrpc/xdr/nfs4_1.x */
-/* XDR specification modification time: Sat Oct 12 08:10:54 2024 */
+/* XDR specification modification time: Mon Oct 14 09:10:13 2024 */
#ifndef _LINUX_XDRGEN_NFS4_1_DEF_H
#define _LINUX_XDRGEN_NFS4_1_DEF_H
@@ -84,8 +84,6 @@ typedef struct open_arguments4 fattr4_open_arguments;
enum { FATTR4_OPEN_ARGUMENTS = 86 };
-enum { OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION = 0x200000 };
-
enum { OPEN4_RESULT_NO_OPEN_STATEID = 0x00000010 };
typedef struct nfstime4 fattr4_time_deleg_access;
@@ -96,8 +94,28 @@ enum { FATTR4_TIME_DELEG_ACCESS = 84 };
enum { FATTR4_TIME_DELEG_MODIFY = 85 };
+enum { OPEN4_SHARE_ACCESS_WANT_DELEG_MASK = 0xFF00 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_NO_PREFERENCE = 0x0000 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_READ_DELEG = 0x0100 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG = 0x0200 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_ANY_DELEG = 0x0300 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_NO_DELEG = 0x0400 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_CANCEL = 0x0500 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_SIGNAL_DELEG_WHEN_RESRC_AVAIL = 0x10000 };
+
+enum { OPEN4_SHARE_ACCESS_WANT_PUSH_DELEG_WHEN_UNCONTENDED = 0x20000 };
+
enum { OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS = 0x100000 };
+enum { OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION = 0x200000 };
+
enum open_delegation_type4 {
OPEN_DELEGATE_NONE = 0,
OPEN_DELEGATE_READ = 1,
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 4/6] nfsd: prepare delegation code for handing out *_ATTRS_DELEG delegations
2024-10-14 19:26 [PATCH 0/6] nfsd: update the delstid patches for latest draft changes Jeff Layton
` (2 preceding siblings ...)
2024-10-14 19:26 ` [PATCH 3/6] nfsd: rename NFS4_SHARE_WANT_* constants to OPEN4_SHARE_ACCESS_WANT_* Jeff Layton
@ 2024-10-14 19:26 ` Jeff Layton
2024-10-14 19:26 ` [PATCH 5/6] nfsd: add support for delegated timestamps Jeff Layton
` (3 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: Jeff Layton @ 2024-10-14 19:26 UTC (permalink / raw)
To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Jonathan Corbet, Trond Myklebust, Anna Schumaker, Thomas Haynes
Cc: linux-nfs, linux-kernel, linux-doc, Jeff Layton
Add some preparatory code to various functions that handle delegation
types to allow them to handle the OPEN_DELEGATE_*_ATTRS_DELEG constants.
Add helpers for detecting whether it's a read or write deleg, and
whether the attributes are delegated.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
fs/nfsd/nfs4state.c | 50 +++++++++++++++++++++++++++++++++-----------------
fs/nfsd/nfs4xdr.c | 2 ++
fs/nfsd/state.h | 16 ++++++++++++++++
3 files changed, 51 insertions(+), 17 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index fe74d8c0c61e76f635a3133a4c71b7fb7b622a48..62f9aeb159d0f2ab4d293bf5c0c56ad7b86eb9d6 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2838,6 +2838,21 @@ static int nfs4_show_lock(struct seq_file *s, struct nfs4_stid *st)
return 0;
}
+static char *nfs4_show_deleg_type(u32 dl_type)
+{
+ switch (dl_type) {
+ case OPEN_DELEGATE_READ:
+ return "r";
+ case OPEN_DELEGATE_WRITE:
+ return "w";
+ case OPEN_DELEGATE_READ_ATTRS_DELEG:
+ return "ra";
+ case OPEN_DELEGATE_WRITE_ATTRS_DELEG:
+ return "wa";
+ }
+ return "?";
+}
+
static int nfs4_show_deleg(struct seq_file *s, struct nfs4_stid *st)
{
struct nfs4_delegation *ds;
@@ -2851,8 +2866,7 @@ static int nfs4_show_deleg(struct seq_file *s, struct nfs4_stid *st)
nfs4_show_stateid(s, &st->sc_stateid);
seq_puts(s, ": { type: deleg, ");
- seq_printf(s, "access: %s",
- ds->dl_type == OPEN_DELEGATE_READ ? "r" : "w");
+ seq_printf(s, "access: %s", nfs4_show_deleg_type(ds->dl_type));
/* XXX: lease time, whether it's being recalled. */
@@ -5433,7 +5447,7 @@ nfsd4_process_open1(struct nfsd4_compound_state *cstate,
static inline __be32
nfs4_check_delegmode(struct nfs4_delegation *dp, int flags)
{
- if ((flags & WR_STATE) && (dp->dl_type == OPEN_DELEGATE_READ))
+ if ((flags & WR_STATE) && deleg_is_read(dp->dl_type))
return nfserr_openmode;
else
return nfs_ok;
@@ -5665,8 +5679,7 @@ static bool nfsd4_cb_channel_good(struct nfs4_client *clp)
return clp->cl_minorversion && clp->cl_cb_state == NFSD4_CB_UNKNOWN;
}
-static struct file_lease *nfs4_alloc_init_lease(struct nfs4_delegation *dp,
- int flag)
+static struct file_lease *nfs4_alloc_init_lease(struct nfs4_delegation *dp)
{
struct file_lease *fl;
@@ -5675,7 +5688,7 @@ static struct file_lease *nfs4_alloc_init_lease(struct nfs4_delegation *dp,
return NULL;
fl->fl_lmops = &nfsd_lease_mng_ops;
fl->c.flc_flags = FL_DELEG;
- fl->c.flc_type = flag == OPEN_DELEGATE_READ ? F_RDLCK : F_WRLCK;
+ fl->c.flc_type = deleg_is_read(dp->dl_type) ? F_RDLCK : F_WRLCK;
fl->c.flc_owner = (fl_owner_t)dp;
fl->c.flc_pid = current->tgid;
fl->c.flc_file = dp->dl_stid.sc_file->fi_deleg_file->nf_file;
@@ -5862,7 +5875,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
if (!dp)
goto out_delegees;
- fl = nfs4_alloc_init_lease(dp, dl_type);
+ fl = nfs4_alloc_init_lease(dp);
if (!fl)
goto out_clnt_odstate;
@@ -6062,14 +6075,14 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
static void nfsd4_deleg_xgrade_none_ext(struct nfsd4_open *open,
struct nfs4_delegation *dp)
{
- if (open->op_deleg_want == OPEN4_SHARE_ACCESS_WANT_READ_DELEG &&
- dp->dl_type == OPEN_DELEGATE_WRITE) {
- open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
- open->op_why_no_deleg = WND4_NOT_SUPP_DOWNGRADE;
- } else if (open->op_deleg_want == OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG &&
- dp->dl_type == OPEN_DELEGATE_WRITE) {
- open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
- open->op_why_no_deleg = WND4_NOT_SUPP_UPGRADE;
+ if (deleg_is_write(dp->dl_type)) {
+ if (open->op_deleg_want == OPEN4_SHARE_ACCESS_WANT_READ_DELEG) {
+ open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
+ open->op_why_no_deleg = WND4_NOT_SUPP_DOWNGRADE;
+ } else if (open->op_deleg_want == OPEN4_SHARE_ACCESS_WANT_WRITE_DELEG) {
+ open->op_delegate_type = OPEN_DELEGATE_NONE_EXT;
+ open->op_why_no_deleg = WND4_NOT_SUPP_UPGRADE;
+ }
}
/* Otherwise the client must be confused wanting a delegation
* it already has, therefore we don't return
@@ -6080,11 +6093,14 @@ static void nfsd4_deleg_xgrade_none_ext(struct nfsd4_open *open,
/* Are we only returning a delegation stateid? */
static bool open_xor_delegation(struct nfsd4_open *open)
{
+ /* Was one requested? */
if (!(open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION))
return false;
- if (open->op_delegate_type != OPEN_DELEGATE_READ &&
- open->op_delegate_type != OPEN_DELEGATE_WRITE)
+
+ /* Did we actually get a delegation? */
+ if (!deleg_is_read(open->op_delegate_type) && !deleg_is_write(open->op_delegate_type))
return false;
+
return true;
}
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index e95f6ba5cc65611b47d5d297584ff6e478d80a1f..1c9d9349e4447c0078c7de0d533cf6278941679d 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4296,10 +4296,12 @@ nfsd4_encode_open_delegation4(struct xdr_stream *xdr, struct nfsd4_open *open)
status = nfs_ok;
break;
case OPEN_DELEGATE_READ:
+ case OPEN_DELEGATE_READ_ATTRS_DELEG:
/* read */
status = nfsd4_encode_open_read_delegation4(xdr, open);
break;
case OPEN_DELEGATE_WRITE:
+ case OPEN_DELEGATE_WRITE_ATTRS_DELEG:
/* write */
status = nfsd4_encode_open_write_delegation4(xdr, open);
break;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index c7c7ec21e5104761221bd78b31110d902df1dc9b..9d0e844515aa6ea0ec62f2b538ecc2c6a5e34652 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -190,6 +190,22 @@ struct nfs4_delegation {
struct nfs4_cb_fattr dl_cb_fattr;
};
+static inline bool deleg_is_read(u32 dl_type)
+{
+ return (dl_type == OPEN_DELEGATE_READ || dl_type == OPEN_DELEGATE_READ_ATTRS_DELEG);
+}
+
+static inline bool deleg_is_write(u32 dl_type)
+{
+ return (dl_type == OPEN_DELEGATE_WRITE || dl_type == OPEN_DELEGATE_WRITE_ATTRS_DELEG);
+}
+
+static inline bool deleg_attrs_deleg(u32 dl_type)
+{
+ return dl_type == OPEN_DELEGATE_READ_ATTRS_DELEG ||
+ dl_type == OPEN_DELEGATE_WRITE_ATTRS_DELEG;
+}
+
#define cb_to_delegation(cb) \
container_of(cb, struct nfs4_delegation, dl_recall)
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [PATCH 5/6] nfsd: add support for delegated timestamps
2024-10-14 19:26 [PATCH 0/6] nfsd: update the delstid patches for latest draft changes Jeff Layton
` (3 preceding siblings ...)
2024-10-14 19:26 ` [PATCH 4/6] nfsd: prepare delegation code for handing out *_ATTRS_DELEG delegations Jeff Layton
@ 2024-10-14 19:26 ` Jeff Layton
2024-11-19 1:01 ` NeilBrown
2024-10-14 19:26 ` [PATCH 6/6] nfsd: handle delegated timestamps in SETATTR Jeff Layton
` (2 subsequent siblings)
7 siblings, 1 reply; 18+ messages in thread
From: Jeff Layton @ 2024-10-14 19:26 UTC (permalink / raw)
To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Jonathan Corbet, Trond Myklebust, Anna Schumaker, Thomas Haynes
Cc: linux-nfs, linux-kernel, linux-doc, Jeff Layton
Add support for the delegated timestamps on write delegations. This
allows the server to proxy timestamps from the delegation holder to
other clients that are doing GETATTRs vs. the same inode.
When OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS bit is set in the OPEN
call, set the dl_type to the *_ATTRS_DELEG flavor of delegation.
Add timespec64 fields to nfs4_cb_fattr and decode the timestamps into
those. Vet those timestamps according to the delstid spec and update
the inode attrs if necessary.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
fs/nfsd/nfs4callback.c | 42 +++++++++++++++++++--
fs/nfsd/nfs4state.c | 99 +++++++++++++++++++++++++++++++++++++++++++-------
fs/nfsd/nfs4xdr.c | 13 ++++++-
fs/nfsd/nfsd.h | 2 +
fs/nfsd/state.h | 2 +
fs/nfsd/xdr4cb.h | 10 +++--
include/linux/time64.h | 5 +++
7 files changed, 151 insertions(+), 22 deletions(-)
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 776838bb83e6b707a4df76326cdc68f32daf1755..08245596289a960eb8b2e78df276544e7d3f4ff8 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -42,6 +42,7 @@
#include "trace.h"
#include "xdr4cb.h"
#include "xdr4.h"
+#include "nfs4xdr_gen.h"
#define NFSDDBG_FACILITY NFSDDBG_PROC
@@ -93,12 +94,35 @@ static int decode_cb_fattr4(struct xdr_stream *xdr, uint32_t *bitmap,
{
fattr->ncf_cb_change = 0;
fattr->ncf_cb_fsize = 0;
+ fattr->ncf_cb_atime.tv_sec = 0;
+ fattr->ncf_cb_atime.tv_nsec = 0;
+ fattr->ncf_cb_mtime.tv_sec = 0;
+ fattr->ncf_cb_mtime.tv_nsec = 0;
+
if (bitmap[0] & FATTR4_WORD0_CHANGE)
if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_change) < 0)
return -NFSERR_BAD_XDR;
if (bitmap[0] & FATTR4_WORD0_SIZE)
if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_fsize) < 0)
return -NFSERR_BAD_XDR;
+ if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
+ fattr4_time_deleg_access access;
+
+ if (!xdrgen_decode_fattr4_time_deleg_access(xdr, &access))
+ return -NFSERR_BAD_XDR;
+ fattr->ncf_cb_atime.tv_sec = access.seconds;
+ fattr->ncf_cb_atime.tv_nsec = access.nseconds;
+
+ }
+ if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
+ fattr4_time_deleg_modify modify;
+
+ if (!xdrgen_decode_fattr4_time_deleg_modify(xdr, &modify))
+ return -NFSERR_BAD_XDR;
+ fattr->ncf_cb_mtime.tv_sec = modify.seconds;
+ fattr->ncf_cb_mtime.tv_nsec = modify.nseconds;
+
+ }
return 0;
}
@@ -364,15 +388,21 @@ encode_cb_getattr4args(struct xdr_stream *xdr, struct nfs4_cb_compound_hdr *hdr,
struct nfs4_delegation *dp = container_of(fattr, struct nfs4_delegation, dl_cb_fattr);
struct knfsd_fh *fh = &dp->dl_stid.sc_file->fi_fhandle;
struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
- u32 bmap[1];
+ u32 bmap_size = 1;
+ u32 bmap[3];
bmap[0] = FATTR4_WORD0_SIZE;
if (!ncf->ncf_file_modified)
bmap[0] |= FATTR4_WORD0_CHANGE;
+ if (deleg_attrs_deleg(dp->dl_type)) {
+ bmap[1] = 0;
+ bmap[2] = FATTR4_WORD2_TIME_DELEG_ACCESS | FATTR4_WORD2_TIME_DELEG_MODIFY;
+ bmap_size = 3;
+ }
encode_nfs_cb_opnum4(xdr, OP_CB_GETATTR);
encode_nfs_fh4(xdr, fh);
- encode_bitmap4(xdr, bmap, ARRAY_SIZE(bmap));
+ encode_bitmap4(xdr, bmap, bmap_size);
hdr->nops++;
}
@@ -597,7 +627,7 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
struct nfs4_cb_compound_hdr hdr;
int status;
u32 bitmap[3] = {0};
- u32 attrlen;
+ u32 attrlen, maxlen;
struct nfs4_cb_fattr *ncf =
container_of(cb, struct nfs4_cb_fattr, ncf_getattr);
@@ -616,7 +646,11 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
return -NFSERR_BAD_XDR;
if (xdr_stream_decode_u32(xdr, &attrlen) < 0)
return -NFSERR_BAD_XDR;
- if (attrlen > (sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize)))
+ maxlen = sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize);
+ if (bitmap[2] != 0)
+ maxlen += (sizeof(ncf->ncf_cb_mtime.tv_sec) +
+ sizeof(ncf->ncf_cb_mtime.tv_nsec)) * 2;
+ if (attrlen > maxlen)
return -NFSERR_BAD_XDR;
status = decode_cb_fattr4(xdr, bitmap, ncf);
return status;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 62f9aeb159d0f2ab4d293bf5c0c56ad7b86eb9d6..2c8d2bb5261ad189c6dfb1c4050c23d8cf061325 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5803,13 +5803,14 @@ static struct nfs4_delegation *
nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
struct svc_fh *parent)
{
- int status = 0;
+ bool deleg_ts = open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS;
struct nfs4_client *clp = stp->st_stid.sc_client;
struct nfs4_file *fp = stp->st_stid.sc_file;
struct nfs4_clnt_odstate *odstate = stp->st_clnt_odstate;
struct nfs4_delegation *dp;
struct nfsd_file *nf = NULL;
struct file_lease *fl;
+ int status = 0;
u32 dl_type;
/*
@@ -5834,7 +5835,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
*/
if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_BOTH) {
nf = find_rw_file(fp);
- dl_type = OPEN_DELEGATE_WRITE;
+ dl_type = deleg_ts ? OPEN_DELEGATE_WRITE_ATTRS_DELEG : OPEN_DELEGATE_WRITE;
}
/*
@@ -5843,7 +5844,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
*/
if (!nf && (open->op_share_access & NFS4_SHARE_ACCESS_READ)) {
nf = find_readable_file(fp);
- dl_type = OPEN_DELEGATE_READ;
+ dl_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG : OPEN_DELEGATE_READ;
}
if (!nf)
@@ -6001,13 +6002,14 @@ static void
nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
struct svc_fh *currentfh)
{
- struct nfs4_delegation *dp;
+ bool deleg_ts = open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS;
struct nfs4_openowner *oo = openowner(stp->st_stateowner);
struct nfs4_client *clp = stp->st_stid.sc_client;
struct svc_fh *parent = NULL;
- int cb_up;
- int status = 0;
+ struct nfs4_delegation *dp;
struct kstat stat;
+ int status = 0;
+ int cb_up;
cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
open->op_recall = false;
@@ -6048,12 +6050,14 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
destroy_delegation(dp);
goto out_no_deleg;
}
- open->op_delegate_type = OPEN_DELEGATE_WRITE;
+ open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_WRITE_ATTRS_DELEG :
+ OPEN_DELEGATE_WRITE;
dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
} else {
- open->op_delegate_type = OPEN_DELEGATE_READ;
+ open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG :
+ OPEN_DELEGATE_READ;
trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
}
nfs4_put_stid(&dp->dl_stid);
@@ -8887,6 +8891,78 @@ nfsd4_get_writestateid(struct nfsd4_compound_state *cstate,
get_stateid(cstate, &u->write.wr_stateid);
}
+/**
+ * set_cb_time - vet and set the timespec for a cb_getattr update
+ * @cb: timestamp from the CB_GETATTR response
+ * @orig: original timestamp in the inode
+ * @now: current time
+ *
+ * Given a timestamp in a CB_GETATTR response, check it against the
+ * current timestamp in the inode and the current time. Returns true
+ * if the inode's timestamp needs to be updated, and false otherwise.
+ * @cb may also be changed if the timestamp needs to be clamped.
+ */
+static bool set_cb_time(struct timespec64 *cb, const struct timespec64 *orig,
+ const struct timespec64 *now)
+{
+
+ /*
+ * "When the time presented is before the original time, then the
+ * update is ignored." Also no need to update if there is no change.
+ */
+ if (timespec64_compare(cb, orig) <= 0)
+ return false;
+
+ /*
+ * "When the time presented is in the future, the server can either
+ * clamp the new time to the current time, or it may
+ * return NFS4ERR_DELAY to the client, allowing it to retry."
+ */
+ if (timespec64_compare(cb, now) > 0) {
+ /* clamp it */
+ *cb = *now;
+ }
+
+ return true;
+}
+
+static int cb_getattr_update_times(struct dentry *dentry, struct nfs4_delegation *dp)
+{
+ struct inode *inode = d_inode(dentry);
+ struct timespec64 now = current_time(inode);
+ struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
+ struct iattr attrs = { };
+ int ret;
+
+ if (deleg_attrs_deleg(dp->dl_type)) {
+ struct timespec64 atime = inode_get_atime(inode);
+ struct timespec64 mtime = inode_get_mtime(inode);
+
+ attrs.ia_atime = ncf->ncf_cb_atime;
+ attrs.ia_mtime = ncf->ncf_cb_mtime;
+
+ if (set_cb_time(&attrs.ia_atime, &atime, &now))
+ attrs.ia_valid |= ATTR_ATIME | ATTR_ATIME_SET;
+
+ if (set_cb_time(&attrs.ia_mtime, &mtime, &now)) {
+ attrs.ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET;
+ attrs.ia_ctime = attrs.ia_mtime;
+ }
+ } else {
+ attrs.ia_valid |= ATTR_MTIME | ATTR_CTIME;
+ attrs.ia_mtime = attrs.ia_ctime = now;
+ }
+
+ if (!attrs.ia_valid)
+ return 0;
+
+ attrs.ia_valid |= ATTR_DELEG;
+ inode_lock(inode);
+ ret = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
+ inode_unlock(inode);
+ return ret;
+}
+
/**
* nfsd4_deleg_getattr_conflict - Recall if GETATTR causes conflict
* @rqstp: RPC transaction context
@@ -8913,7 +8989,6 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
struct file_lock_context *ctx;
struct nfs4_delegation *dp = NULL;
struct file_lease *fl;
- struct iattr attrs;
struct nfs4_cb_fattr *ncf;
struct inode *inode = d_inode(dentry);
@@ -8975,11 +9050,7 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
* not update the file's metadata with the client's
* modified size
*/
- attrs.ia_mtime = attrs.ia_ctime = current_time(inode);
- attrs.ia_valid = ATTR_MTIME | ATTR_CTIME | ATTR_DELEG;
- inode_lock(inode);
- err = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
- inode_unlock(inode);
+ err = cb_getattr_update_times(dentry, dp);
if (err) {
status = nfserrno(err);
goto out_status;
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 1c9d9349e4447c0078c7de0d533cf6278941679d..0e9f59f6be015bfa37893973f38fec880ff4c0b1 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3409,6 +3409,7 @@ static __be32 nfsd4_encode_fattr4_xattr_support(struct xdr_stream *xdr,
#define NFSD_OA_SHARE_ACCESS_WANT (BIT(OPEN_ARGS_SHARE_ACCESS_WANT_ANY_DELEG) | \
BIT(OPEN_ARGS_SHARE_ACCESS_WANT_NO_DELEG) | \
BIT(OPEN_ARGS_SHARE_ACCESS_WANT_CANCEL) | \
+ BIT(OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS) | \
BIT(OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION))
#define NFSD_OA_OPEN_CLAIM (BIT(OPEN_ARGS_OPEN_CLAIM_NULL) | \
@@ -3602,7 +3603,11 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
if (status)
goto out;
}
- if (attrmask[0] & (FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE)) {
+ if ((attrmask[0] & (FATTR4_WORD0_CHANGE |
+ FATTR4_WORD0_SIZE)) ||
+ (attrmask[1] & (FATTR4_WORD1_TIME_ACCESS |
+ FATTR4_WORD1_TIME_MODIFY |
+ FATTR4_WORD1_TIME_METADATA))) {
status = nfsd4_deleg_getattr_conflict(rqstp, dentry, &dp);
if (status)
goto out;
@@ -3617,8 +3622,14 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
if (ncf->ncf_file_modified) {
++ncf->ncf_initial_cinfo;
args.stat.size = ncf->ncf_cur_fsize;
+ if (!timespec64_is_epoch(&ncf->ncf_cb_mtime))
+ args.stat.mtime = ncf->ncf_cb_mtime;
}
args.change_attr = ncf->ncf_initial_cinfo;
+
+ if (!timespec64_is_epoch(&ncf->ncf_cb_atime))
+ args.stat.atime = ncf->ncf_cb_atime;
+
nfs4_put_stid(&dp->dl_stid);
} else {
args.change_attr = nfsd4_change_attribute(&args.stat);
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 1955c8e9c4c793728fa75dd136cadc735245483f..004415651295891b3440f52a4c986e3a668a48cb 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -459,6 +459,8 @@ enum {
FATTR4_WORD2_MODE_UMASK | \
NFSD4_2_SECURITY_ATTRS | \
FATTR4_WORD2_XATTR_SUPPORT | \
+ FATTR4_WORD2_TIME_DELEG_ACCESS | \
+ FATTR4_WORD2_TIME_DELEG_MODIFY | \
FATTR4_WORD2_OPEN_ARGUMENTS)
extern const u32 nfsd_suppattrs[3][3];
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 9d0e844515aa6ea0ec62f2b538ecc2c6a5e34652..6351e6eca7cc63ccf82a3a081cef39042d52f4e9 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -142,6 +142,8 @@ struct nfs4_cb_fattr {
/* from CB_GETATTR reply */
u64 ncf_cb_change;
u64 ncf_cb_fsize;
+ struct timespec64 ncf_cb_mtime;
+ struct timespec64 ncf_cb_atime;
unsigned long ncf_cb_flags;
bool ncf_file_modified;
diff --git a/fs/nfsd/xdr4cb.h b/fs/nfsd/xdr4cb.h
index e8b00309c449fe2667f7d48cda88ec0cff924f93..f1a315cd31b74f73f1d52702ae7b5c93d51ddf82 100644
--- a/fs/nfsd/xdr4cb.h
+++ b/fs/nfsd/xdr4cb.h
@@ -59,16 +59,20 @@
* 1: CB_GETATTR opcode (32-bit)
* N: file_handle
* 1: number of entry in attribute array (32-bit)
- * 1: entry 0 in attribute array (32-bit)
+ * 3: entry 0-2 in attribute array (32-bit * 3)
*/
#define NFS4_enc_cb_getattr_sz (cb_compound_enc_hdr_sz + \
cb_sequence_enc_sz + \
- 1 + enc_nfs4_fh_sz + 1 + 1)
+ 1 + enc_nfs4_fh_sz + 1 + 3)
/*
* 4: fattr_bitmap_maxsz
* 1: attribute array len
* 2: change attr (64-bit)
* 2: size (64-bit)
+ * 2: atime.seconds (64-bit)
+ * 1: atime.nanoseconds (32-bit)
+ * 2: mtime.seconds (64-bit)
+ * 1: mtime.nanoseconds (32-bit)
*/
#define NFS4_dec_cb_getattr_sz (cb_compound_dec_hdr_sz + \
- cb_sequence_dec_sz + 4 + 1 + 2 + 2 + op_dec_sz)
+ cb_sequence_dec_sz + 4 + 1 + 2 + 2 + 2 + 1 + 2 + 1 + op_dec_sz)
diff --git a/include/linux/time64.h b/include/linux/time64.h
index f1bcea8c124a361b6c1e3c98ef915840c22a8413..9934331c7b86b7fb981c7aec0494ac2f5e72977e 100644
--- a/include/linux/time64.h
+++ b/include/linux/time64.h
@@ -49,6 +49,11 @@ static inline int timespec64_equal(const struct timespec64 *a,
return (a->tv_sec == b->tv_sec) && (a->tv_nsec == b->tv_nsec);
}
+static inline bool timespec64_is_epoch(const struct timespec64 *ts)
+{
+ return ts->tv_sec == 0 && ts->tv_nsec == 0;
+}
+
/*
* lhs < rhs: return <0
* lhs == rhs: return 0
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 5/6] nfsd: add support for delegated timestamps
2024-10-14 19:26 ` [PATCH 5/6] nfsd: add support for delegated timestamps Jeff Layton
@ 2024-11-19 1:01 ` NeilBrown
2024-11-19 1:03 ` Chuck Lever
2024-11-19 1:31 ` Chuck Lever
0 siblings, 2 replies; 18+ messages in thread
From: NeilBrown @ 2024-11-19 1:01 UTC (permalink / raw)
To: Jeff Layton
Cc: Chuck Lever, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Jonathan Corbet, Trond Myklebust, Anna Schumaker, Thomas Haynes,
linux-nfs, linux-kernel, linux-doc, Jeff Layton
On Tue, 15 Oct 2024, Jeff Layton wrote:
> Add support for the delegated timestamps on write delegations. This
> allows the server to proxy timestamps from the delegation holder to
> other clients that are doing GETATTRs vs. the same inode.
>
> When OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS bit is set in the OPEN
> call, set the dl_type to the *_ATTRS_DELEG flavor of delegation.
>
> Add timespec64 fields to nfs4_cb_fattr and decode the timestamps into
> those. Vet those timestamps according to the delstid spec and update
> the inode attrs if necessary.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> fs/nfsd/nfs4callback.c | 42 +++++++++++++++++++--
> fs/nfsd/nfs4state.c | 99 +++++++++++++++++++++++++++++++++++++++++++-------
> fs/nfsd/nfs4xdr.c | 13 ++++++-
> fs/nfsd/nfsd.h | 2 +
> fs/nfsd/state.h | 2 +
> fs/nfsd/xdr4cb.h | 10 +++--
> include/linux/time64.h | 5 +++
> 7 files changed, 151 insertions(+), 22 deletions(-)
>
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 776838bb83e6b707a4df76326cdc68f32daf1755..08245596289a960eb8b2e78df276544e7d3f4ff8 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -42,6 +42,7 @@
> #include "trace.h"
> #include "xdr4cb.h"
> #include "xdr4.h"
> +#include "nfs4xdr_gen.h"
>
> #define NFSDDBG_FACILITY NFSDDBG_PROC
>
> @@ -93,12 +94,35 @@ static int decode_cb_fattr4(struct xdr_stream *xdr, uint32_t *bitmap,
> {
> fattr->ncf_cb_change = 0;
> fattr->ncf_cb_fsize = 0;
> + fattr->ncf_cb_atime.tv_sec = 0;
> + fattr->ncf_cb_atime.tv_nsec = 0;
> + fattr->ncf_cb_mtime.tv_sec = 0;
> + fattr->ncf_cb_mtime.tv_nsec = 0;
> +
> if (bitmap[0] & FATTR4_WORD0_CHANGE)
> if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_change) < 0)
> return -NFSERR_BAD_XDR;
> if (bitmap[0] & FATTR4_WORD0_SIZE)
> if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_fsize) < 0)
> return -NFSERR_BAD_XDR;
> + if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
> + fattr4_time_deleg_access access;
> +
> + if (!xdrgen_decode_fattr4_time_deleg_access(xdr, &access))
> + return -NFSERR_BAD_XDR;
> + fattr->ncf_cb_atime.tv_sec = access.seconds;
> + fattr->ncf_cb_atime.tv_nsec = access.nseconds;
> +
> + }
> + if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
> + fattr4_time_deleg_modify modify;
> +
> + if (!xdrgen_decode_fattr4_time_deleg_modify(xdr, &modify))
> + return -NFSERR_BAD_XDR;
> + fattr->ncf_cb_mtime.tv_sec = modify.seconds;
> + fattr->ncf_cb_mtime.tv_nsec = modify.nseconds;
> +
> + }
> return 0;
> }
>
> @@ -364,15 +388,21 @@ encode_cb_getattr4args(struct xdr_stream *xdr, struct nfs4_cb_compound_hdr *hdr,
> struct nfs4_delegation *dp = container_of(fattr, struct nfs4_delegation, dl_cb_fattr);
> struct knfsd_fh *fh = &dp->dl_stid.sc_file->fi_fhandle;
> struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
> - u32 bmap[1];
> + u32 bmap_size = 1;
> + u32 bmap[3];
>
> bmap[0] = FATTR4_WORD0_SIZE;
> if (!ncf->ncf_file_modified)
> bmap[0] |= FATTR4_WORD0_CHANGE;
>
> + if (deleg_attrs_deleg(dp->dl_type)) {
> + bmap[1] = 0;
> + bmap[2] = FATTR4_WORD2_TIME_DELEG_ACCESS | FATTR4_WORD2_TIME_DELEG_MODIFY;
> + bmap_size = 3;
> + }
> encode_nfs_cb_opnum4(xdr, OP_CB_GETATTR);
> encode_nfs_fh4(xdr, fh);
> - encode_bitmap4(xdr, bmap, ARRAY_SIZE(bmap));
> + encode_bitmap4(xdr, bmap, bmap_size);
> hdr->nops++;
> }
>
> @@ -597,7 +627,7 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
> struct nfs4_cb_compound_hdr hdr;
> int status;
> u32 bitmap[3] = {0};
> - u32 attrlen;
> + u32 attrlen, maxlen;
> struct nfs4_cb_fattr *ncf =
> container_of(cb, struct nfs4_cb_fattr, ncf_getattr);
>
> @@ -616,7 +646,11 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
> return -NFSERR_BAD_XDR;
> if (xdr_stream_decode_u32(xdr, &attrlen) < 0)
> return -NFSERR_BAD_XDR;
> - if (attrlen > (sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize)))
> + maxlen = sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize);
> + if (bitmap[2] != 0)
> + maxlen += (sizeof(ncf->ncf_cb_mtime.tv_sec) +
> + sizeof(ncf->ncf_cb_mtime.tv_nsec)) * 2;
> + if (attrlen > maxlen)
> return -NFSERR_BAD_XDR;
> status = decode_cb_fattr4(xdr, bitmap, ncf);
> return status;
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 62f9aeb159d0f2ab4d293bf5c0c56ad7b86eb9d6..2c8d2bb5261ad189c6dfb1c4050c23d8cf061325 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5803,13 +5803,14 @@ static struct nfs4_delegation *
> nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> struct svc_fh *parent)
> {
> - int status = 0;
> + bool deleg_ts = open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS;
> struct nfs4_client *clp = stp->st_stid.sc_client;
> struct nfs4_file *fp = stp->st_stid.sc_file;
> struct nfs4_clnt_odstate *odstate = stp->st_clnt_odstate;
> struct nfs4_delegation *dp;
> struct nfsd_file *nf = NULL;
> struct file_lease *fl;
> + int status = 0;
> u32 dl_type;
>
> /*
> @@ -5834,7 +5835,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> */
> if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_BOTH) {
> nf = find_rw_file(fp);
> - dl_type = OPEN_DELEGATE_WRITE;
> + dl_type = deleg_ts ? OPEN_DELEGATE_WRITE_ATTRS_DELEG : OPEN_DELEGATE_WRITE;
> }
>
> /*
> @@ -5843,7 +5844,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> */
> if (!nf && (open->op_share_access & NFS4_SHARE_ACCESS_READ)) {
> nf = find_readable_file(fp);
> - dl_type = OPEN_DELEGATE_READ;
> + dl_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG : OPEN_DELEGATE_READ;
> }
>
> if (!nf)
> @@ -6001,13 +6002,14 @@ static void
> nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> struct svc_fh *currentfh)
> {
> - struct nfs4_delegation *dp;
> + bool deleg_ts = open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS;
> struct nfs4_openowner *oo = openowner(stp->st_stateowner);
> struct nfs4_client *clp = stp->st_stid.sc_client;
> struct svc_fh *parent = NULL;
> - int cb_up;
> - int status = 0;
> + struct nfs4_delegation *dp;
> struct kstat stat;
> + int status = 0;
> + int cb_up;
>
> cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
> open->op_recall = false;
> @@ -6048,12 +6050,14 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> destroy_delegation(dp);
> goto out_no_deleg;
> }
> - open->op_delegate_type = OPEN_DELEGATE_WRITE;
> + open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_WRITE_ATTRS_DELEG :
> + OPEN_DELEGATE_WRITE;
> dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
> dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
> trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> } else {
> - open->op_delegate_type = OPEN_DELEGATE_READ;
> + open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG :
> + OPEN_DELEGATE_READ;
> trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
> }
> nfs4_put_stid(&dp->dl_stid);
> @@ -8887,6 +8891,78 @@ nfsd4_get_writestateid(struct nfsd4_compound_state *cstate,
> get_stateid(cstate, &u->write.wr_stateid);
> }
>
> +/**
> + * set_cb_time - vet and set the timespec for a cb_getattr update
> + * @cb: timestamp from the CB_GETATTR response
> + * @orig: original timestamp in the inode
> + * @now: current time
> + *
> + * Given a timestamp in a CB_GETATTR response, check it against the
> + * current timestamp in the inode and the current time. Returns true
> + * if the inode's timestamp needs to be updated, and false otherwise.
> + * @cb may also be changed if the timestamp needs to be clamped.
> + */
> +static bool set_cb_time(struct timespec64 *cb, const struct timespec64 *orig,
> + const struct timespec64 *now)
> +{
> +
> + /*
> + * "When the time presented is before the original time, then the
> + * update is ignored." Also no need to update if there is no change.
> + */
> + if (timespec64_compare(cb, orig) <= 0)
> + return false;
> +
> + /*
> + * "When the time presented is in the future, the server can either
> + * clamp the new time to the current time, or it may
> + * return NFS4ERR_DELAY to the client, allowing it to retry."
> + */
> + if (timespec64_compare(cb, now) > 0) {
> + /* clamp it */
> + *cb = *now;
> + }
> +
> + return true;
> +}
> +
> +static int cb_getattr_update_times(struct dentry *dentry, struct nfs4_delegation *dp)
> +{
> + struct inode *inode = d_inode(dentry);
> + struct timespec64 now = current_time(inode);
> + struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
> + struct iattr attrs = { };
> + int ret;
> +
> + if (deleg_attrs_deleg(dp->dl_type)) {
> + struct timespec64 atime = inode_get_atime(inode);
> + struct timespec64 mtime = inode_get_mtime(inode);
> +
> + attrs.ia_atime = ncf->ncf_cb_atime;
> + attrs.ia_mtime = ncf->ncf_cb_mtime;
> +
> + if (set_cb_time(&attrs.ia_atime, &atime, &now))
> + attrs.ia_valid |= ATTR_ATIME | ATTR_ATIME_SET;
> +
> + if (set_cb_time(&attrs.ia_mtime, &mtime, &now)) {
> + attrs.ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET;
> + attrs.ia_ctime = attrs.ia_mtime;
> + }
> + } else {
> + attrs.ia_valid |= ATTR_MTIME | ATTR_CTIME;
> + attrs.ia_mtime = attrs.ia_ctime = now;
> + }
> +
> + if (!attrs.ia_valid)
> + return 0;
> +
> + attrs.ia_valid |= ATTR_DELEG;
> + inode_lock(inode);
> + ret = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
> + inode_unlock(inode);
> + return ret;
> +}
> +
> /**
> * nfsd4_deleg_getattr_conflict - Recall if GETATTR causes conflict
> * @rqstp: RPC transaction context
> @@ -8913,7 +8989,6 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
> struct file_lock_context *ctx;
> struct nfs4_delegation *dp = NULL;
> struct file_lease *fl;
> - struct iattr attrs;
> struct nfs4_cb_fattr *ncf;
> struct inode *inode = d_inode(dentry);
>
> @@ -8975,11 +9050,7 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
> * not update the file's metadata with the client's
> * modified size
> */
> - attrs.ia_mtime = attrs.ia_ctime = current_time(inode);
> - attrs.ia_valid = ATTR_MTIME | ATTR_CTIME | ATTR_DELEG;
> - inode_lock(inode);
> - err = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
> - inode_unlock(inode);
> + err = cb_getattr_update_times(dentry, dp);
> if (err) {
> status = nfserrno(err);
> goto out_status;
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 1c9d9349e4447c0078c7de0d533cf6278941679d..0e9f59f6be015bfa37893973f38fec880ff4c0b1 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -3409,6 +3409,7 @@ static __be32 nfsd4_encode_fattr4_xattr_support(struct xdr_stream *xdr,
> #define NFSD_OA_SHARE_ACCESS_WANT (BIT(OPEN_ARGS_SHARE_ACCESS_WANT_ANY_DELEG) | \
> BIT(OPEN_ARGS_SHARE_ACCESS_WANT_NO_DELEG) | \
> BIT(OPEN_ARGS_SHARE_ACCESS_WANT_CANCEL) | \
> + BIT(OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS) | \
> BIT(OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION))
>
> #define NFSD_OA_OPEN_CLAIM (BIT(OPEN_ARGS_OPEN_CLAIM_NULL) | \
> @@ -3602,7 +3603,11 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
> if (status)
> goto out;
> }
> - if (attrmask[0] & (FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE)) {
> + if ((attrmask[0] & (FATTR4_WORD0_CHANGE |
> + FATTR4_WORD0_SIZE)) ||
> + (attrmask[1] & (FATTR4_WORD1_TIME_ACCESS |
> + FATTR4_WORD1_TIME_MODIFY |
> + FATTR4_WORD1_TIME_METADATA))) {
> status = nfsd4_deleg_getattr_conflict(rqstp, dentry, &dp);
> if (status)
> goto out;
> @@ -3617,8 +3622,14 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
> if (ncf->ncf_file_modified) {
> ++ncf->ncf_initial_cinfo;
> args.stat.size = ncf->ncf_cur_fsize;
> + if (!timespec64_is_epoch(&ncf->ncf_cb_mtime))
> + args.stat.mtime = ncf->ncf_cb_mtime;
> }
> args.change_attr = ncf->ncf_initial_cinfo;
> +
> + if (!timespec64_is_epoch(&ncf->ncf_cb_atime))
> + args.stat.atime = ncf->ncf_cb_atime;
> +
> nfs4_put_stid(&dp->dl_stid);
> } else {
> args.change_attr = nfsd4_change_attribute(&args.stat);
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 1955c8e9c4c793728fa75dd136cadc735245483f..004415651295891b3440f52a4c986e3a668a48cb 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -459,6 +459,8 @@ enum {
> FATTR4_WORD2_MODE_UMASK | \
> NFSD4_2_SECURITY_ATTRS | \
> FATTR4_WORD2_XATTR_SUPPORT | \
> + FATTR4_WORD2_TIME_DELEG_ACCESS | \
> + FATTR4_WORD2_TIME_DELEG_MODIFY | \
This breaks 4.2 mounts for me (in latest nfsd-nexT). OPEN fails.
By setting these bits we tell the client that we support timestamp
delegation, but you haven't updated nfsd4_decode_share_access() to
understand NFS4_SHARE_WANT_DELEG_TIMESTAMPS in the 'share' flags for an
OPEN request. So the server responds with BADXDR to OPEN requests now.
Mounting with v4.1 still works.
NeilBrown
> FATTR4_WORD2_OPEN_ARGUMENTS)
>
> extern const u32 nfsd_suppattrs[3][3];
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 9d0e844515aa6ea0ec62f2b538ecc2c6a5e34652..6351e6eca7cc63ccf82a3a081cef39042d52f4e9 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -142,6 +142,8 @@ struct nfs4_cb_fattr {
> /* from CB_GETATTR reply */
> u64 ncf_cb_change;
> u64 ncf_cb_fsize;
> + struct timespec64 ncf_cb_mtime;
> + struct timespec64 ncf_cb_atime;
>
> unsigned long ncf_cb_flags;
> bool ncf_file_modified;
> diff --git a/fs/nfsd/xdr4cb.h b/fs/nfsd/xdr4cb.h
> index e8b00309c449fe2667f7d48cda88ec0cff924f93..f1a315cd31b74f73f1d52702ae7b5c93d51ddf82 100644
> --- a/fs/nfsd/xdr4cb.h
> +++ b/fs/nfsd/xdr4cb.h
> @@ -59,16 +59,20 @@
> * 1: CB_GETATTR opcode (32-bit)
> * N: file_handle
> * 1: number of entry in attribute array (32-bit)
> - * 1: entry 0 in attribute array (32-bit)
> + * 3: entry 0-2 in attribute array (32-bit * 3)
> */
> #define NFS4_enc_cb_getattr_sz (cb_compound_enc_hdr_sz + \
> cb_sequence_enc_sz + \
> - 1 + enc_nfs4_fh_sz + 1 + 1)
> + 1 + enc_nfs4_fh_sz + 1 + 3)
> /*
> * 4: fattr_bitmap_maxsz
> * 1: attribute array len
> * 2: change attr (64-bit)
> * 2: size (64-bit)
> + * 2: atime.seconds (64-bit)
> + * 1: atime.nanoseconds (32-bit)
> + * 2: mtime.seconds (64-bit)
> + * 1: mtime.nanoseconds (32-bit)
> */
> #define NFS4_dec_cb_getattr_sz (cb_compound_dec_hdr_sz + \
> - cb_sequence_dec_sz + 4 + 1 + 2 + 2 + op_dec_sz)
> + cb_sequence_dec_sz + 4 + 1 + 2 + 2 + 2 + 1 + 2 + 1 + op_dec_sz)
> diff --git a/include/linux/time64.h b/include/linux/time64.h
> index f1bcea8c124a361b6c1e3c98ef915840c22a8413..9934331c7b86b7fb981c7aec0494ac2f5e72977e 100644
> --- a/include/linux/time64.h
> +++ b/include/linux/time64.h
> @@ -49,6 +49,11 @@ static inline int timespec64_equal(const struct timespec64 *a,
> return (a->tv_sec == b->tv_sec) && (a->tv_nsec == b->tv_nsec);
> }
>
> +static inline bool timespec64_is_epoch(const struct timespec64 *ts)
> +{
> + return ts->tv_sec == 0 && ts->tv_nsec == 0;
> +}
> +
> /*
> * lhs < rhs: return <0
> * lhs == rhs: return 0
>
> --
> 2.47.0
>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 5/6] nfsd: add support for delegated timestamps
2024-11-19 1:01 ` NeilBrown
@ 2024-11-19 1:03 ` Chuck Lever
2024-11-19 1:09 ` Jeff Layton
2024-11-19 1:31 ` Chuck Lever
1 sibling, 1 reply; 18+ messages in thread
From: Chuck Lever @ 2024-11-19 1:03 UTC (permalink / raw)
To: NeilBrown
Cc: Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Jonathan Corbet, Trond Myklebust, Anna Schumaker, Thomas Haynes,
linux-nfs, linux-kernel, linux-doc
On Tue, Nov 19, 2024 at 12:01:33PM +1100, NeilBrown wrote:
> On Tue, 15 Oct 2024, Jeff Layton wrote:
> > Add support for the delegated timestamps on write delegations. This
> > allows the server to proxy timestamps from the delegation holder to
> > other clients that are doing GETATTRs vs. the same inode.
> >
> > When OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS bit is set in the OPEN
> > call, set the dl_type to the *_ATTRS_DELEG flavor of delegation.
> >
> > Add timespec64 fields to nfs4_cb_fattr and decode the timestamps into
> > those. Vet those timestamps according to the delstid spec and update
> > the inode attrs if necessary.
> >
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > ---
> > fs/nfsd/nfs4callback.c | 42 +++++++++++++++++++--
> > fs/nfsd/nfs4state.c | 99 +++++++++++++++++++++++++++++++++++++++++++-------
> > fs/nfsd/nfs4xdr.c | 13 ++++++-
> > fs/nfsd/nfsd.h | 2 +
> > fs/nfsd/state.h | 2 +
> > fs/nfsd/xdr4cb.h | 10 +++--
> > include/linux/time64.h | 5 +++
> > 7 files changed, 151 insertions(+), 22 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> > index 776838bb83e6b707a4df76326cdc68f32daf1755..08245596289a960eb8b2e78df276544e7d3f4ff8 100644
> > --- a/fs/nfsd/nfs4callback.c
> > +++ b/fs/nfsd/nfs4callback.c
> > @@ -42,6 +42,7 @@
> > #include "trace.h"
> > #include "xdr4cb.h"
> > #include "xdr4.h"
> > +#include "nfs4xdr_gen.h"
> >
> > #define NFSDDBG_FACILITY NFSDDBG_PROC
> >
> > @@ -93,12 +94,35 @@ static int decode_cb_fattr4(struct xdr_stream *xdr, uint32_t *bitmap,
> > {
> > fattr->ncf_cb_change = 0;
> > fattr->ncf_cb_fsize = 0;
> > + fattr->ncf_cb_atime.tv_sec = 0;
> > + fattr->ncf_cb_atime.tv_nsec = 0;
> > + fattr->ncf_cb_mtime.tv_sec = 0;
> > + fattr->ncf_cb_mtime.tv_nsec = 0;
> > +
> > if (bitmap[0] & FATTR4_WORD0_CHANGE)
> > if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_change) < 0)
> > return -NFSERR_BAD_XDR;
> > if (bitmap[0] & FATTR4_WORD0_SIZE)
> > if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_fsize) < 0)
> > return -NFSERR_BAD_XDR;
> > + if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
> > + fattr4_time_deleg_access access;
> > +
> > + if (!xdrgen_decode_fattr4_time_deleg_access(xdr, &access))
> > + return -NFSERR_BAD_XDR;
> > + fattr->ncf_cb_atime.tv_sec = access.seconds;
> > + fattr->ncf_cb_atime.tv_nsec = access.nseconds;
> > +
> > + }
> > + if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
> > + fattr4_time_deleg_modify modify;
> > +
> > + if (!xdrgen_decode_fattr4_time_deleg_modify(xdr, &modify))
> > + return -NFSERR_BAD_XDR;
> > + fattr->ncf_cb_mtime.tv_sec = modify.seconds;
> > + fattr->ncf_cb_mtime.tv_nsec = modify.nseconds;
> > +
> > + }
> > return 0;
> > }
> >
> > @@ -364,15 +388,21 @@ encode_cb_getattr4args(struct xdr_stream *xdr, struct nfs4_cb_compound_hdr *hdr,
> > struct nfs4_delegation *dp = container_of(fattr, struct nfs4_delegation, dl_cb_fattr);
> > struct knfsd_fh *fh = &dp->dl_stid.sc_file->fi_fhandle;
> > struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
> > - u32 bmap[1];
> > + u32 bmap_size = 1;
> > + u32 bmap[3];
> >
> > bmap[0] = FATTR4_WORD0_SIZE;
> > if (!ncf->ncf_file_modified)
> > bmap[0] |= FATTR4_WORD0_CHANGE;
> >
> > + if (deleg_attrs_deleg(dp->dl_type)) {
> > + bmap[1] = 0;
> > + bmap[2] = FATTR4_WORD2_TIME_DELEG_ACCESS | FATTR4_WORD2_TIME_DELEG_MODIFY;
> > + bmap_size = 3;
> > + }
> > encode_nfs_cb_opnum4(xdr, OP_CB_GETATTR);
> > encode_nfs_fh4(xdr, fh);
> > - encode_bitmap4(xdr, bmap, ARRAY_SIZE(bmap));
> > + encode_bitmap4(xdr, bmap, bmap_size);
> > hdr->nops++;
> > }
> >
> > @@ -597,7 +627,7 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
> > struct nfs4_cb_compound_hdr hdr;
> > int status;
> > u32 bitmap[3] = {0};
> > - u32 attrlen;
> > + u32 attrlen, maxlen;
> > struct nfs4_cb_fattr *ncf =
> > container_of(cb, struct nfs4_cb_fattr, ncf_getattr);
> >
> > @@ -616,7 +646,11 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
> > return -NFSERR_BAD_XDR;
> > if (xdr_stream_decode_u32(xdr, &attrlen) < 0)
> > return -NFSERR_BAD_XDR;
> > - if (attrlen > (sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize)))
> > + maxlen = sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize);
> > + if (bitmap[2] != 0)
> > + maxlen += (sizeof(ncf->ncf_cb_mtime.tv_sec) +
> > + sizeof(ncf->ncf_cb_mtime.tv_nsec)) * 2;
> > + if (attrlen > maxlen)
> > return -NFSERR_BAD_XDR;
> > status = decode_cb_fattr4(xdr, bitmap, ncf);
> > return status;
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 62f9aeb159d0f2ab4d293bf5c0c56ad7b86eb9d6..2c8d2bb5261ad189c6dfb1c4050c23d8cf061325 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -5803,13 +5803,14 @@ static struct nfs4_delegation *
> > nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > struct svc_fh *parent)
> > {
> > - int status = 0;
> > + bool deleg_ts = open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS;
> > struct nfs4_client *clp = stp->st_stid.sc_client;
> > struct nfs4_file *fp = stp->st_stid.sc_file;
> > struct nfs4_clnt_odstate *odstate = stp->st_clnt_odstate;
> > struct nfs4_delegation *dp;
> > struct nfsd_file *nf = NULL;
> > struct file_lease *fl;
> > + int status = 0;
> > u32 dl_type;
> >
> > /*
> > @@ -5834,7 +5835,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > */
> > if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_BOTH) {
> > nf = find_rw_file(fp);
> > - dl_type = OPEN_DELEGATE_WRITE;
> > + dl_type = deleg_ts ? OPEN_DELEGATE_WRITE_ATTRS_DELEG : OPEN_DELEGATE_WRITE;
> > }
> >
> > /*
> > @@ -5843,7 +5844,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > */
> > if (!nf && (open->op_share_access & NFS4_SHARE_ACCESS_READ)) {
> > nf = find_readable_file(fp);
> > - dl_type = OPEN_DELEGATE_READ;
> > + dl_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG : OPEN_DELEGATE_READ;
> > }
> >
> > if (!nf)
> > @@ -6001,13 +6002,14 @@ static void
> > nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > struct svc_fh *currentfh)
> > {
> > - struct nfs4_delegation *dp;
> > + bool deleg_ts = open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS;
> > struct nfs4_openowner *oo = openowner(stp->st_stateowner);
> > struct nfs4_client *clp = stp->st_stid.sc_client;
> > struct svc_fh *parent = NULL;
> > - int cb_up;
> > - int status = 0;
> > + struct nfs4_delegation *dp;
> > struct kstat stat;
> > + int status = 0;
> > + int cb_up;
> >
> > cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
> > open->op_recall = false;
> > @@ -6048,12 +6050,14 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > destroy_delegation(dp);
> > goto out_no_deleg;
> > }
> > - open->op_delegate_type = OPEN_DELEGATE_WRITE;
> > + open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_WRITE_ATTRS_DELEG :
> > + OPEN_DELEGATE_WRITE;
> > dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
> > dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
> > trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> > } else {
> > - open->op_delegate_type = OPEN_DELEGATE_READ;
> > + open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG :
> > + OPEN_DELEGATE_READ;
> > trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
> > }
> > nfs4_put_stid(&dp->dl_stid);
> > @@ -8887,6 +8891,78 @@ nfsd4_get_writestateid(struct nfsd4_compound_state *cstate,
> > get_stateid(cstate, &u->write.wr_stateid);
> > }
> >
> > +/**
> > + * set_cb_time - vet and set the timespec for a cb_getattr update
> > + * @cb: timestamp from the CB_GETATTR response
> > + * @orig: original timestamp in the inode
> > + * @now: current time
> > + *
> > + * Given a timestamp in a CB_GETATTR response, check it against the
> > + * current timestamp in the inode and the current time. Returns true
> > + * if the inode's timestamp needs to be updated, and false otherwise.
> > + * @cb may also be changed if the timestamp needs to be clamped.
> > + */
> > +static bool set_cb_time(struct timespec64 *cb, const struct timespec64 *orig,
> > + const struct timespec64 *now)
> > +{
> > +
> > + /*
> > + * "When the time presented is before the original time, then the
> > + * update is ignored." Also no need to update if there is no change.
> > + */
> > + if (timespec64_compare(cb, orig) <= 0)
> > + return false;
> > +
> > + /*
> > + * "When the time presented is in the future, the server can either
> > + * clamp the new time to the current time, or it may
> > + * return NFS4ERR_DELAY to the client, allowing it to retry."
> > + */
> > + if (timespec64_compare(cb, now) > 0) {
> > + /* clamp it */
> > + *cb = *now;
> > + }
> > +
> > + return true;
> > +}
> > +
> > +static int cb_getattr_update_times(struct dentry *dentry, struct nfs4_delegation *dp)
> > +{
> > + struct inode *inode = d_inode(dentry);
> > + struct timespec64 now = current_time(inode);
> > + struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
> > + struct iattr attrs = { };
> > + int ret;
> > +
> > + if (deleg_attrs_deleg(dp->dl_type)) {
> > + struct timespec64 atime = inode_get_atime(inode);
> > + struct timespec64 mtime = inode_get_mtime(inode);
> > +
> > + attrs.ia_atime = ncf->ncf_cb_atime;
> > + attrs.ia_mtime = ncf->ncf_cb_mtime;
> > +
> > + if (set_cb_time(&attrs.ia_atime, &atime, &now))
> > + attrs.ia_valid |= ATTR_ATIME | ATTR_ATIME_SET;
> > +
> > + if (set_cb_time(&attrs.ia_mtime, &mtime, &now)) {
> > + attrs.ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET;
> > + attrs.ia_ctime = attrs.ia_mtime;
> > + }
> > + } else {
> > + attrs.ia_valid |= ATTR_MTIME | ATTR_CTIME;
> > + attrs.ia_mtime = attrs.ia_ctime = now;
> > + }
> > +
> > + if (!attrs.ia_valid)
> > + return 0;
> > +
> > + attrs.ia_valid |= ATTR_DELEG;
> > + inode_lock(inode);
> > + ret = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
> > + inode_unlock(inode);
> > + return ret;
> > +}
> > +
> > /**
> > * nfsd4_deleg_getattr_conflict - Recall if GETATTR causes conflict
> > * @rqstp: RPC transaction context
> > @@ -8913,7 +8989,6 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
> > struct file_lock_context *ctx;
> > struct nfs4_delegation *dp = NULL;
> > struct file_lease *fl;
> > - struct iattr attrs;
> > struct nfs4_cb_fattr *ncf;
> > struct inode *inode = d_inode(dentry);
> >
> > @@ -8975,11 +9050,7 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
> > * not update the file's metadata with the client's
> > * modified size
> > */
> > - attrs.ia_mtime = attrs.ia_ctime = current_time(inode);
> > - attrs.ia_valid = ATTR_MTIME | ATTR_CTIME | ATTR_DELEG;
> > - inode_lock(inode);
> > - err = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
> > - inode_unlock(inode);
> > + err = cb_getattr_update_times(dentry, dp);
> > if (err) {
> > status = nfserrno(err);
> > goto out_status;
> > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > index 1c9d9349e4447c0078c7de0d533cf6278941679d..0e9f59f6be015bfa37893973f38fec880ff4c0b1 100644
> > --- a/fs/nfsd/nfs4xdr.c
> > +++ b/fs/nfsd/nfs4xdr.c
> > @@ -3409,6 +3409,7 @@ static __be32 nfsd4_encode_fattr4_xattr_support(struct xdr_stream *xdr,
> > #define NFSD_OA_SHARE_ACCESS_WANT (BIT(OPEN_ARGS_SHARE_ACCESS_WANT_ANY_DELEG) | \
> > BIT(OPEN_ARGS_SHARE_ACCESS_WANT_NO_DELEG) | \
> > BIT(OPEN_ARGS_SHARE_ACCESS_WANT_CANCEL) | \
> > + BIT(OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS) | \
> > BIT(OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION))
> >
> > #define NFSD_OA_OPEN_CLAIM (BIT(OPEN_ARGS_OPEN_CLAIM_NULL) | \
> > @@ -3602,7 +3603,11 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
> > if (status)
> > goto out;
> > }
> > - if (attrmask[0] & (FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE)) {
> > + if ((attrmask[0] & (FATTR4_WORD0_CHANGE |
> > + FATTR4_WORD0_SIZE)) ||
> > + (attrmask[1] & (FATTR4_WORD1_TIME_ACCESS |
> > + FATTR4_WORD1_TIME_MODIFY |
> > + FATTR4_WORD1_TIME_METADATA))) {
> > status = nfsd4_deleg_getattr_conflict(rqstp, dentry, &dp);
> > if (status)
> > goto out;
> > @@ -3617,8 +3622,14 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
> > if (ncf->ncf_file_modified) {
> > ++ncf->ncf_initial_cinfo;
> > args.stat.size = ncf->ncf_cur_fsize;
> > + if (!timespec64_is_epoch(&ncf->ncf_cb_mtime))
> > + args.stat.mtime = ncf->ncf_cb_mtime;
> > }
> > args.change_attr = ncf->ncf_initial_cinfo;
> > +
> > + if (!timespec64_is_epoch(&ncf->ncf_cb_atime))
> > + args.stat.atime = ncf->ncf_cb_atime;
> > +
> > nfs4_put_stid(&dp->dl_stid);
> > } else {
> > args.change_attr = nfsd4_change_attribute(&args.stat);
> > diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> > index 1955c8e9c4c793728fa75dd136cadc735245483f..004415651295891b3440f52a4c986e3a668a48cb 100644
> > --- a/fs/nfsd/nfsd.h
> > +++ b/fs/nfsd/nfsd.h
> > @@ -459,6 +459,8 @@ enum {
> > FATTR4_WORD2_MODE_UMASK | \
> > NFSD4_2_SECURITY_ATTRS | \
> > FATTR4_WORD2_XATTR_SUPPORT | \
> > + FATTR4_WORD2_TIME_DELEG_ACCESS | \
> > + FATTR4_WORD2_TIME_DELEG_MODIFY | \
>
> This breaks 4.2 mounts for me (in latest nfsd-nexT). OPEN fails.
Yep, we're on it.
> By setting these bits we tell the client that we support timestamp
> delegation, but you haven't updated nfsd4_decode_share_access() to
> understand NFS4_SHARE_WANT_DELEG_TIMESTAMPS in the 'share' flags for an
> OPEN request. So the server responds with BADXDR to OPEN requests now.
>
> Mounting with v4.1 still works.
>
> NeilBrown
>
>
> > FATTR4_WORD2_OPEN_ARGUMENTS)
> >
> > extern const u32 nfsd_suppattrs[3][3];
> > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> > index 9d0e844515aa6ea0ec62f2b538ecc2c6a5e34652..6351e6eca7cc63ccf82a3a081cef39042d52f4e9 100644
> > --- a/fs/nfsd/state.h
> > +++ b/fs/nfsd/state.h
> > @@ -142,6 +142,8 @@ struct nfs4_cb_fattr {
> > /* from CB_GETATTR reply */
> > u64 ncf_cb_change;
> > u64 ncf_cb_fsize;
> > + struct timespec64 ncf_cb_mtime;
> > + struct timespec64 ncf_cb_atime;
> >
> > unsigned long ncf_cb_flags;
> > bool ncf_file_modified;
> > diff --git a/fs/nfsd/xdr4cb.h b/fs/nfsd/xdr4cb.h
> > index e8b00309c449fe2667f7d48cda88ec0cff924f93..f1a315cd31b74f73f1d52702ae7b5c93d51ddf82 100644
> > --- a/fs/nfsd/xdr4cb.h
> > +++ b/fs/nfsd/xdr4cb.h
> > @@ -59,16 +59,20 @@
> > * 1: CB_GETATTR opcode (32-bit)
> > * N: file_handle
> > * 1: number of entry in attribute array (32-bit)
> > - * 1: entry 0 in attribute array (32-bit)
> > + * 3: entry 0-2 in attribute array (32-bit * 3)
> > */
> > #define NFS4_enc_cb_getattr_sz (cb_compound_enc_hdr_sz + \
> > cb_sequence_enc_sz + \
> > - 1 + enc_nfs4_fh_sz + 1 + 1)
> > + 1 + enc_nfs4_fh_sz + 1 + 3)
> > /*
> > * 4: fattr_bitmap_maxsz
> > * 1: attribute array len
> > * 2: change attr (64-bit)
> > * 2: size (64-bit)
> > + * 2: atime.seconds (64-bit)
> > + * 1: atime.nanoseconds (32-bit)
> > + * 2: mtime.seconds (64-bit)
> > + * 1: mtime.nanoseconds (32-bit)
> > */
> > #define NFS4_dec_cb_getattr_sz (cb_compound_dec_hdr_sz + \
> > - cb_sequence_dec_sz + 4 + 1 + 2 + 2 + op_dec_sz)
> > + cb_sequence_dec_sz + 4 + 1 + 2 + 2 + 2 + 1 + 2 + 1 + op_dec_sz)
> > diff --git a/include/linux/time64.h b/include/linux/time64.h
> > index f1bcea8c124a361b6c1e3c98ef915840c22a8413..9934331c7b86b7fb981c7aec0494ac2f5e72977e 100644
> > --- a/include/linux/time64.h
> > +++ b/include/linux/time64.h
> > @@ -49,6 +49,11 @@ static inline int timespec64_equal(const struct timespec64 *a,
> > return (a->tv_sec == b->tv_sec) && (a->tv_nsec == b->tv_nsec);
> > }
> >
> > +static inline bool timespec64_is_epoch(const struct timespec64 *ts)
> > +{
> > + return ts->tv_sec == 0 && ts->tv_nsec == 0;
> > +}
> > +
> > /*
> > * lhs < rhs: return <0
> > * lhs == rhs: return 0
> >
> > --
> > 2.47.0
> >
> >
> >
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 5/6] nfsd: add support for delegated timestamps
2024-11-19 1:03 ` Chuck Lever
@ 2024-11-19 1:09 ` Jeff Layton
2024-11-19 1:19 ` Chuck Lever
0 siblings, 1 reply; 18+ messages in thread
From: Jeff Layton @ 2024-11-19 1:09 UTC (permalink / raw)
To: Chuck Lever, NeilBrown
Cc: Olga Kornievskaia, Dai Ngo, Tom Talpey, Jonathan Corbet,
Trond Myklebust, Anna Schumaker, Thomas Haynes, linux-nfs,
linux-kernel, linux-doc
On Mon, 2024-11-18 at 20:03 -0500, Chuck Lever wrote:
> On Tue, Nov 19, 2024 at 12:01:33PM +1100, NeilBrown wrote:
> > On Tue, 15 Oct 2024, Jeff Layton wrote:
> > > Add support for the delegated timestamps on write delegations. This
> > > allows the server to proxy timestamps from the delegation holder to
> > > other clients that are doing GETATTRs vs. the same inode.
> > >
> > > When OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS bit is set in the OPEN
> > > call, set the dl_type to the *_ATTRS_DELEG flavor of delegation.
> > >
> > > Add timespec64 fields to nfs4_cb_fattr and decode the timestamps into
> > > those. Vet those timestamps according to the delstid spec and update
> > > the inode attrs if necessary.
> > >
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > > ---
> > > fs/nfsd/nfs4callback.c | 42 +++++++++++++++++++--
> > > fs/nfsd/nfs4state.c | 99 +++++++++++++++++++++++++++++++++++++++++++-------
> > > fs/nfsd/nfs4xdr.c | 13 ++++++-
> > > fs/nfsd/nfsd.h | 2 +
> > > fs/nfsd/state.h | 2 +
> > > fs/nfsd/xdr4cb.h | 10 +++--
> > > include/linux/time64.h | 5 +++
> > > 7 files changed, 151 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> > > index 776838bb83e6b707a4df76326cdc68f32daf1755..08245596289a960eb8b2e78df276544e7d3f4ff8 100644
> > > --- a/fs/nfsd/nfs4callback.c
> > > +++ b/fs/nfsd/nfs4callback.c
> > > @@ -42,6 +42,7 @@
> > > #include "trace.h"
> > > #include "xdr4cb.h"
> > > #include "xdr4.h"
> > > +#include "nfs4xdr_gen.h"
> > >
> > > #define NFSDDBG_FACILITY NFSDDBG_PROC
> > >
> > > @@ -93,12 +94,35 @@ static int decode_cb_fattr4(struct xdr_stream *xdr, uint32_t *bitmap,
> > > {
> > > fattr->ncf_cb_change = 0;
> > > fattr->ncf_cb_fsize = 0;
> > > + fattr->ncf_cb_atime.tv_sec = 0;
> > > + fattr->ncf_cb_atime.tv_nsec = 0;
> > > + fattr->ncf_cb_mtime.tv_sec = 0;
> > > + fattr->ncf_cb_mtime.tv_nsec = 0;
> > > +
> > > if (bitmap[0] & FATTR4_WORD0_CHANGE)
> > > if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_change) < 0)
> > > return -NFSERR_BAD_XDR;
> > > if (bitmap[0] & FATTR4_WORD0_SIZE)
> > > if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_fsize) < 0)
> > > return -NFSERR_BAD_XDR;
> > > + if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
> > > + fattr4_time_deleg_access access;
> > > +
> > > + if (!xdrgen_decode_fattr4_time_deleg_access(xdr, &access))
> > > + return -NFSERR_BAD_XDR;
> > > + fattr->ncf_cb_atime.tv_sec = access.seconds;
> > > + fattr->ncf_cb_atime.tv_nsec = access.nseconds;
> > > +
> > > + }
> > > + if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
> > > + fattr4_time_deleg_modify modify;
> > > +
> > > + if (!xdrgen_decode_fattr4_time_deleg_modify(xdr, &modify))
> > > + return -NFSERR_BAD_XDR;
> > > + fattr->ncf_cb_mtime.tv_sec = modify.seconds;
> > > + fattr->ncf_cb_mtime.tv_nsec = modify.nseconds;
> > > +
> > > + }
> > > return 0;
> > > }
> > >
> > > @@ -364,15 +388,21 @@ encode_cb_getattr4args(struct xdr_stream *xdr, struct nfs4_cb_compound_hdr *hdr,
> > > struct nfs4_delegation *dp = container_of(fattr, struct nfs4_delegation, dl_cb_fattr);
> > > struct knfsd_fh *fh = &dp->dl_stid.sc_file->fi_fhandle;
> > > struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
> > > - u32 bmap[1];
> > > + u32 bmap_size = 1;
> > > + u32 bmap[3];
> > >
> > > bmap[0] = FATTR4_WORD0_SIZE;
> > > if (!ncf->ncf_file_modified)
> > > bmap[0] |= FATTR4_WORD0_CHANGE;
> > >
> > > + if (deleg_attrs_deleg(dp->dl_type)) {
> > > + bmap[1] = 0;
> > > + bmap[2] = FATTR4_WORD2_TIME_DELEG_ACCESS | FATTR4_WORD2_TIME_DELEG_MODIFY;
> > > + bmap_size = 3;
> > > + }
> > > encode_nfs_cb_opnum4(xdr, OP_CB_GETATTR);
> > > encode_nfs_fh4(xdr, fh);
> > > - encode_bitmap4(xdr, bmap, ARRAY_SIZE(bmap));
> > > + encode_bitmap4(xdr, bmap, bmap_size);
> > > hdr->nops++;
> > > }
> > >
> > > @@ -597,7 +627,7 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
> > > struct nfs4_cb_compound_hdr hdr;
> > > int status;
> > > u32 bitmap[3] = {0};
> > > - u32 attrlen;
> > > + u32 attrlen, maxlen;
> > > struct nfs4_cb_fattr *ncf =
> > > container_of(cb, struct nfs4_cb_fattr, ncf_getattr);
> > >
> > > @@ -616,7 +646,11 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
> > > return -NFSERR_BAD_XDR;
> > > if (xdr_stream_decode_u32(xdr, &attrlen) < 0)
> > > return -NFSERR_BAD_XDR;
> > > - if (attrlen > (sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize)))
> > > + maxlen = sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize);
> > > + if (bitmap[2] != 0)
> > > + maxlen += (sizeof(ncf->ncf_cb_mtime.tv_sec) +
> > > + sizeof(ncf->ncf_cb_mtime.tv_nsec)) * 2;
> > > + if (attrlen > maxlen)
> > > return -NFSERR_BAD_XDR;
> > > status = decode_cb_fattr4(xdr, bitmap, ncf);
> > > return status;
> > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > > index 62f9aeb159d0f2ab4d293bf5c0c56ad7b86eb9d6..2c8d2bb5261ad189c6dfb1c4050c23d8cf061325 100644
> > > --- a/fs/nfsd/nfs4state.c
> > > +++ b/fs/nfsd/nfs4state.c
> > > @@ -5803,13 +5803,14 @@ static struct nfs4_delegation *
> > > nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > > struct svc_fh *parent)
> > > {
> > > - int status = 0;
> > > + bool deleg_ts = open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS;
> > > struct nfs4_client *clp = stp->st_stid.sc_client;
> > > struct nfs4_file *fp = stp->st_stid.sc_file;
> > > struct nfs4_clnt_odstate *odstate = stp->st_clnt_odstate;
> > > struct nfs4_delegation *dp;
> > > struct nfsd_file *nf = NULL;
> > > struct file_lease *fl;
> > > + int status = 0;
> > > u32 dl_type;
> > >
> > > /*
> > > @@ -5834,7 +5835,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > > */
> > > if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_BOTH) {
> > > nf = find_rw_file(fp);
> > > - dl_type = OPEN_DELEGATE_WRITE;
> > > + dl_type = deleg_ts ? OPEN_DELEGATE_WRITE_ATTRS_DELEG : OPEN_DELEGATE_WRITE;
> > > }
> > >
> > > /*
> > > @@ -5843,7 +5844,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > > */
> > > if (!nf && (open->op_share_access & NFS4_SHARE_ACCESS_READ)) {
> > > nf = find_readable_file(fp);
> > > - dl_type = OPEN_DELEGATE_READ;
> > > + dl_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG : OPEN_DELEGATE_READ;
> > > }
> > >
> > > if (!nf)
> > > @@ -6001,13 +6002,14 @@ static void
> > > nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > > struct svc_fh *currentfh)
> > > {
> > > - struct nfs4_delegation *dp;
> > > + bool deleg_ts = open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS;
> > > struct nfs4_openowner *oo = openowner(stp->st_stateowner);
> > > struct nfs4_client *clp = stp->st_stid.sc_client;
> > > struct svc_fh *parent = NULL;
> > > - int cb_up;
> > > - int status = 0;
> > > + struct nfs4_delegation *dp;
> > > struct kstat stat;
> > > + int status = 0;
> > > + int cb_up;
> > >
> > > cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
> > > open->op_recall = false;
> > > @@ -6048,12 +6050,14 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > > destroy_delegation(dp);
> > > goto out_no_deleg;
> > > }
> > > - open->op_delegate_type = OPEN_DELEGATE_WRITE;
> > > + open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_WRITE_ATTRS_DELEG :
> > > + OPEN_DELEGATE_WRITE;
> > > dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
> > > dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
> > > trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> > > } else {
> > > - open->op_delegate_type = OPEN_DELEGATE_READ;
> > > + open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG :
> > > + OPEN_DELEGATE_READ;
> > > trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
> > > }
> > > nfs4_put_stid(&dp->dl_stid);
> > > @@ -8887,6 +8891,78 @@ nfsd4_get_writestateid(struct nfsd4_compound_state *cstate,
> > > get_stateid(cstate, &u->write.wr_stateid);
> > > }
> > >
> > > +/**
> > > + * set_cb_time - vet and set the timespec for a cb_getattr update
> > > + * @cb: timestamp from the CB_GETATTR response
> > > + * @orig: original timestamp in the inode
> > > + * @now: current time
> > > + *
> > > + * Given a timestamp in a CB_GETATTR response, check it against the
> > > + * current timestamp in the inode and the current time. Returns true
> > > + * if the inode's timestamp needs to be updated, and false otherwise.
> > > + * @cb may also be changed if the timestamp needs to be clamped.
> > > + */
> > > +static bool set_cb_time(struct timespec64 *cb, const struct timespec64 *orig,
> > > + const struct timespec64 *now)
> > > +{
> > > +
> > > + /*
> > > + * "When the time presented is before the original time, then the
> > > + * update is ignored." Also no need to update if there is no change.
> > > + */
> > > + if (timespec64_compare(cb, orig) <= 0)
> > > + return false;
> > > +
> > > + /*
> > > + * "When the time presented is in the future, the server can either
> > > + * clamp the new time to the current time, or it may
> > > + * return NFS4ERR_DELAY to the client, allowing it to retry."
> > > + */
> > > + if (timespec64_compare(cb, now) > 0) {
> > > + /* clamp it */
> > > + *cb = *now;
> > > + }
> > > +
> > > + return true;
> > > +}
> > > +
> > > +static int cb_getattr_update_times(struct dentry *dentry, struct nfs4_delegation *dp)
> > > +{
> > > + struct inode *inode = d_inode(dentry);
> > > + struct timespec64 now = current_time(inode);
> > > + struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
> > > + struct iattr attrs = { };
> > > + int ret;
> > > +
> > > + if (deleg_attrs_deleg(dp->dl_type)) {
> > > + struct timespec64 atime = inode_get_atime(inode);
> > > + struct timespec64 mtime = inode_get_mtime(inode);
> > > +
> > > + attrs.ia_atime = ncf->ncf_cb_atime;
> > > + attrs.ia_mtime = ncf->ncf_cb_mtime;
> > > +
> > > + if (set_cb_time(&attrs.ia_atime, &atime, &now))
> > > + attrs.ia_valid |= ATTR_ATIME | ATTR_ATIME_SET;
> > > +
> > > + if (set_cb_time(&attrs.ia_mtime, &mtime, &now)) {
> > > + attrs.ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET;
> > > + attrs.ia_ctime = attrs.ia_mtime;
> > > + }
> > > + } else {
> > > + attrs.ia_valid |= ATTR_MTIME | ATTR_CTIME;
> > > + attrs.ia_mtime = attrs.ia_ctime = now;
> > > + }
> > > +
> > > + if (!attrs.ia_valid)
> > > + return 0;
> > > +
> > > + attrs.ia_valid |= ATTR_DELEG;
> > > + inode_lock(inode);
> > > + ret = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
> > > + inode_unlock(inode);
> > > + return ret;
> > > +}
> > > +
> > > /**
> > > * nfsd4_deleg_getattr_conflict - Recall if GETATTR causes conflict
> > > * @rqstp: RPC transaction context
> > > @@ -8913,7 +8989,6 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
> > > struct file_lock_context *ctx;
> > > struct nfs4_delegation *dp = NULL;
> > > struct file_lease *fl;
> > > - struct iattr attrs;
> > > struct nfs4_cb_fattr *ncf;
> > > struct inode *inode = d_inode(dentry);
> > >
> > > @@ -8975,11 +9050,7 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
> > > * not update the file's metadata with the client's
> > > * modified size
> > > */
> > > - attrs.ia_mtime = attrs.ia_ctime = current_time(inode);
> > > - attrs.ia_valid = ATTR_MTIME | ATTR_CTIME | ATTR_DELEG;
> > > - inode_lock(inode);
> > > - err = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
> > > - inode_unlock(inode);
> > > + err = cb_getattr_update_times(dentry, dp);
> > > if (err) {
> > > status = nfserrno(err);
> > > goto out_status;
> > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > > index 1c9d9349e4447c0078c7de0d533cf6278941679d..0e9f59f6be015bfa37893973f38fec880ff4c0b1 100644
> > > --- a/fs/nfsd/nfs4xdr.c
> > > +++ b/fs/nfsd/nfs4xdr.c
> > > @@ -3409,6 +3409,7 @@ static __be32 nfsd4_encode_fattr4_xattr_support(struct xdr_stream *xdr,
> > > #define NFSD_OA_SHARE_ACCESS_WANT (BIT(OPEN_ARGS_SHARE_ACCESS_WANT_ANY_DELEG) | \
> > > BIT(OPEN_ARGS_SHARE_ACCESS_WANT_NO_DELEG) | \
> > > BIT(OPEN_ARGS_SHARE_ACCESS_WANT_CANCEL) | \
> > > + BIT(OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS) | \
> > > BIT(OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION))
> > >
> > > #define NFSD_OA_OPEN_CLAIM (BIT(OPEN_ARGS_OPEN_CLAIM_NULL) | \
> > > @@ -3602,7 +3603,11 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
> > > if (status)
> > > goto out;
> > > }
> > > - if (attrmask[0] & (FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE)) {
> > > + if ((attrmask[0] & (FATTR4_WORD0_CHANGE |
> > > + FATTR4_WORD0_SIZE)) ||
> > > + (attrmask[1] & (FATTR4_WORD1_TIME_ACCESS |
> > > + FATTR4_WORD1_TIME_MODIFY |
> > > + FATTR4_WORD1_TIME_METADATA))) {
> > > status = nfsd4_deleg_getattr_conflict(rqstp, dentry, &dp);
> > > if (status)
> > > goto out;
> > > @@ -3617,8 +3622,14 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
> > > if (ncf->ncf_file_modified) {
> > > ++ncf->ncf_initial_cinfo;
> > > args.stat.size = ncf->ncf_cur_fsize;
> > > + if (!timespec64_is_epoch(&ncf->ncf_cb_mtime))
> > > + args.stat.mtime = ncf->ncf_cb_mtime;
> > > }
> > > args.change_attr = ncf->ncf_initial_cinfo;
> > > +
> > > + if (!timespec64_is_epoch(&ncf->ncf_cb_atime))
> > > + args.stat.atime = ncf->ncf_cb_atime;
> > > +
> > > nfs4_put_stid(&dp->dl_stid);
> > > } else {
> > > args.change_attr = nfsd4_change_attribute(&args.stat);
> > > diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> > > index 1955c8e9c4c793728fa75dd136cadc735245483f..004415651295891b3440f52a4c986e3a668a48cb 100644
> > > --- a/fs/nfsd/nfsd.h
> > > +++ b/fs/nfsd/nfsd.h
> > > @@ -459,6 +459,8 @@ enum {
> > > FATTR4_WORD2_MODE_UMASK | \
> > > NFSD4_2_SECURITY_ATTRS | \
> > > FATTR4_WORD2_XATTR_SUPPORT | \
> > > + FATTR4_WORD2_TIME_DELEG_ACCESS | \
> > > + FATTR4_WORD2_TIME_DELEG_MODIFY | \
> >
> > This breaks 4.2 mounts for me (in latest nfsd-nexT). OPEN fails.
>
> Yep, we're on it.
>
I see the problem. The OPEN_XOR_DELEGATION patch reworked some of the
NFS4_SHARE_WANT_* handling, and this patch relied on those changes.
When that patch was dropped, it broke this patch.
What we should probably do is split out the flag rework from that patch
into a separate patch that this can rely on. Not sure if you want to
embark upon all of that during the merge window though. It may be
better to just drop these patches as well.
>
> > By setting these bits we tell the client that we support timestamp
> > delegation, but you haven't updated nfsd4_decode_share_access() to
> > understand NFS4_SHARE_WANT_DELEG_TIMESTAMPS in the 'share' flags for an
> > OPEN request. So the server responds with BADXDR to OPEN requests now.
> >
> > Mounting with v4.1 still works.
> >
> > NeilBrown
> >
> >
> > > FATTR4_WORD2_OPEN_ARGUMENTS)
> > >
> > > extern const u32 nfsd_suppattrs[3][3];
> > > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> > > index 9d0e844515aa6ea0ec62f2b538ecc2c6a5e34652..6351e6eca7cc63ccf82a3a081cef39042d52f4e9 100644
> > > --- a/fs/nfsd/state.h
> > > +++ b/fs/nfsd/state.h
> > > @@ -142,6 +142,8 @@ struct nfs4_cb_fattr {
> > > /* from CB_GETATTR reply */
> > > u64 ncf_cb_change;
> > > u64 ncf_cb_fsize;
> > > + struct timespec64 ncf_cb_mtime;
> > > + struct timespec64 ncf_cb_atime;
> > >
> > > unsigned long ncf_cb_flags;
> > > bool ncf_file_modified;
> > > diff --git a/fs/nfsd/xdr4cb.h b/fs/nfsd/xdr4cb.h
> > > index e8b00309c449fe2667f7d48cda88ec0cff924f93..f1a315cd31b74f73f1d52702ae7b5c93d51ddf82 100644
> > > --- a/fs/nfsd/xdr4cb.h
> > > +++ b/fs/nfsd/xdr4cb.h
> > > @@ -59,16 +59,20 @@
> > > * 1: CB_GETATTR opcode (32-bit)
> > > * N: file_handle
> > > * 1: number of entry in attribute array (32-bit)
> > > - * 1: entry 0 in attribute array (32-bit)
> > > + * 3: entry 0-2 in attribute array (32-bit * 3)
> > > */
> > > #define NFS4_enc_cb_getattr_sz (cb_compound_enc_hdr_sz + \
> > > cb_sequence_enc_sz + \
> > > - 1 + enc_nfs4_fh_sz + 1 + 1)
> > > + 1 + enc_nfs4_fh_sz + 1 + 3)
> > > /*
> > > * 4: fattr_bitmap_maxsz
> > > * 1: attribute array len
> > > * 2: change attr (64-bit)
> > > * 2: size (64-bit)
> > > + * 2: atime.seconds (64-bit)
> > > + * 1: atime.nanoseconds (32-bit)
> > > + * 2: mtime.seconds (64-bit)
> > > + * 1: mtime.nanoseconds (32-bit)
> > > */
> > > #define NFS4_dec_cb_getattr_sz (cb_compound_dec_hdr_sz + \
> > > - cb_sequence_dec_sz + 4 + 1 + 2 + 2 + op_dec_sz)
> > > + cb_sequence_dec_sz + 4 + 1 + 2 + 2 + 2 + 1 + 2 + 1 + op_dec_sz)
> > > diff --git a/include/linux/time64.h b/include/linux/time64.h
> > > index f1bcea8c124a361b6c1e3c98ef915840c22a8413..9934331c7b86b7fb981c7aec0494ac2f5e72977e 100644
> > > --- a/include/linux/time64.h
> > > +++ b/include/linux/time64.h
> > > @@ -49,6 +49,11 @@ static inline int timespec64_equal(const struct timespec64 *a,
> > > return (a->tv_sec == b->tv_sec) && (a->tv_nsec == b->tv_nsec);
> > > }
> > >
> > > +static inline bool timespec64_is_epoch(const struct timespec64 *ts)
> > > +{
> > > + return ts->tv_sec == 0 && ts->tv_nsec == 0;
> > > +}
> > > +
> > > /*
> > > * lhs < rhs: return <0
> > > * lhs == rhs: return 0
> > >
> > > --
> > > 2.47.0
> > >
> > >
> > >
> >
>
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 5/6] nfsd: add support for delegated timestamps
2024-11-19 1:09 ` Jeff Layton
@ 2024-11-19 1:19 ` Chuck Lever
0 siblings, 0 replies; 18+ messages in thread
From: Chuck Lever @ 2024-11-19 1:19 UTC (permalink / raw)
To: Jeff Layton
Cc: NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Jonathan Corbet, Trond Myklebust, Anna Schumaker, Thomas Haynes,
linux-nfs, linux-kernel, linux-doc
On Mon, Nov 18, 2024 at 08:09:35PM -0500, Jeff Layton wrote:
> On Mon, 2024-11-18 at 20:03 -0500, Chuck Lever wrote:
> > On Tue, Nov 19, 2024 at 12:01:33PM +1100, NeilBrown wrote:
> > > On Tue, 15 Oct 2024, Jeff Layton wrote:
> > > > Add support for the delegated timestamps on write delegations. This
> > > > allows the server to proxy timestamps from the delegation holder to
> > > > other clients that are doing GETATTRs vs. the same inode.
> > > >
> > > > When OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS bit is set in the OPEN
> > > > call, set the dl_type to the *_ATTRS_DELEG flavor of delegation.
> > > >
> > > > Add timespec64 fields to nfs4_cb_fattr and decode the timestamps into
> > > > those. Vet those timestamps according to the delstid spec and update
> > > > the inode attrs if necessary.
> > > >
> > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > > > ---
> > > > fs/nfsd/nfs4callback.c | 42 +++++++++++++++++++--
> > > > fs/nfsd/nfs4state.c | 99 +++++++++++++++++++++++++++++++++++++++++++-------
> > > > fs/nfsd/nfs4xdr.c | 13 ++++++-
> > > > fs/nfsd/nfsd.h | 2 +
> > > > fs/nfsd/state.h | 2 +
> > > > fs/nfsd/xdr4cb.h | 10 +++--
> > > > include/linux/time64.h | 5 +++
> > > > 7 files changed, 151 insertions(+), 22 deletions(-)
> > > >
> > > > diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> > > > index 776838bb83e6b707a4df76326cdc68f32daf1755..08245596289a960eb8b2e78df276544e7d3f4ff8 100644
> > > > --- a/fs/nfsd/nfs4callback.c
> > > > +++ b/fs/nfsd/nfs4callback.c
> > > > @@ -42,6 +42,7 @@
> > > > #include "trace.h"
> > > > #include "xdr4cb.h"
> > > > #include "xdr4.h"
> > > > +#include "nfs4xdr_gen.h"
> > > >
> > > > #define NFSDDBG_FACILITY NFSDDBG_PROC
> > > >
> > > > @@ -93,12 +94,35 @@ static int decode_cb_fattr4(struct xdr_stream *xdr, uint32_t *bitmap,
> > > > {
> > > > fattr->ncf_cb_change = 0;
> > > > fattr->ncf_cb_fsize = 0;
> > > > + fattr->ncf_cb_atime.tv_sec = 0;
> > > > + fattr->ncf_cb_atime.tv_nsec = 0;
> > > > + fattr->ncf_cb_mtime.tv_sec = 0;
> > > > + fattr->ncf_cb_mtime.tv_nsec = 0;
> > > > +
> > > > if (bitmap[0] & FATTR4_WORD0_CHANGE)
> > > > if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_change) < 0)
> > > > return -NFSERR_BAD_XDR;
> > > > if (bitmap[0] & FATTR4_WORD0_SIZE)
> > > > if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_fsize) < 0)
> > > > return -NFSERR_BAD_XDR;
> > > > + if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
> > > > + fattr4_time_deleg_access access;
> > > > +
> > > > + if (!xdrgen_decode_fattr4_time_deleg_access(xdr, &access))
> > > > + return -NFSERR_BAD_XDR;
> > > > + fattr->ncf_cb_atime.tv_sec = access.seconds;
> > > > + fattr->ncf_cb_atime.tv_nsec = access.nseconds;
> > > > +
> > > > + }
> > > > + if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
> > > > + fattr4_time_deleg_modify modify;
> > > > +
> > > > + if (!xdrgen_decode_fattr4_time_deleg_modify(xdr, &modify))
> > > > + return -NFSERR_BAD_XDR;
> > > > + fattr->ncf_cb_mtime.tv_sec = modify.seconds;
> > > > + fattr->ncf_cb_mtime.tv_nsec = modify.nseconds;
> > > > +
> > > > + }
> > > > return 0;
> > > > }
> > > >
> > > > @@ -364,15 +388,21 @@ encode_cb_getattr4args(struct xdr_stream *xdr, struct nfs4_cb_compound_hdr *hdr,
> > > > struct nfs4_delegation *dp = container_of(fattr, struct nfs4_delegation, dl_cb_fattr);
> > > > struct knfsd_fh *fh = &dp->dl_stid.sc_file->fi_fhandle;
> > > > struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
> > > > - u32 bmap[1];
> > > > + u32 bmap_size = 1;
> > > > + u32 bmap[3];
> > > >
> > > > bmap[0] = FATTR4_WORD0_SIZE;
> > > > if (!ncf->ncf_file_modified)
> > > > bmap[0] |= FATTR4_WORD0_CHANGE;
> > > >
> > > > + if (deleg_attrs_deleg(dp->dl_type)) {
> > > > + bmap[1] = 0;
> > > > + bmap[2] = FATTR4_WORD2_TIME_DELEG_ACCESS | FATTR4_WORD2_TIME_DELEG_MODIFY;
> > > > + bmap_size = 3;
> > > > + }
> > > > encode_nfs_cb_opnum4(xdr, OP_CB_GETATTR);
> > > > encode_nfs_fh4(xdr, fh);
> > > > - encode_bitmap4(xdr, bmap, ARRAY_SIZE(bmap));
> > > > + encode_bitmap4(xdr, bmap, bmap_size);
> > > > hdr->nops++;
> > > > }
> > > >
> > > > @@ -597,7 +627,7 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
> > > > struct nfs4_cb_compound_hdr hdr;
> > > > int status;
> > > > u32 bitmap[3] = {0};
> > > > - u32 attrlen;
> > > > + u32 attrlen, maxlen;
> > > > struct nfs4_cb_fattr *ncf =
> > > > container_of(cb, struct nfs4_cb_fattr, ncf_getattr);
> > > >
> > > > @@ -616,7 +646,11 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
> > > > return -NFSERR_BAD_XDR;
> > > > if (xdr_stream_decode_u32(xdr, &attrlen) < 0)
> > > > return -NFSERR_BAD_XDR;
> > > > - if (attrlen > (sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize)))
> > > > + maxlen = sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize);
> > > > + if (bitmap[2] != 0)
> > > > + maxlen += (sizeof(ncf->ncf_cb_mtime.tv_sec) +
> > > > + sizeof(ncf->ncf_cb_mtime.tv_nsec)) * 2;
> > > > + if (attrlen > maxlen)
> > > > return -NFSERR_BAD_XDR;
> > > > status = decode_cb_fattr4(xdr, bitmap, ncf);
> > > > return status;
> > > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > > > index 62f9aeb159d0f2ab4d293bf5c0c56ad7b86eb9d6..2c8d2bb5261ad189c6dfb1c4050c23d8cf061325 100644
> > > > --- a/fs/nfsd/nfs4state.c
> > > > +++ b/fs/nfsd/nfs4state.c
> > > > @@ -5803,13 +5803,14 @@ static struct nfs4_delegation *
> > > > nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > > > struct svc_fh *parent)
> > > > {
> > > > - int status = 0;
> > > > + bool deleg_ts = open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS;
> > > > struct nfs4_client *clp = stp->st_stid.sc_client;
> > > > struct nfs4_file *fp = stp->st_stid.sc_file;
> > > > struct nfs4_clnt_odstate *odstate = stp->st_clnt_odstate;
> > > > struct nfs4_delegation *dp;
> > > > struct nfsd_file *nf = NULL;
> > > > struct file_lease *fl;
> > > > + int status = 0;
> > > > u32 dl_type;
> > > >
> > > > /*
> > > > @@ -5834,7 +5835,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > > > */
> > > > if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_BOTH) {
> > > > nf = find_rw_file(fp);
> > > > - dl_type = OPEN_DELEGATE_WRITE;
> > > > + dl_type = deleg_ts ? OPEN_DELEGATE_WRITE_ATTRS_DELEG : OPEN_DELEGATE_WRITE;
> > > > }
> > > >
> > > > /*
> > > > @@ -5843,7 +5844,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > > > */
> > > > if (!nf && (open->op_share_access & NFS4_SHARE_ACCESS_READ)) {
> > > > nf = find_readable_file(fp);
> > > > - dl_type = OPEN_DELEGATE_READ;
> > > > + dl_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG : OPEN_DELEGATE_READ;
> > > > }
> > > >
> > > > if (!nf)
> > > > @@ -6001,13 +6002,14 @@ static void
> > > > nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > > > struct svc_fh *currentfh)
> > > > {
> > > > - struct nfs4_delegation *dp;
> > > > + bool deleg_ts = open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS;
> > > > struct nfs4_openowner *oo = openowner(stp->st_stateowner);
> > > > struct nfs4_client *clp = stp->st_stid.sc_client;
> > > > struct svc_fh *parent = NULL;
> > > > - int cb_up;
> > > > - int status = 0;
> > > > + struct nfs4_delegation *dp;
> > > > struct kstat stat;
> > > > + int status = 0;
> > > > + int cb_up;
> > > >
> > > > cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
> > > > open->op_recall = false;
> > > > @@ -6048,12 +6050,14 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > > > destroy_delegation(dp);
> > > > goto out_no_deleg;
> > > > }
> > > > - open->op_delegate_type = OPEN_DELEGATE_WRITE;
> > > > + open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_WRITE_ATTRS_DELEG :
> > > > + OPEN_DELEGATE_WRITE;
> > > > dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
> > > > dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
> > > > trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> > > > } else {
> > > > - open->op_delegate_type = OPEN_DELEGATE_READ;
> > > > + open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG :
> > > > + OPEN_DELEGATE_READ;
> > > > trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
> > > > }
> > > > nfs4_put_stid(&dp->dl_stid);
> > > > @@ -8887,6 +8891,78 @@ nfsd4_get_writestateid(struct nfsd4_compound_state *cstate,
> > > > get_stateid(cstate, &u->write.wr_stateid);
> > > > }
> > > >
> > > > +/**
> > > > + * set_cb_time - vet and set the timespec for a cb_getattr update
> > > > + * @cb: timestamp from the CB_GETATTR response
> > > > + * @orig: original timestamp in the inode
> > > > + * @now: current time
> > > > + *
> > > > + * Given a timestamp in a CB_GETATTR response, check it against the
> > > > + * current timestamp in the inode and the current time. Returns true
> > > > + * if the inode's timestamp needs to be updated, and false otherwise.
> > > > + * @cb may also be changed if the timestamp needs to be clamped.
> > > > + */
> > > > +static bool set_cb_time(struct timespec64 *cb, const struct timespec64 *orig,
> > > > + const struct timespec64 *now)
> > > > +{
> > > > +
> > > > + /*
> > > > + * "When the time presented is before the original time, then the
> > > > + * update is ignored." Also no need to update if there is no change.
> > > > + */
> > > > + if (timespec64_compare(cb, orig) <= 0)
> > > > + return false;
> > > > +
> > > > + /*
> > > > + * "When the time presented is in the future, the server can either
> > > > + * clamp the new time to the current time, or it may
> > > > + * return NFS4ERR_DELAY to the client, allowing it to retry."
> > > > + */
> > > > + if (timespec64_compare(cb, now) > 0) {
> > > > + /* clamp it */
> > > > + *cb = *now;
> > > > + }
> > > > +
> > > > + return true;
> > > > +}
> > > > +
> > > > +static int cb_getattr_update_times(struct dentry *dentry, struct nfs4_delegation *dp)
> > > > +{
> > > > + struct inode *inode = d_inode(dentry);
> > > > + struct timespec64 now = current_time(inode);
> > > > + struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
> > > > + struct iattr attrs = { };
> > > > + int ret;
> > > > +
> > > > + if (deleg_attrs_deleg(dp->dl_type)) {
> > > > + struct timespec64 atime = inode_get_atime(inode);
> > > > + struct timespec64 mtime = inode_get_mtime(inode);
> > > > +
> > > > + attrs.ia_atime = ncf->ncf_cb_atime;
> > > > + attrs.ia_mtime = ncf->ncf_cb_mtime;
> > > > +
> > > > + if (set_cb_time(&attrs.ia_atime, &atime, &now))
> > > > + attrs.ia_valid |= ATTR_ATIME | ATTR_ATIME_SET;
> > > > +
> > > > + if (set_cb_time(&attrs.ia_mtime, &mtime, &now)) {
> > > > + attrs.ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET;
> > > > + attrs.ia_ctime = attrs.ia_mtime;
> > > > + }
> > > > + } else {
> > > > + attrs.ia_valid |= ATTR_MTIME | ATTR_CTIME;
> > > > + attrs.ia_mtime = attrs.ia_ctime = now;
> > > > + }
> > > > +
> > > > + if (!attrs.ia_valid)
> > > > + return 0;
> > > > +
> > > > + attrs.ia_valid |= ATTR_DELEG;
> > > > + inode_lock(inode);
> > > > + ret = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
> > > > + inode_unlock(inode);
> > > > + return ret;
> > > > +}
> > > > +
> > > > /**
> > > > * nfsd4_deleg_getattr_conflict - Recall if GETATTR causes conflict
> > > > * @rqstp: RPC transaction context
> > > > @@ -8913,7 +8989,6 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
> > > > struct file_lock_context *ctx;
> > > > struct nfs4_delegation *dp = NULL;
> > > > struct file_lease *fl;
> > > > - struct iattr attrs;
> > > > struct nfs4_cb_fattr *ncf;
> > > > struct inode *inode = d_inode(dentry);
> > > >
> > > > @@ -8975,11 +9050,7 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
> > > > * not update the file's metadata with the client's
> > > > * modified size
> > > > */
> > > > - attrs.ia_mtime = attrs.ia_ctime = current_time(inode);
> > > > - attrs.ia_valid = ATTR_MTIME | ATTR_CTIME | ATTR_DELEG;
> > > > - inode_lock(inode);
> > > > - err = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
> > > > - inode_unlock(inode);
> > > > + err = cb_getattr_update_times(dentry, dp);
> > > > if (err) {
> > > > status = nfserrno(err);
> > > > goto out_status;
> > > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > > > index 1c9d9349e4447c0078c7de0d533cf6278941679d..0e9f59f6be015bfa37893973f38fec880ff4c0b1 100644
> > > > --- a/fs/nfsd/nfs4xdr.c
> > > > +++ b/fs/nfsd/nfs4xdr.c
> > > > @@ -3409,6 +3409,7 @@ static __be32 nfsd4_encode_fattr4_xattr_support(struct xdr_stream *xdr,
> > > > #define NFSD_OA_SHARE_ACCESS_WANT (BIT(OPEN_ARGS_SHARE_ACCESS_WANT_ANY_DELEG) | \
> > > > BIT(OPEN_ARGS_SHARE_ACCESS_WANT_NO_DELEG) | \
> > > > BIT(OPEN_ARGS_SHARE_ACCESS_WANT_CANCEL) | \
> > > > + BIT(OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS) | \
> > > > BIT(OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION))
> > > >
> > > > #define NFSD_OA_OPEN_CLAIM (BIT(OPEN_ARGS_OPEN_CLAIM_NULL) | \
> > > > @@ -3602,7 +3603,11 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
> > > > if (status)
> > > > goto out;
> > > > }
> > > > - if (attrmask[0] & (FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE)) {
> > > > + if ((attrmask[0] & (FATTR4_WORD0_CHANGE |
> > > > + FATTR4_WORD0_SIZE)) ||
> > > > + (attrmask[1] & (FATTR4_WORD1_TIME_ACCESS |
> > > > + FATTR4_WORD1_TIME_MODIFY |
> > > > + FATTR4_WORD1_TIME_METADATA))) {
> > > > status = nfsd4_deleg_getattr_conflict(rqstp, dentry, &dp);
> > > > if (status)
> > > > goto out;
> > > > @@ -3617,8 +3622,14 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
> > > > if (ncf->ncf_file_modified) {
> > > > ++ncf->ncf_initial_cinfo;
> > > > args.stat.size = ncf->ncf_cur_fsize;
> > > > + if (!timespec64_is_epoch(&ncf->ncf_cb_mtime))
> > > > + args.stat.mtime = ncf->ncf_cb_mtime;
> > > > }
> > > > args.change_attr = ncf->ncf_initial_cinfo;
> > > > +
> > > > + if (!timespec64_is_epoch(&ncf->ncf_cb_atime))
> > > > + args.stat.atime = ncf->ncf_cb_atime;
> > > > +
> > > > nfs4_put_stid(&dp->dl_stid);
> > > > } else {
> > > > args.change_attr = nfsd4_change_attribute(&args.stat);
> > > > diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> > > > index 1955c8e9c4c793728fa75dd136cadc735245483f..004415651295891b3440f52a4c986e3a668a48cb 100644
> > > > --- a/fs/nfsd/nfsd.h
> > > > +++ b/fs/nfsd/nfsd.h
> > > > @@ -459,6 +459,8 @@ enum {
> > > > FATTR4_WORD2_MODE_UMASK | \
> > > > NFSD4_2_SECURITY_ATTRS | \
> > > > FATTR4_WORD2_XATTR_SUPPORT | \
> > > > + FATTR4_WORD2_TIME_DELEG_ACCESS | \
> > > > + FATTR4_WORD2_TIME_DELEG_MODIFY | \
> > >
> > > This breaks 4.2 mounts for me (in latest nfsd-nexT). OPEN fails.
> >
> > Yep, we're on it.
> >
>
> I see the problem. The OPEN_XOR_DELEGATION patch reworked some of the
> NFS4_SHARE_WANT_* handling, and this patch relied on those changes.
> When that patch was dropped, it broke this patch.
>
> What we should probably do is split out the flag rework from that patch
> into a separate patch that this can rely on. Not sure if you want to
> embark upon all of that during the merge window though. It may be
> better to just drop these patches as well.
I'm going to drop all of delstid -- it's just way too late to do any
real surgery on this stuff, and it seems very tightly interwoven.
> > > By setting these bits we tell the client that we support timestamp
> > > delegation, but you haven't updated nfsd4_decode_share_access() to
> > > understand NFS4_SHARE_WANT_DELEG_TIMESTAMPS in the 'share' flags for an
> > > OPEN request. So the server responds with BADXDR to OPEN requests now.
> > >
> > > Mounting with v4.1 still works.
> > >
> > > NeilBrown
> > >
> > >
> > > > FATTR4_WORD2_OPEN_ARGUMENTS)
> > > >
> > > > extern const u32 nfsd_suppattrs[3][3];
> > > > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> > > > index 9d0e844515aa6ea0ec62f2b538ecc2c6a5e34652..6351e6eca7cc63ccf82a3a081cef39042d52f4e9 100644
> > > > --- a/fs/nfsd/state.h
> > > > +++ b/fs/nfsd/state.h
> > > > @@ -142,6 +142,8 @@ struct nfs4_cb_fattr {
> > > > /* from CB_GETATTR reply */
> > > > u64 ncf_cb_change;
> > > > u64 ncf_cb_fsize;
> > > > + struct timespec64 ncf_cb_mtime;
> > > > + struct timespec64 ncf_cb_atime;
> > > >
> > > > unsigned long ncf_cb_flags;
> > > > bool ncf_file_modified;
> > > > diff --git a/fs/nfsd/xdr4cb.h b/fs/nfsd/xdr4cb.h
> > > > index e8b00309c449fe2667f7d48cda88ec0cff924f93..f1a315cd31b74f73f1d52702ae7b5c93d51ddf82 100644
> > > > --- a/fs/nfsd/xdr4cb.h
> > > > +++ b/fs/nfsd/xdr4cb.h
> > > > @@ -59,16 +59,20 @@
> > > > * 1: CB_GETATTR opcode (32-bit)
> > > > * N: file_handle
> > > > * 1: number of entry in attribute array (32-bit)
> > > > - * 1: entry 0 in attribute array (32-bit)
> > > > + * 3: entry 0-2 in attribute array (32-bit * 3)
> > > > */
> > > > #define NFS4_enc_cb_getattr_sz (cb_compound_enc_hdr_sz + \
> > > > cb_sequence_enc_sz + \
> > > > - 1 + enc_nfs4_fh_sz + 1 + 1)
> > > > + 1 + enc_nfs4_fh_sz + 1 + 3)
> > > > /*
> > > > * 4: fattr_bitmap_maxsz
> > > > * 1: attribute array len
> > > > * 2: change attr (64-bit)
> > > > * 2: size (64-bit)
> > > > + * 2: atime.seconds (64-bit)
> > > > + * 1: atime.nanoseconds (32-bit)
> > > > + * 2: mtime.seconds (64-bit)
> > > > + * 1: mtime.nanoseconds (32-bit)
> > > > */
> > > > #define NFS4_dec_cb_getattr_sz (cb_compound_dec_hdr_sz + \
> > > > - cb_sequence_dec_sz + 4 + 1 + 2 + 2 + op_dec_sz)
> > > > + cb_sequence_dec_sz + 4 + 1 + 2 + 2 + 2 + 1 + 2 + 1 + op_dec_sz)
> > > > diff --git a/include/linux/time64.h b/include/linux/time64.h
> > > > index f1bcea8c124a361b6c1e3c98ef915840c22a8413..9934331c7b86b7fb981c7aec0494ac2f5e72977e 100644
> > > > --- a/include/linux/time64.h
> > > > +++ b/include/linux/time64.h
> > > > @@ -49,6 +49,11 @@ static inline int timespec64_equal(const struct timespec64 *a,
> > > > return (a->tv_sec == b->tv_sec) && (a->tv_nsec == b->tv_nsec);
> > > > }
> > > >
> > > > +static inline bool timespec64_is_epoch(const struct timespec64 *ts)
> > > > +{
> > > > + return ts->tv_sec == 0 && ts->tv_nsec == 0;
> > > > +}
> > > > +
> > > > /*
> > > > * lhs < rhs: return <0
> > > > * lhs == rhs: return 0
> > > >
> > > > --
> > > > 2.47.0
> > > >
> > > >
> > > >
> > >
> >
>
> --
> Jeff Layton <jlayton@kernel.org>
--
Chuck Lever
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/6] nfsd: add support for delegated timestamps
2024-11-19 1:01 ` NeilBrown
2024-11-19 1:03 ` Chuck Lever
@ 2024-11-19 1:31 ` Chuck Lever
1 sibling, 0 replies; 18+ messages in thread
From: Chuck Lever @ 2024-11-19 1:31 UTC (permalink / raw)
To: NeilBrown
Cc: Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Jonathan Corbet, Trond Myklebust, Anna Schumaker, Thomas Haynes,
linux-nfs, linux-kernel, linux-doc
On Tue, Nov 19, 2024 at 12:01:33PM +1100, NeilBrown wrote:
> On Tue, 15 Oct 2024, Jeff Layton wrote:
> > Add support for the delegated timestamps on write delegations. This
> > allows the server to proxy timestamps from the delegation holder to
> > other clients that are doing GETATTRs vs. the same inode.
> >
> > When OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS bit is set in the OPEN
> > call, set the dl_type to the *_ATTRS_DELEG flavor of delegation.
> >
> > Add timespec64 fields to nfs4_cb_fattr and decode the timestamps into
> > those. Vet those timestamps according to the delstid spec and update
> > the inode attrs if necessary.
> >
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > ---
> > fs/nfsd/nfs4callback.c | 42 +++++++++++++++++++--
> > fs/nfsd/nfs4state.c | 99 +++++++++++++++++++++++++++++++++++++++++++-------
> > fs/nfsd/nfs4xdr.c | 13 ++++++-
> > fs/nfsd/nfsd.h | 2 +
> > fs/nfsd/state.h | 2 +
> > fs/nfsd/xdr4cb.h | 10 +++--
> > include/linux/time64.h | 5 +++
> > 7 files changed, 151 insertions(+), 22 deletions(-)
> >
> > diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> > index 776838bb83e6b707a4df76326cdc68f32daf1755..08245596289a960eb8b2e78df276544e7d3f4ff8 100644
> > --- a/fs/nfsd/nfs4callback.c
> > +++ b/fs/nfsd/nfs4callback.c
> > @@ -42,6 +42,7 @@
> > #include "trace.h"
> > #include "xdr4cb.h"
> > #include "xdr4.h"
> > +#include "nfs4xdr_gen.h"
> >
> > #define NFSDDBG_FACILITY NFSDDBG_PROC
> >
> > @@ -93,12 +94,35 @@ static int decode_cb_fattr4(struct xdr_stream *xdr, uint32_t *bitmap,
> > {
> > fattr->ncf_cb_change = 0;
> > fattr->ncf_cb_fsize = 0;
> > + fattr->ncf_cb_atime.tv_sec = 0;
> > + fattr->ncf_cb_atime.tv_nsec = 0;
> > + fattr->ncf_cb_mtime.tv_sec = 0;
> > + fattr->ncf_cb_mtime.tv_nsec = 0;
> > +
> > if (bitmap[0] & FATTR4_WORD0_CHANGE)
> > if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_change) < 0)
> > return -NFSERR_BAD_XDR;
> > if (bitmap[0] & FATTR4_WORD0_SIZE)
> > if (xdr_stream_decode_u64(xdr, &fattr->ncf_cb_fsize) < 0)
> > return -NFSERR_BAD_XDR;
> > + if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
> > + fattr4_time_deleg_access access;
> > +
> > + if (!xdrgen_decode_fattr4_time_deleg_access(xdr, &access))
> > + return -NFSERR_BAD_XDR;
> > + fattr->ncf_cb_atime.tv_sec = access.seconds;
> > + fattr->ncf_cb_atime.tv_nsec = access.nseconds;
> > +
> > + }
> > + if (bitmap[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
> > + fattr4_time_deleg_modify modify;
> > +
> > + if (!xdrgen_decode_fattr4_time_deleg_modify(xdr, &modify))
> > + return -NFSERR_BAD_XDR;
> > + fattr->ncf_cb_mtime.tv_sec = modify.seconds;
> > + fattr->ncf_cb_mtime.tv_nsec = modify.nseconds;
> > +
> > + }
> > return 0;
> > }
> >
> > @@ -364,15 +388,21 @@ encode_cb_getattr4args(struct xdr_stream *xdr, struct nfs4_cb_compound_hdr *hdr,
> > struct nfs4_delegation *dp = container_of(fattr, struct nfs4_delegation, dl_cb_fattr);
> > struct knfsd_fh *fh = &dp->dl_stid.sc_file->fi_fhandle;
> > struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
> > - u32 bmap[1];
> > + u32 bmap_size = 1;
> > + u32 bmap[3];
> >
> > bmap[0] = FATTR4_WORD0_SIZE;
> > if (!ncf->ncf_file_modified)
> > bmap[0] |= FATTR4_WORD0_CHANGE;
> >
> > + if (deleg_attrs_deleg(dp->dl_type)) {
> > + bmap[1] = 0;
> > + bmap[2] = FATTR4_WORD2_TIME_DELEG_ACCESS | FATTR4_WORD2_TIME_DELEG_MODIFY;
> > + bmap_size = 3;
> > + }
> > encode_nfs_cb_opnum4(xdr, OP_CB_GETATTR);
> > encode_nfs_fh4(xdr, fh);
> > - encode_bitmap4(xdr, bmap, ARRAY_SIZE(bmap));
> > + encode_bitmap4(xdr, bmap, bmap_size);
> > hdr->nops++;
> > }
> >
> > @@ -597,7 +627,7 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
> > struct nfs4_cb_compound_hdr hdr;
> > int status;
> > u32 bitmap[3] = {0};
> > - u32 attrlen;
> > + u32 attrlen, maxlen;
> > struct nfs4_cb_fattr *ncf =
> > container_of(cb, struct nfs4_cb_fattr, ncf_getattr);
> >
> > @@ -616,7 +646,11 @@ static int nfs4_xdr_dec_cb_getattr(struct rpc_rqst *rqstp,
> > return -NFSERR_BAD_XDR;
> > if (xdr_stream_decode_u32(xdr, &attrlen) < 0)
> > return -NFSERR_BAD_XDR;
> > - if (attrlen > (sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize)))
> > + maxlen = sizeof(ncf->ncf_cb_change) + sizeof(ncf->ncf_cb_fsize);
> > + if (bitmap[2] != 0)
> > + maxlen += (sizeof(ncf->ncf_cb_mtime.tv_sec) +
> > + sizeof(ncf->ncf_cb_mtime.tv_nsec)) * 2;
> > + if (attrlen > maxlen)
> > return -NFSERR_BAD_XDR;
> > status = decode_cb_fattr4(xdr, bitmap, ncf);
> > return status;
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 62f9aeb159d0f2ab4d293bf5c0c56ad7b86eb9d6..2c8d2bb5261ad189c6dfb1c4050c23d8cf061325 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -5803,13 +5803,14 @@ static struct nfs4_delegation *
> > nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > struct svc_fh *parent)
> > {
> > - int status = 0;
> > + bool deleg_ts = open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS;
> > struct nfs4_client *clp = stp->st_stid.sc_client;
> > struct nfs4_file *fp = stp->st_stid.sc_file;
> > struct nfs4_clnt_odstate *odstate = stp->st_clnt_odstate;
> > struct nfs4_delegation *dp;
> > struct nfsd_file *nf = NULL;
> > struct file_lease *fl;
> > + int status = 0;
> > u32 dl_type;
> >
> > /*
> > @@ -5834,7 +5835,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > */
> > if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_BOTH) {
> > nf = find_rw_file(fp);
> > - dl_type = OPEN_DELEGATE_WRITE;
> > + dl_type = deleg_ts ? OPEN_DELEGATE_WRITE_ATTRS_DELEG : OPEN_DELEGATE_WRITE;
> > }
> >
> > /*
> > @@ -5843,7 +5844,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > */
> > if (!nf && (open->op_share_access & NFS4_SHARE_ACCESS_READ)) {
> > nf = find_readable_file(fp);
> > - dl_type = OPEN_DELEGATE_READ;
> > + dl_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG : OPEN_DELEGATE_READ;
> > }
> >
> > if (!nf)
> > @@ -6001,13 +6002,14 @@ static void
> > nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > struct svc_fh *currentfh)
> > {
> > - struct nfs4_delegation *dp;
> > + bool deleg_ts = open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS;
> > struct nfs4_openowner *oo = openowner(stp->st_stateowner);
> > struct nfs4_client *clp = stp->st_stid.sc_client;
> > struct svc_fh *parent = NULL;
> > - int cb_up;
> > - int status = 0;
> > + struct nfs4_delegation *dp;
> > struct kstat stat;
> > + int status = 0;
> > + int cb_up;
> >
> > cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client);
> > open->op_recall = false;
> > @@ -6048,12 +6050,14 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > destroy_delegation(dp);
> > goto out_no_deleg;
> > }
> > - open->op_delegate_type = OPEN_DELEGATE_WRITE;
> > + open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_WRITE_ATTRS_DELEG :
> > + OPEN_DELEGATE_WRITE;
> > dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
> > dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
> > trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> > } else {
> > - open->op_delegate_type = OPEN_DELEGATE_READ;
> > + open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG :
> > + OPEN_DELEGATE_READ;
> > trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
> > }
> > nfs4_put_stid(&dp->dl_stid);
> > @@ -8887,6 +8891,78 @@ nfsd4_get_writestateid(struct nfsd4_compound_state *cstate,
> > get_stateid(cstate, &u->write.wr_stateid);
> > }
> >
> > +/**
> > + * set_cb_time - vet and set the timespec for a cb_getattr update
> > + * @cb: timestamp from the CB_GETATTR response
> > + * @orig: original timestamp in the inode
> > + * @now: current time
> > + *
> > + * Given a timestamp in a CB_GETATTR response, check it against the
> > + * current timestamp in the inode and the current time. Returns true
> > + * if the inode's timestamp needs to be updated, and false otherwise.
> > + * @cb may also be changed if the timestamp needs to be clamped.
> > + */
> > +static bool set_cb_time(struct timespec64 *cb, const struct timespec64 *orig,
> > + const struct timespec64 *now)
> > +{
> > +
> > + /*
> > + * "When the time presented is before the original time, then the
> > + * update is ignored." Also no need to update if there is no change.
> > + */
> > + if (timespec64_compare(cb, orig) <= 0)
> > + return false;
> > +
> > + /*
> > + * "When the time presented is in the future, the server can either
> > + * clamp the new time to the current time, or it may
> > + * return NFS4ERR_DELAY to the client, allowing it to retry."
> > + */
> > + if (timespec64_compare(cb, now) > 0) {
> > + /* clamp it */
> > + *cb = *now;
> > + }
> > +
> > + return true;
> > +}
> > +
> > +static int cb_getattr_update_times(struct dentry *dentry, struct nfs4_delegation *dp)
> > +{
> > + struct inode *inode = d_inode(dentry);
> > + struct timespec64 now = current_time(inode);
> > + struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
> > + struct iattr attrs = { };
> > + int ret;
> > +
> > + if (deleg_attrs_deleg(dp->dl_type)) {
> > + struct timespec64 atime = inode_get_atime(inode);
> > + struct timespec64 mtime = inode_get_mtime(inode);
> > +
> > + attrs.ia_atime = ncf->ncf_cb_atime;
> > + attrs.ia_mtime = ncf->ncf_cb_mtime;
> > +
> > + if (set_cb_time(&attrs.ia_atime, &atime, &now))
> > + attrs.ia_valid |= ATTR_ATIME | ATTR_ATIME_SET;
> > +
> > + if (set_cb_time(&attrs.ia_mtime, &mtime, &now)) {
> > + attrs.ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET;
> > + attrs.ia_ctime = attrs.ia_mtime;
> > + }
> > + } else {
> > + attrs.ia_valid |= ATTR_MTIME | ATTR_CTIME;
> > + attrs.ia_mtime = attrs.ia_ctime = now;
> > + }
> > +
> > + if (!attrs.ia_valid)
> > + return 0;
> > +
> > + attrs.ia_valid |= ATTR_DELEG;
> > + inode_lock(inode);
> > + ret = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
> > + inode_unlock(inode);
> > + return ret;
> > +}
> > +
> > /**
> > * nfsd4_deleg_getattr_conflict - Recall if GETATTR causes conflict
> > * @rqstp: RPC transaction context
> > @@ -8913,7 +8989,6 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
> > struct file_lock_context *ctx;
> > struct nfs4_delegation *dp = NULL;
> > struct file_lease *fl;
> > - struct iattr attrs;
> > struct nfs4_cb_fattr *ncf;
> > struct inode *inode = d_inode(dentry);
> >
> > @@ -8975,11 +9050,7 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
> > * not update the file's metadata with the client's
> > * modified size
> > */
> > - attrs.ia_mtime = attrs.ia_ctime = current_time(inode);
> > - attrs.ia_valid = ATTR_MTIME | ATTR_CTIME | ATTR_DELEG;
> > - inode_lock(inode);
> > - err = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
> > - inode_unlock(inode);
> > + err = cb_getattr_update_times(dentry, dp);
> > if (err) {
> > status = nfserrno(err);
> > goto out_status;
> > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> > index 1c9d9349e4447c0078c7de0d533cf6278941679d..0e9f59f6be015bfa37893973f38fec880ff4c0b1 100644
> > --- a/fs/nfsd/nfs4xdr.c
> > +++ b/fs/nfsd/nfs4xdr.c
> > @@ -3409,6 +3409,7 @@ static __be32 nfsd4_encode_fattr4_xattr_support(struct xdr_stream *xdr,
> > #define NFSD_OA_SHARE_ACCESS_WANT (BIT(OPEN_ARGS_SHARE_ACCESS_WANT_ANY_DELEG) | \
> > BIT(OPEN_ARGS_SHARE_ACCESS_WANT_NO_DELEG) | \
> > BIT(OPEN_ARGS_SHARE_ACCESS_WANT_CANCEL) | \
> > + BIT(OPEN_ARGS_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS) | \
> > BIT(OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION))
> >
> > #define NFSD_OA_OPEN_CLAIM (BIT(OPEN_ARGS_OPEN_CLAIM_NULL) | \
> > @@ -3602,7 +3603,11 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
> > if (status)
> > goto out;
> > }
> > - if (attrmask[0] & (FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE)) {
> > + if ((attrmask[0] & (FATTR4_WORD0_CHANGE |
> > + FATTR4_WORD0_SIZE)) ||
> > + (attrmask[1] & (FATTR4_WORD1_TIME_ACCESS |
> > + FATTR4_WORD1_TIME_MODIFY |
> > + FATTR4_WORD1_TIME_METADATA))) {
> > status = nfsd4_deleg_getattr_conflict(rqstp, dentry, &dp);
> > if (status)
> > goto out;
> > @@ -3617,8 +3622,14 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
> > if (ncf->ncf_file_modified) {
> > ++ncf->ncf_initial_cinfo;
> > args.stat.size = ncf->ncf_cur_fsize;
> > + if (!timespec64_is_epoch(&ncf->ncf_cb_mtime))
> > + args.stat.mtime = ncf->ncf_cb_mtime;
> > }
> > args.change_attr = ncf->ncf_initial_cinfo;
> > +
> > + if (!timespec64_is_epoch(&ncf->ncf_cb_atime))
> > + args.stat.atime = ncf->ncf_cb_atime;
> > +
> > nfs4_put_stid(&dp->dl_stid);
> > } else {
> > args.change_attr = nfsd4_change_attribute(&args.stat);
> > diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> > index 1955c8e9c4c793728fa75dd136cadc735245483f..004415651295891b3440f52a4c986e3a668a48cb 100644
> > --- a/fs/nfsd/nfsd.h
> > +++ b/fs/nfsd/nfsd.h
> > @@ -459,6 +459,8 @@ enum {
> > FATTR4_WORD2_MODE_UMASK | \
> > NFSD4_2_SECURITY_ATTRS | \
> > FATTR4_WORD2_XATTR_SUPPORT | \
> > + FATTR4_WORD2_TIME_DELEG_ACCESS | \
> > + FATTR4_WORD2_TIME_DELEG_MODIFY | \
>
> This breaks 4.2 mounts for me (in latest nfsd-nexT). OPEN fails.
>
> By setting these bits we tell the client that we support timestamp
> delegation, but you haven't updated nfsd4_decode_share_access() to
> understand NFS4_SHARE_WANT_DELEG_TIMESTAMPS in the 'share' flags for an
> OPEN request. So the server responds with BADXDR to OPEN requests now.
>
> Mounting with v4.1 still works.
I've pushed a version of nfsd-next that doesn't have the delstid
patch series in it. My CI facilities are running other tests over
night, so this has been compile-tested only, until tomorrow
(US/Eastern).
> NeilBrown
>
>
> > FATTR4_WORD2_OPEN_ARGUMENTS)
> >
> > extern const u32 nfsd_suppattrs[3][3];
> > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> > index 9d0e844515aa6ea0ec62f2b538ecc2c6a5e34652..6351e6eca7cc63ccf82a3a081cef39042d52f4e9 100644
> > --- a/fs/nfsd/state.h
> > +++ b/fs/nfsd/state.h
> > @@ -142,6 +142,8 @@ struct nfs4_cb_fattr {
> > /* from CB_GETATTR reply */
> > u64 ncf_cb_change;
> > u64 ncf_cb_fsize;
> > + struct timespec64 ncf_cb_mtime;
> > + struct timespec64 ncf_cb_atime;
> >
> > unsigned long ncf_cb_flags;
> > bool ncf_file_modified;
> > diff --git a/fs/nfsd/xdr4cb.h b/fs/nfsd/xdr4cb.h
> > index e8b00309c449fe2667f7d48cda88ec0cff924f93..f1a315cd31b74f73f1d52702ae7b5c93d51ddf82 100644
> > --- a/fs/nfsd/xdr4cb.h
> > +++ b/fs/nfsd/xdr4cb.h
> > @@ -59,16 +59,20 @@
> > * 1: CB_GETATTR opcode (32-bit)
> > * N: file_handle
> > * 1: number of entry in attribute array (32-bit)
> > - * 1: entry 0 in attribute array (32-bit)
> > + * 3: entry 0-2 in attribute array (32-bit * 3)
> > */
> > #define NFS4_enc_cb_getattr_sz (cb_compound_enc_hdr_sz + \
> > cb_sequence_enc_sz + \
> > - 1 + enc_nfs4_fh_sz + 1 + 1)
> > + 1 + enc_nfs4_fh_sz + 1 + 3)
> > /*
> > * 4: fattr_bitmap_maxsz
> > * 1: attribute array len
> > * 2: change attr (64-bit)
> > * 2: size (64-bit)
> > + * 2: atime.seconds (64-bit)
> > + * 1: atime.nanoseconds (32-bit)
> > + * 2: mtime.seconds (64-bit)
> > + * 1: mtime.nanoseconds (32-bit)
> > */
> > #define NFS4_dec_cb_getattr_sz (cb_compound_dec_hdr_sz + \
> > - cb_sequence_dec_sz + 4 + 1 + 2 + 2 + op_dec_sz)
> > + cb_sequence_dec_sz + 4 + 1 + 2 + 2 + 2 + 1 + 2 + 1 + op_dec_sz)
> > diff --git a/include/linux/time64.h b/include/linux/time64.h
> > index f1bcea8c124a361b6c1e3c98ef915840c22a8413..9934331c7b86b7fb981c7aec0494ac2f5e72977e 100644
> > --- a/include/linux/time64.h
> > +++ b/include/linux/time64.h
> > @@ -49,6 +49,11 @@ static inline int timespec64_equal(const struct timespec64 *a,
> > return (a->tv_sec == b->tv_sec) && (a->tv_nsec == b->tv_nsec);
> > }
> >
> > +static inline bool timespec64_is_epoch(const struct timespec64 *ts)
> > +{
> > + return ts->tv_sec == 0 && ts->tv_nsec == 0;
> > +}
> > +
> > /*
> > * lhs < rhs: return <0
> > * lhs == rhs: return 0
> >
> > --
> > 2.47.0
> >
> >
> >
>
--
Chuck Lever
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 6/6] nfsd: handle delegated timestamps in SETATTR
2024-10-14 19:26 [PATCH 0/6] nfsd: update the delstid patches for latest draft changes Jeff Layton
` (4 preceding siblings ...)
2024-10-14 19:26 ` [PATCH 5/6] nfsd: add support for delegated timestamps Jeff Layton
@ 2024-10-14 19:26 ` Jeff Layton
2024-10-15 19:17 ` [PATCH 0/6] nfsd: update the delstid patches for latest draft changes cel
2024-10-17 22:39 ` Olga Kornievskaia
7 siblings, 0 replies; 18+ messages in thread
From: Jeff Layton @ 2024-10-14 19:26 UTC (permalink / raw)
To: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Jonathan Corbet, Trond Myklebust, Anna Schumaker, Thomas Haynes
Cc: linux-nfs, linux-kernel, linux-doc, Jeff Layton
Allow SETATTR to handle delegated timestamps. This patch assumes that
only the delegation holder has the ability to set the timestamps in this
way, so we allow this only if the SETATTR stateid refers to a
*_ATTRS_DELEG delegation.
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
fs/nfsd/nfs4proc.c | 26 +++++++++++++++++++++++---
fs/nfsd/nfs4xdr.c | 20 ++++++++++++++++++++
2 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index fcce1f2e5454b0f3c4aa4b85fb4a4e24b2dee932..320c4f79662e65848dc824885566d48e696fe97c 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1133,18 +1133,38 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
.na_iattr = &setattr->sa_iattr,
.na_seclabel = &setattr->sa_label,
};
+ bool save_no_wcc, deleg_attrs;
+ struct nfs4_stid *st = NULL;
struct inode *inode;
__be32 status = nfs_ok;
- bool save_no_wcc;
int err;
- if (setattr->sa_iattr.ia_valid & ATTR_SIZE) {
+ deleg_attrs = setattr->sa_bmval[2] & (FATTR4_WORD2_TIME_DELEG_ACCESS |
+ FATTR4_WORD2_TIME_DELEG_MODIFY);
+
+ if (deleg_attrs || (setattr->sa_iattr.ia_valid & ATTR_SIZE)) {
status = nfs4_preprocess_stateid_op(rqstp, cstate,
&cstate->current_fh, &setattr->sa_stateid,
- WR_STATE, NULL, NULL);
+ WR_STATE, NULL, &st);
if (status)
return status;
}
+
+ if (deleg_attrs) {
+ status = nfserr_bad_stateid;
+ if (st->sc_type & SC_TYPE_DELEG) {
+ struct nfs4_delegation *dp = delegstateid(st);
+
+ /* Only for *_ATTRS_DELEG flavors */
+ if (deleg_attrs_deleg(dp->dl_type))
+ status = nfs_ok;
+ }
+ }
+ if (st)
+ nfs4_put_stid(st);
+ if (status)
+ return status;
+
err = fh_want_write(&cstate->current_fh);
if (err)
return nfserrno(err);
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 0e9f59f6be015bfa37893973f38fec880ff4c0b1..857b39fcdb772585601f760705564968bb0d554c 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -521,6 +521,26 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
*umask = mask & S_IRWXUGO;
iattr->ia_valid |= ATTR_MODE;
}
+ if (bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
+ fattr4_time_deleg_access access;
+
+ if (!xdrgen_decode_fattr4_time_deleg_access(argp->xdr, &access))
+ return nfserr_bad_xdr;
+ iattr->ia_atime.tv_sec = access.seconds;
+ iattr->ia_atime.tv_nsec = access.nseconds;
+ iattr->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET | ATTR_DELEG;
+ }
+ if (bmval[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
+ fattr4_time_deleg_modify modify;
+
+ if (!xdrgen_decode_fattr4_time_deleg_modify(argp->xdr, &modify))
+ return nfserr_bad_xdr;
+ iattr->ia_mtime.tv_sec = modify.seconds;
+ iattr->ia_mtime.tv_nsec = modify.nseconds;
+ iattr->ia_ctime.tv_sec = modify.seconds;
+ iattr->ia_ctime.tv_nsec = modify.seconds;
+ iattr->ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET | ATTR_DELEG;
+ }
/* request sanity: did attrlist4 contain the expected number of words? */
if (attrlist4_count != xdr_stream_pos(argp->xdr) - starting_pos)
--
2.47.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH 0/6] nfsd: update the delstid patches for latest draft changes
2024-10-14 19:26 [PATCH 0/6] nfsd: update the delstid patches for latest draft changes Jeff Layton
` (5 preceding siblings ...)
2024-10-14 19:26 ` [PATCH 6/6] nfsd: handle delegated timestamps in SETATTR Jeff Layton
@ 2024-10-15 19:17 ` cel
2024-10-17 22:39 ` Olga Kornievskaia
7 siblings, 0 replies; 18+ messages in thread
From: cel @ 2024-10-15 19:17 UTC (permalink / raw)
To: Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Jonathan Corbet, Trond Myklebust, Anna Schumaker, Thomas Haynes,
Jeff Layton
Cc: Chuck Lever, linux-nfs, linux-kernel, linux-doc
From: Chuck Lever <chuck.lever@oracle.com>
On Mon, 14 Oct 2024 15:26:48 -0400, Jeff Layton wrote:
> This patchset is an update to the delstid patches that went into Chuck's
> nfsd-next branch recently. The original versions of the spec left out
> OPEN_DELEGATE_READ_ATTRS_DELEG and OPEN_DELEGATE_WRITE_ATTRS_DELEG. This
> set adds proper support for them.
>
> My suggestion is to drop these two patches from nfsd-next:
>
> [...]
Applied to nfsd-next for v6.13, thanks!
[1/6] nfsd: drop inode parameter from nfsd4_change_attribute()
commit: aacaab97e76307be16bf147b64dc45c0c9013a8a
[2/6] nfsd: switch to autogenerated definitions for open_delegation_type4
commit: c1be156cbb5960cf760409416fddbf300b2500c3
[3/6] nfsd: rename NFS4_SHARE_WANT_* constants to OPEN4_SHARE_ACCESS_WANT_*
commit: 466247e4e33b7e43589e5fa00bcd721a67463935
[4/6] nfsd: prepare delegation code for handing out *_ATTRS_DELEG delegations
commit: 6dd23bce4068b7ecb26390c81713ab6b617990a3
[5/6] nfsd: add support for delegated timestamps
commit: 3d342dc5ffe941fed1f6b32ecf59b9b5769f38be
[6/6] nfsd: handle delegated timestamps in SETATTR
commit: f6b1cfab609da048eba93210f47bf8ef43068119
--
Chuck Lever
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 0/6] nfsd: update the delstid patches for latest draft changes
2024-10-14 19:26 [PATCH 0/6] nfsd: update the delstid patches for latest draft changes Jeff Layton
` (6 preceding siblings ...)
2024-10-15 19:17 ` [PATCH 0/6] nfsd: update the delstid patches for latest draft changes cel
@ 2024-10-17 22:39 ` Olga Kornievskaia
2024-10-18 12:05 ` Jeff Layton
7 siblings, 1 reply; 18+ messages in thread
From: Olga Kornievskaia @ 2024-10-17 22:39 UTC (permalink / raw)
To: Jeff Layton
Cc: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Jonathan Corbet, Trond Myklebust, Anna Schumaker, Thomas Haynes,
linux-nfs, linux-kernel, linux-doc
Seeing strangeness in a network trace with this patch series where
SETATTR is sent with time_deleg_access and server is returning with
EINVAL. Test is open() with read delegation, triggering a cb_recall
via a local access. I can see that the client has changed from sending
just a delegreturn to sending a setattr+delegreturn. Is there no
server support and this is normal to return EINVAL.
On Mon, Oct 14, 2024 at 3:27 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> This patchset is an update to the delstid patches that went into Chuck's
> nfsd-next branch recently. The original versions of the spec left out
> OPEN_DELEGATE_READ_ATTRS_DELEG and OPEN_DELEGATE_WRITE_ATTRS_DELEG. This
> set adds proper support for them.
>
> My suggestion is to drop these two patches from nfsd-next:
>
> 544c67cc0f26 nfsd: handle delegated timestamps in SETATTR
> eee2c04ca5c1 nfsd: add support for delegated timestamps
>
> ...and then apply this set on top of the remaining pile. The resulting
> set is a bit larger than the original, as I took the liberty of adding
> some more symbols to the autogenerated part of the spec.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> Jeff Layton (6):
> nfsd: drop inode parameter from nfsd4_change_attribute()
> nfsd: switch to autogenerated definitions for open_delegation_type4
> nfsd: rename NFS4_SHARE_WANT_* constants to OPEN4_SHARE_ACCESS_WANT_*
> nfsd: prepare delegation code for handing out *_ATTRS_DELEG delegations
> nfsd: add support for delegated timestamps
> nfsd: handle delegated timestamps in SETATTR
>
> Documentation/sunrpc/xdr/nfs4_1.x | 22 ++++-
> fs/nfsd/nfs4callback.c | 42 ++++++++-
> fs/nfsd/nfs4proc.c | 26 ++++-
> fs/nfsd/nfs4state.c | 178 ++++++++++++++++++++++++++---------
> fs/nfsd/nfs4xdr.c | 57 ++++++++---
> fs/nfsd/nfs4xdr_gen.c | 19 +++-
> fs/nfsd/nfs4xdr_gen.h | 2 +-
> fs/nfsd/nfsd.h | 2 +
> fs/nfsd/nfsfh.c | 11 +--
> fs/nfsd/nfsfh.h | 3 +-
> fs/nfsd/state.h | 18 ++++
> fs/nfsd/xdr4cb.h | 10 +-
> include/linux/nfs4.h | 2 +-
> include/linux/sunrpc/xdrgen/nfs4_1.h | 35 ++++++-
> include/linux/time64.h | 5 +
> 15 files changed, 348 insertions(+), 84 deletions(-)
> ---
> base-commit: 9f8009c5be9367d01cd1627d6a379b4c642d8a28
> change-id: 20241014-delstid-bf05220ad941
>
> Best regards,
> --
> Jeff Layton <jlayton@kernel.org>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 0/6] nfsd: update the delstid patches for latest draft changes
2024-10-17 22:39 ` Olga Kornievskaia
@ 2024-10-18 12:05 ` Jeff Layton
0 siblings, 0 replies; 18+ messages in thread
From: Jeff Layton @ 2024-10-18 12:05 UTC (permalink / raw)
To: Olga Kornievskaia
Cc: Chuck Lever, Neil Brown, Olga Kornievskaia, Dai Ngo, Tom Talpey,
Jonathan Corbet, Trond Myklebust, Anna Schumaker, Thomas Haynes,
linux-nfs, linux-kernel, linux-doc
On Thu, 2024-10-17 at 18:39 -0400, Olga Kornievskaia wrote:
> Seeing strangeness in a network trace with this patch series where
> SETATTR is sent with time_deleg_access and server is returning with
> EINVAL. Test is open() with read delegation, triggering a cb_recall
> via a local access. I can see that the client has changed from sending
> just a delegreturn to sending a setattr+delegreturn. Is there no
> server support and this is normal to return EINVAL.
>
No, that's a server bug. I think it's this in nfsd4_setattr:
if (deleg_attrs || (setattr->sa_iattr.ia_valid & ATTR_SIZE)) {
status = nfs4_preprocess_stateid_op(rqstp, cstate,
&cstate->current_fh, &setattr->sa_stateid,
WR_STATE, NULL, &st);
if (status)
return status;
}
We're asking for a WR_STATE in the nfs4_preprocess_stateid_op, but
there isn't one. There is only a read delegation, so we get back
BAD_STATEID and that eventually runs into -EINVAL. I'll need to look
over this and figure out how to fix it properly.
Thanks for the bug report.
> On Mon, Oct 14, 2024 at 3:27 PM Jeff Layton <jlayton@kernel.org> wrote:
> >
> > This patchset is an update to the delstid patches that went into Chuck's
> > nfsd-next branch recently. The original versions of the spec left out
> > OPEN_DELEGATE_READ_ATTRS_DELEG and OPEN_DELEGATE_WRITE_ATTRS_DELEG. This
> > set adds proper support for them.
> >
> > My suggestion is to drop these two patches from nfsd-next:
> >
> > 544c67cc0f26 nfsd: handle delegated timestamps in SETATTR
> > eee2c04ca5c1 nfsd: add support for delegated timestamps
> >
> > ...and then apply this set on top of the remaining pile. The resulting
> > set is a bit larger than the original, as I took the liberty of adding
> > some more symbols to the autogenerated part of the spec.
> >
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > Jeff Layton (6):
> > nfsd: drop inode parameter from nfsd4_change_attribute()
> > nfsd: switch to autogenerated definitions for open_delegation_type4
> > nfsd: rename NFS4_SHARE_WANT_* constants to OPEN4_SHARE_ACCESS_WANT_*
> > nfsd: prepare delegation code for handing out *_ATTRS_DELEG delegations
> > nfsd: add support for delegated timestamps
> > nfsd: handle delegated timestamps in SETATTR
> >
> > Documentation/sunrpc/xdr/nfs4_1.x | 22 ++++-
> > fs/nfsd/nfs4callback.c | 42 ++++++++-
> > fs/nfsd/nfs4proc.c | 26 ++++-
> > fs/nfsd/nfs4state.c | 178 ++++++++++++++++++++++++++---------
> > fs/nfsd/nfs4xdr.c | 57 ++++++++---
> > fs/nfsd/nfs4xdr_gen.c | 19 +++-
> > fs/nfsd/nfs4xdr_gen.h | 2 +-
> > fs/nfsd/nfsd.h | 2 +
> > fs/nfsd/nfsfh.c | 11 +--
> > fs/nfsd/nfsfh.h | 3 +-
> > fs/nfsd/state.h | 18 ++++
> > fs/nfsd/xdr4cb.h | 10 +-
> > include/linux/nfs4.h | 2 +-
> > include/linux/sunrpc/xdrgen/nfs4_1.h | 35 ++++++-
> > include/linux/time64.h | 5 +
> > 15 files changed, 348 insertions(+), 84 deletions(-)
> > ---
> > base-commit: 9f8009c5be9367d01cd1627d6a379b4c642d8a28
> > change-id: 20241014-delstid-bf05220ad941
> >
> > Best regards,
> > --
> > Jeff Layton <jlayton@kernel.org>
> >
> >
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 18+ messages in thread