| Message ID | 20240905-delstid-v4-1-d3e5fd34d107@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | nfsd: implement the "delstid" draft | expand |
On Thu, Sep 05, 2024 at 08:41:45AM -0400, Jeff Layton wrote: > At this point in compound processing, currentfh refers to the parent of > the file, not the file itself. Get the correct dentry from the delegation > stateid instead. > > Signed-off-by: Jeff Layton <jlayton@kernel.org> Subject says "fixes ..." so IMO a Fixes: tag is warranted. Suggestions welcome. > --- > fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++-------- > 1 file changed, 23 insertions(+), 8 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index df69dc6af467..db90677fc016 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -5914,6 +5914,26 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status) > } > } > > +static bool > +nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh, > + struct kstat *stat) > +{ > + struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file); > + struct path path; > + > + if (!nf) > + return false; > + > + path.mnt = currentfh->fh_export->ex_path.mnt; > + path.dentry = file_dentry(nf->nf_file); > + > + if (vfs_getattr(&path, stat, > + (STATX_INO | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE), > + AT_STATX_SYNC_AS_STAT)) > + return false; > + return true; > +} > + > /* > * The Linux NFS server does not offer write delegations to NFSv4.0 > * clients in order to avoid conflicts between write delegations and > @@ -5949,7 +5969,6 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, > int cb_up; > int status = 0; > struct kstat stat; > - struct path path; > > cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client); > open->op_recall = false; > @@ -5985,20 +6004,16 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, > memcpy(&open->op_delegate_stateid, &dp->dl_stid.sc_stateid, sizeof(dp->dl_stid.sc_stateid)); > > if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) { > - open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE; > - trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid); > - path.mnt = currentfh->fh_export->ex_path.mnt; > - path.dentry = currentfh->fh_dentry; > - if (vfs_getattr(&path, &stat, > - (STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE), > - AT_STATX_SYNC_AS_STAT)) { > + if (!nfs4_delegation_stat(dp, currentfh, &stat)) { > nfs4_put_stid(&dp->dl_stid); > destroy_delegation(dp); > goto out_no_deleg; > } > + 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)); > + trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid); > } else { > open->op_delegate_type = NFS4_OPEN_DELEGATE_READ; > trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid); > > -- > 2.46.0 >
On Thu, 2024-09-05 at 11:46 -0400, Chuck Lever wrote: > On Thu, Sep 05, 2024 at 08:41:45AM -0400, Jeff Layton wrote: > > At this point in compound processing, currentfh refers to the parent of > > the file, not the file itself. Get the correct dentry from the delegation > > stateid instead. > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > Subject says "fixes ..." so IMO a Fixes: tag is warranted. > Suggestions welcome. > > Yes, sorry. I think we want: Fixes: c5967721e106 ("NFSD: handle GETATTR conflict with write delegation") > > --- > > fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++-------- > > 1 file changed, 23 insertions(+), 8 deletions(-) > > > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > > index df69dc6af467..db90677fc016 100644 > > --- a/fs/nfsd/nfs4state.c > > +++ b/fs/nfsd/nfs4state.c > > @@ -5914,6 +5914,26 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status) > > } > > } > > > > +static bool > > +nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh, > > + struct kstat *stat) > > +{ > > + struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file); > > + struct path path; > > + > > + if (!nf) > > + return false; > > + > > + path.mnt = currentfh->fh_export->ex_path.mnt; > > + path.dentry = file_dentry(nf->nf_file); > > + > > + if (vfs_getattr(&path, stat, > > + (STATX_INO | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE), > > + AT_STATX_SYNC_AS_STAT)) > > + return false; > > + return true; > > +} > > + > > /* > > * The Linux NFS server does not offer write delegations to NFSv4.0 > > * clients in order to avoid conflicts between write delegations and > > @@ -5949,7 +5969,6 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, > > int cb_up; > > int status = 0; > > struct kstat stat; > > - struct path path; > > > > cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client); > > open->op_recall = false; > > @@ -5985,20 +6004,16 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, > > memcpy(&open->op_delegate_stateid, &dp->dl_stid.sc_stateid, sizeof(dp->dl_stid.sc_stateid)); > > > > if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) { > > - open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE; > > - trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid); > > - path.mnt = currentfh->fh_export->ex_path.mnt; > > - path.dentry = currentfh->fh_dentry; > > - if (vfs_getattr(&path, &stat, > > - (STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE), > > - AT_STATX_SYNC_AS_STAT)) { > > + if (!nfs4_delegation_stat(dp, currentfh, &stat)) { > > nfs4_put_stid(&dp->dl_stid); > > destroy_delegation(dp); > > goto out_no_deleg; > > } > > + 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)); > > + trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid); > > } else { > > open->op_delegate_type = NFS4_OPEN_DELEGATE_READ; > > trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid); > > > > -- > > 2.46.0 > > >
On Thu, 2024-09-05 at 08:41 -0400, Jeff Layton wrote: > At this point in compound processing, currentfh refers to the parent of > the file, not the file itself. Get the correct dentry from the delegation > stateid instead. > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++-------- > 1 file changed, 23 insertions(+), 8 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index df69dc6af467..db90677fc016 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -5914,6 +5914,26 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status) > } > } > > +static bool > +nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh, > + struct kstat *stat) > +{ > + struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file); > + struct path path; > + > + if (!nf) > + return false; > + > + path.mnt = currentfh->fh_export->ex_path.mnt; > + path.dentry = file_dentry(nf->nf_file); > + > + if (vfs_getattr(&path, stat, > + (STATX_INO | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE), Minor oversight here. I added STATX_INO when I was debugging, but we don't need it here. We should probably drop that flag (though it's mostly harmless). Chuck, would you be ok with fixing that up? > + AT_STATX_SYNC_AS_STAT)) > + return false; > + return true; > +} > + > /* > * The Linux NFS server does not offer write delegations to NFSv4.0 > * clients in order to avoid conflicts between write delegations and > @@ -5949,7 +5969,6 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, > int cb_up; > int status = 0; > struct kstat stat; > - struct path path; > > cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client); > open->op_recall = false; > @@ -5985,20 +6004,16 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, > memcpy(&open->op_delegate_stateid, &dp->dl_stid.sc_stateid, sizeof(dp->dl_stid.sc_stateid)); > > if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) { > - open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE; > - trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid); > - path.mnt = currentfh->fh_export->ex_path.mnt; > - path.dentry = currentfh->fh_dentry; > - if (vfs_getattr(&path, &stat, > - (STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE), > - AT_STATX_SYNC_AS_STAT)) { > + if (!nfs4_delegation_stat(dp, currentfh, &stat)) { > nfs4_put_stid(&dp->dl_stid); > destroy_delegation(dp); > goto out_no_deleg; > } > + 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)); > + trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid); > } else { > open->op_delegate_type = NFS4_OPEN_DELEGATE_READ; > trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid); > Thanks!
On Fri, Sep 06, 2024 at 10:08:29AM -0400, Jeff Layton wrote: > On Thu, 2024-09-05 at 08:41 -0400, Jeff Layton wrote: > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > > index df69dc6af467..db90677fc016 100644 > > --- a/fs/nfsd/nfs4state.c > > +++ b/fs/nfsd/nfs4state.c > > @@ -5914,6 +5914,26 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status) > > } > > } > > > > +static bool > > +nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh, > > + struct kstat *stat) > > +{ > > + struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file); > > + struct path path; > > + > > + if (!nf) > > + return false; > > + > > + path.mnt = currentfh->fh_export->ex_path.mnt; > > + path.dentry = file_dentry(nf->nf_file); > > + > > + if (vfs_getattr(&path, stat, > > + (STATX_INO | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE), > > Minor oversight here. > > I added STATX_INO when I was debugging, but we don't need it here. We > should probably drop that flag (though it's mostly harmless). Chuck, > would you be ok with fixing that up? Fixed and squashed into nfsd-next.
On Thu, Sep 05, 2024 at 08:41:45AM -0400, Jeff Layton wrote: > At this point in compound processing, currentfh refers to the parent of > the file, not the file itself. Get the correct dentry from the delegation > stateid instead. > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++-------- > 1 file changed, 23 insertions(+), 8 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index df69dc6af467..db90677fc016 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -5914,6 +5914,26 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status) > } > } > > +static bool > +nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh, > + struct kstat *stat) > +{ > + struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file); The xfstests workflow on NFSv4.2 exhausts the capacity of both the main and scratch devices (backed by xfs) about half-way through each test run. Deleting all visible files on both devices frees only a little bit of space. The test exports can be unshared but not unmounted (EBUSY). Looks like unlinked but still open files, maybe. Bisected to this here patch. Should there be a matching nfsd_file_put() book-end for the new find_rw_file() call site? > + struct path path; > + > + if (!nf) > + return false; > + > + path.mnt = currentfh->fh_export->ex_path.mnt; > + path.dentry = file_dentry(nf->nf_file); > + > + if (vfs_getattr(&path, stat, > + (STATX_INO | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE), > + AT_STATX_SYNC_AS_STAT)) > + return false; > + return true; > +} > + > /* > * The Linux NFS server does not offer write delegations to NFSv4.0 > * clients in order to avoid conflicts between write delegations and > @@ -5949,7 +5969,6 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, > int cb_up; > int status = 0; > struct kstat stat; > - struct path path; > > cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client); > open->op_recall = false; > @@ -5985,20 +6004,16 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, > memcpy(&open->op_delegate_stateid, &dp->dl_stid.sc_stateid, sizeof(dp->dl_stid.sc_stateid)); > > if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) { > - open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE; > - trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid); > - path.mnt = currentfh->fh_export->ex_path.mnt; > - path.dentry = currentfh->fh_dentry; > - if (vfs_getattr(&path, &stat, > - (STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE), > - AT_STATX_SYNC_AS_STAT)) { > + if (!nfs4_delegation_stat(dp, currentfh, &stat)) { > nfs4_put_stid(&dp->dl_stid); > destroy_delegation(dp); > goto out_no_deleg; > } > + 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)); > + trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid); > } else { > open->op_delegate_type = NFS4_OPEN_DELEGATE_READ; > trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid); > > -- > 2.46.0 >
On Sun, 2024-09-08 at 14:00 -0400, Chuck Lever wrote: > On Thu, Sep 05, 2024 at 08:41:45AM -0400, Jeff Layton wrote: > > At this point in compound processing, currentfh refers to the parent of > > the file, not the file itself. Get the correct dentry from the delegation > > stateid instead. > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > --- > > fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++-------- > > 1 file changed, 23 insertions(+), 8 deletions(-) > > > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > > index df69dc6af467..db90677fc016 100644 > > --- a/fs/nfsd/nfs4state.c > > +++ b/fs/nfsd/nfs4state.c > > @@ -5914,6 +5914,26 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status) > > } > > } > > > > +static bool > > +nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh, > > + struct kstat *stat) > > +{ > > + struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file); > > The xfstests workflow on NFSv4.2 exhausts the capacity of both the > main and scratch devices (backed by xfs) about half-way through > each test run. > > Deleting all visible files on both devices frees only a little bit > of space. The test exports can be unshared but not unmounted > (EBUSY). Looks like unlinked but still open files, maybe. > > Bisected to this here patch. > > Should there be a matching nfsd_file_put() book-end for the new > find_rw_file() call site? > Yes. Braino on my end. I was thinking that find_rw_file didn't take a reference, but it does and we do need to put it. Would you like me to respin, or do you just want to add it in the appropriate spot? > > > + struct path path; > > + > > + if (!nf) > > + return false; > > + > > + path.mnt = currentfh->fh_export->ex_path.mnt; > > + path.dentry = file_dentry(nf->nf_file); > > + > > + if (vfs_getattr(&path, stat, > > + (STATX_INO | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE), > > + AT_STATX_SYNC_AS_STAT)) > > + return false; > > + return true; > > +} > > + > > /* > > * The Linux NFS server does not offer write delegations to NFSv4.0 > > * clients in order to avoid conflicts between write delegations and > > @@ -5949,7 +5969,6 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, > > int cb_up; > > int status = 0; > > struct kstat stat; > > - struct path path; > > > > cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client); > > open->op_recall = false; > > @@ -5985,20 +6004,16 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, > > memcpy(&open->op_delegate_stateid, &dp->dl_stid.sc_stateid, sizeof(dp->dl_stid.sc_stateid)); > > > > if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) { > > - open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE; > > - trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid); > > - path.mnt = currentfh->fh_export->ex_path.mnt; > > - path.dentry = currentfh->fh_dentry; > > - if (vfs_getattr(&path, &stat, > > - (STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE), > > - AT_STATX_SYNC_AS_STAT)) { > > + if (!nfs4_delegation_stat(dp, currentfh, &stat)) { > > nfs4_put_stid(&dp->dl_stid); > > destroy_delegation(dp); > > goto out_no_deleg; > > } > > + 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)); > > + trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid); > > } else { > > open->op_delegate_type = NFS4_OPEN_DELEGATE_READ; > > trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid); > > > > -- > > 2.46.0 > > >
On Sun, 8 Sept 2024 at 22:40, Jeff Layton <jlayton@kernel.org> wrote: > > On Sun, 2024-09-08 at 14:00 -0400, Chuck Lever wrote: > > On Thu, Sep 05, 2024 at 08:41:45AM -0400, Jeff Layton wrote: > > > At this point in compound processing, currentfh refers to the parent of > > > the file, not the file itself. Get the correct dentry from the delegation > > > stateid instead. > > > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > > --- > > > fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++-------- > > > 1 file changed, 23 insertions(+), 8 deletions(-) > > > > > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > > > index df69dc6af467..db90677fc016 100644 > > > --- a/fs/nfsd/nfs4state.c > > > +++ b/fs/nfsd/nfs4state.c > > > @@ -5914,6 +5914,26 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status) > > > } > > > } > > > > > > +static bool > > > +nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh, > > > + struct kstat *stat) > > > +{ > > > + struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file); > > > > The xfstests workflow on NFSv4.2 exhausts the capacity of both the > > main and scratch devices (backed by xfs) about half-way through > > each test run. > > > > Deleting all visible files on both devices frees only a little bit > > of space. The test exports can be unshared but not unmounted > > (EBUSY). Looks like unlinked but still open files, maybe. > > > > Bisected to this here patch. > > > > Should there be a matching nfsd_file_put() book-end for the new > > find_rw_file() call site? > > > > Yes. Braino on my end. I was thinking that find_rw_file didn't take a > reference, but it does and we do need to put it. Would you like me to > respin, or do you just want to add it in the appropriate spot? Did the respin ever happen? Ced
On Thu, 5 Sept 2024 at 14:42, Jeff Layton <jlayton@kernel.org> wrote: > > At this point in compound processing, currentfh refers to the parent of > the file, not the file itself. Get the correct dentry from the delegation > stateid instead. > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++-------- > 1 file changed, 23 insertions(+), 8 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index df69dc6af467..db90677fc016 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c Is this commit going to be backported to Linux 6.6 LTS? Ced
On 12/27/24 2:53 AM, Cedric Blancher wrote: > On Thu, 5 Sept 2024 at 14:42, Jeff Layton <jlayton@kernel.org> wrote: >> >> At this point in compound processing, currentfh refers to the parent of >> the file, not the file itself. Get the correct dentry from the delegation >> stateid instead. >> >> Signed-off-by: Jeff Layton <jlayton@kernel.org> >> --- >> fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++-------- >> 1 file changed, 23 insertions(+), 8 deletions(-) >> >> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >> index df69dc6af467..db90677fc016 100644 >> --- a/fs/nfsd/nfs4state.c >> +++ b/fs/nfsd/nfs4state.c > > Is this commit going to be backported to Linux 6.6 LTS? The quoted patch is https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/commit/fs/nfsd/nfs4state.c?id=bf92e5008b17f935a6de8b708551e02c2294121c in the master branch of Linus' tree. This commit has the following tag: Fixes: c5967721e106 ("NFSD: handle GETATTR conflict with write delegation") Commit c5967721e106 was merged in 6.9. Thus bf92e5008b17 won't be backported to kernels earlier than 6.9 unless they also have commit c5967721e106. It looks like 6.6 LTS does not have that commit.
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index df69dc6af467..db90677fc016 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -5914,6 +5914,26 @@ static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status) } } +static bool +nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh, + struct kstat *stat) +{ + struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file); + struct path path; + + if (!nf) + return false; + + path.mnt = currentfh->fh_export->ex_path.mnt; + path.dentry = file_dentry(nf->nf_file); + + if (vfs_getattr(&path, stat, + (STATX_INO | STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE), + AT_STATX_SYNC_AS_STAT)) + return false; + return true; +} + /* * The Linux NFS server does not offer write delegations to NFSv4.0 * clients in order to avoid conflicts between write delegations and @@ -5949,7 +5969,6 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, int cb_up; int status = 0; struct kstat stat; - struct path path; cb_up = nfsd4_cb_channel_good(oo->oo_owner.so_client); open->op_recall = false; @@ -5985,20 +6004,16 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, memcpy(&open->op_delegate_stateid, &dp->dl_stid.sc_stateid, sizeof(dp->dl_stid.sc_stateid)); if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) { - open->op_delegate_type = NFS4_OPEN_DELEGATE_WRITE; - trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid); - path.mnt = currentfh->fh_export->ex_path.mnt; - path.dentry = currentfh->fh_dentry; - if (vfs_getattr(&path, &stat, - (STATX_SIZE | STATX_CTIME | STATX_CHANGE_COOKIE), - AT_STATX_SYNC_AS_STAT)) { + if (!nfs4_delegation_stat(dp, currentfh, &stat)) { nfs4_put_stid(&dp->dl_stid); destroy_delegation(dp); goto out_no_deleg; } + 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)); + trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid); } else { open->op_delegate_type = NFS4_OPEN_DELEGATE_READ; trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
At this point in compound processing, currentfh refers to the parent of the file, not the file itself. Get the correct dentry from the delegation stateid instead. Signed-off-by: Jeff Layton <jlayton@kernel.org> --- fs/nfsd/nfs4state.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-)