Re: [PATCH 2/2] NFSD: handle GETATTR conflict with write delegation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2/14/24 10:14 AM, Chuck Lever wrote:
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?

Won't RPC timeout on waiting for reply?
This is the same behavior as when recalling a delegation,
nfsd_break_deleg_cb and nfsd4_cb_recall_release.

-Dai







[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux