On Wed, 2024-07-24 at 14:47 -0700, Sagi Grimberg wrote: > > > > On 24/07/2024 22:29, Olga Kornievskaia wrote: > > On Wed, Jul 24, 2024 at 1:01 PM Sagi Grimberg <sagi@xxxxxxxxxxx> > > wrote: > > > Based on a patch fom Dai Ngo, allow NFSv4 client to use write > > > delegation > > > stateid for READ operation. Per RFC 8881 section 9.1.2. Use of > > > the > > > Stateid and Locking. > > > > > > In addition, for anonymous stateids, check for pending > > > delegations by > > > the filehandle and client_id, and if a conflict found, recall the > > > delegation > > > before allowing the read to take place. > > > > > > Suggested-by: Dai Ngo <dai.ngo@xxxxxxxxxx> > > > Signed-off-by: Sagi Grimberg <sagi@xxxxxxxxxxx> > > > --- > > > fs/nfsd/nfs4proc.c | 22 +++++++++++++++++++-- > > > fs/nfsd/nfs4state.c | 47 > > > +++++++++++++++++++++++++++++++++++++++++++++ > > > fs/nfsd/nfs4xdr.c | 9 +++++++++ > > > fs/nfsd/state.h | 2 ++ > > > fs/nfsd/xdr4.h | 2 ++ > > > 5 files changed, 80 insertions(+), 2 deletions(-) > > > > > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c > > > index 7b70309ad8fb..324984ec70c6 100644 > > > --- a/fs/nfsd/nfs4proc.c > > > +++ b/fs/nfsd/nfs4proc.c > > > @@ -979,8 +979,24 @@ nfsd4_read(struct svc_rqst *rqstp, struct > > > nfsd4_compound_state *cstate, > > > /* check stateid */ > > > status = nfs4_preprocess_stateid_op(rqstp, cstate, > > > &cstate->current_fh, > > > &read->rd_stateid, > > > RD_STATE, > > > - &read->rd_nf, NULL); > > > - > > > + &read->rd_nf, &read- > > > >rd_wd_stid); > > Now I can see that this patch wants to leverage the "returned > > stateid > > of the nfs4_preprocess_stateid_op() but the logic in the previous > > patch was in the way because it distinguished the copy_notify by > > the > > non-null passed in stateid. So I would suggest that in order to not > > break the copy_notify and help with this functionality something > > else > > is sent into nfs4_proprocess_staetid_op() to allow for the stateid > > to > > be passed and then distinguish between copy_notify and read. > > My thinking is that instead having the generic stateid pre-processing > be aware which proc may accept a special stateid and which may not, > we'd want to have the call-sites enforce that... > > But I am not the expert here, and will be happy to change based on > your preference... I tend to agree with Sagi here. Keeping this sort of operation-specific check in the operation-specific function makes more sense to me too. -- Jeff Layton <jlayton@xxxxxxxxxx>
