On 3/4/25 4:27 AM, Jeff Layton wrote:
On Mon, 2025-03-03 at 19:33 -0800, Dai Ngo wrote:
Allow READ using write delegation stateid granted on OPENs with
OPEN4_SHARE_ACCESS_WRITE only, to accommodate clients whose WRITE
implementation may unavoidably do (e.g., due to buffer cache
constraints).
When a write delegation is granted for OPEN with OPEN4_SHARE_ACCESS_WRITE,
a new pair of nfsd_file and struct file are allocated with read access
and added to nfs4_file's fi_fds[]. This is to allow the client to use
the delegation stateid to do reads.
No additional actions is needed when the delegation is returned. The
nfsd_file for read remains attached to the nfs4_file and is freed when
the open stateid is closed.
Suggested-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
Signed-off-by: Dai Ngo <dai.ngo@xxxxxxxxxx>
---
fs/nfsd/nfs4state.c | 41 ++++++++++++++++++++++++++++++++++-------
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b533225e57cf..d2c6c43b5d0c 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6126,6 +6126,30 @@ nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
return rc == 0;
}
+/*
+ * Add NFS4_SHARE_ACCESS_READ to the write delegation granted on OPEN
+ * with NFS4_SHARE_ACCESS_READ by allocating separate nfsd_file and
+ * struct file to be used for read with delegation stateid.
+ *
+ */
+static void
+nfs4_add_rdaccess_to_wrdeleg(struct svc_rqst *rqstp, struct svc_fh *fh,
+ struct nfs4_ol_stateid *stp)
Let's call this nfsd4_add_rdaccess_to_wrdeleg().
Okay.
+{
+ struct nfs4_file *fp;
+ struct nfsd_file *nf = NULL;
+
+ if (nfsd_file_acquire_opened(rqstp, fh, NFSD_MAY_READ, NULL, &nf))
+ return;
This function should return an error if nfsd_file_acquire_opened()
fails, and the caller should not give out the delegation in that case.
Okay.
+ fp = stp->st_stid.sc_file;
+ spin_lock(&fp->fi_lock);
+ __nfs4_file_get_access(fp, NFS4_SHARE_ACCESS_READ);
+ set_access(NFS4_SHARE_ACCESS_READ, stp);
+ fp = stp->st_stid.sc_file;
+ fp->fi_fds[O_RDONLY] = nf;
+ spin_unlock(&fp->fi_lock);
+}
+
/*
* The Linux NFS server does not offer write delegations to NFSv4.0
* clients in order to avoid conflicts between write delegations and
@@ -6151,8 +6175,9 @@ nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
* open or lock state.
*/
static void
-nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
- struct svc_fh *currentfh)
+nfs4_open_delegation(struct svc_rqst *rqstp, struct nfsd4_open *open,
+ struct nfs4_ol_stateid *stp, struct svc_fh *currentfh,
+ struct svc_fh *fh)
{
bool deleg_ts = open->op_deleg_want & OPEN4_SHARE_ACCESS_WANT_DELEG_TIMESTAMPS;
struct nfs4_openowner *oo = openowner(stp->st_stateowner);
@@ -6207,6 +6232,10 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
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);
+
+ if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) ==
+ NFS4_SHARE_ACCESS_WRITE)
+ nfs4_add_rdaccess_to_wrdeleg(rqstp, fh, stp);
nit: I'd also move the if statement above into
nfsd4_add_rdaccess_to_wrdeleg().
Okay.
} else {
open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG :
OPEN_DELEGATE_READ;
@@ -6353,7 +6382,8 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
* Attempt to hand out a delegation. No error return, because the
* OPEN succeeds even if we fail.
*/
- nfs4_open_delegation(open, stp, &resp->cstate.current_fh);
+ nfs4_open_delegation(rqstp, open, stp,
+ &resp->cstate.current_fh, current_fh);
/*
* If there is an existing open stateid, it must be updated and
@@ -7098,10 +7128,6 @@ nfs4_find_file(struct nfs4_stid *s, int flags)
switch (s->sc_type) {
case SC_TYPE_DELEG:
- spin_lock(&s->sc_file->fi_lock);
- ret = nfsd_file_get(s->sc_file->fi_deleg_file);
- spin_unlock(&s->sc_file->fi_lock);
- break;
case SC_TYPE_OPEN:
case SC_TYPE_LOCK:
if (flags & RD_STATE)
@@ -7277,6 +7303,7 @@ nfs4_preprocess_stateid_op(struct svc_rqst *rqstp,
status = find_cpntf_state(nn, stateid, &s);
if (status)
return status;
+
status = nfsd4_stid_check_stateid_generation(stateid, s,
nfsd4_has_session(cstate));
if (status)
I think this approach looks valid though.
Nice work!
Thanks for your review.
-Dai
