On Wed, Feb 14, 2024 at 10:10:50AM -0800, dai.ngo@xxxxxxxxxx wrote: > > On 2/14/24 6:50 AM, Chuck Lever wrote: > > On Tue, Feb 13, 2024 at 09:47:27AM -0800, Dai Ngo wrote: > > > If the GETATTR request on a file that has write delegation in effect > > > and the request attributes include the change info and size attribute > > > then the request is handled as below: > > > > > > Server sends CB_GETATTR to client to get the latest change info and file > > > size. If these values are the same as the server's cached values then > > > the GETATTR proceeds as normal. > > > > > > If either the change info or file size is different from the server's > > > cached values, or the file was already marked as modified, then: > > > > > > . update time_modify and time_metadata into file's metadata > > > with current time > > > > > > . encode GETATTR as normal except the file size is encoded with > > > the value returned from CB_GETATTR > > > > > > . mark the file as modified > > > > > > The CB_GETATTR is given 30ms to complte. If the CB_GETATTR fails for > > > any reasons, the delegation is recalled and NFS4ERR_DELAY is returned > > > for the GETATTR from the second client. > > > > > > Signed-off-by: Dai Ngo <dai.ngo@xxxxxxxxxx> > > > --- > > > fs/nfsd/nfs4state.c | 119 ++++++++++++++++++++++++++++++++++++++++---- > > > fs/nfsd/nfs4xdr.c | 10 +++- > > > fs/nfsd/nfsd.h | 1 + > > > fs/nfsd/state.h | 10 +++- > > > 4 files changed, 127 insertions(+), 13 deletions(-) > > > > > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > > > index d9260e77ef2d..87987515e03d 100644 > > > --- a/fs/nfsd/nfs4state.c > > > +++ b/fs/nfsd/nfs4state.c > > > @@ -127,6 +127,7 @@ static void free_session(struct nfsd4_session *); > > > static const struct nfsd4_callback_ops nfsd4_cb_recall_ops; > > > static const struct nfsd4_callback_ops nfsd4_cb_notify_lock_ops; > > > +static const struct nfsd4_callback_ops nfsd4_cb_getattr_ops; > > > static struct workqueue_struct *laundry_wq; > > > @@ -1189,6 +1190,10 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_file *fp, > > > dp->dl_recalled = false; > > > nfsd4_init_cb(&dp->dl_recall, dp->dl_stid.sc_client, > > > &nfsd4_cb_recall_ops, NFSPROC4_CLNT_CB_RECALL); > > > + nfsd4_init_cb(&dp->dl_cb_fattr.ncf_getattr, dp->dl_stid.sc_client, > > > + &nfsd4_cb_getattr_ops, NFSPROC4_CLNT_CB_GETATTR); > > > + dp->dl_cb_fattr.ncf_file_modified = false; > > > + dp->dl_cb_fattr.ncf_cb_bmap[0] = FATTR4_WORD0_CHANGE | FATTR4_WORD0_SIZE; > > > get_nfs4_file(fp); > > > dp->dl_stid.sc_file = fp; > > > return dp; > > > @@ -3044,11 +3049,59 @@ nfsd4_cb_recall_any_release(struct nfsd4_callback *cb) > > > spin_unlock(&nn->client_lock); > > > } > > > +static int > > > +nfsd4_cb_getattr_done(struct nfsd4_callback *cb, struct rpc_task *task) > > > +{ > > > + struct nfs4_cb_fattr *ncf = > > > + container_of(cb, struct nfs4_cb_fattr, ncf_getattr); > > > + > > > + ncf->ncf_cb_status = task->tk_status; > > > + switch (task->tk_status) { > > > + case -NFS4ERR_DELAY: > > > + rpc_delay(task, 2 * HZ); > > > + return 0; > > > + default: > > > + return 1; > > > + } > > > +} > > > + > > > +static void > > > +nfsd4_cb_getattr_release(struct nfsd4_callback *cb) > > > +{ > > > + struct nfs4_cb_fattr *ncf = > > > + container_of(cb, struct nfs4_cb_fattr, ncf_getattr); > > > + struct nfs4_delegation *dp = > > > + container_of(ncf, struct nfs4_delegation, dl_cb_fattr); > > > + > > > + nfs4_put_stid(&dp->dl_stid); > > > + clear_bit(CB_GETATTR_BUSY, &ncf->ncf_cb_flags); > > > + wake_up_bit(&ncf->ncf_cb_flags, CB_GETATTR_BUSY); > > > +} > > > + > > What happens if the client responds after the GETATTR timer elapses? > > Can nfsd4_cb_getattr_release over-write memory that is now being > > used for something else? > > The refcount added in nfs4_cb_getattr keeps the delegation state valid > until it's released here by nfs4_put_stid. If the client never replies, does that pin the stateid? -- Chuck Lever