[PATCH 11/14] cifs: Clamp length according to credits and rsize

From: David Howells
Date: Wed Apr 06 2022 - 19:06:00 EST


A read request can get expanded beyond the capacity of the available
credits and rsize, so use the ->clamp_length() method to cut the request up
into pieces rather than trying to do it in ->issue_read(), at which point
the subrequest size is already determined.

Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
cc: Steve French <sfrench@xxxxxxxxx>
cc: Shyam Prasad N <nspmangalore@xxxxxxxxx>
cc: Rohith Surabattula <rohiths.msft@xxxxxxxxx>
cc: linux-cifs@xxxxxxxxxxxxxxx
---

fs/cifs/cifsglob.h | 2 +
fs/cifs/file.c | 71 ++++++++++++++++++++++++++++++++++++----------------
fs/cifs/smb2pdu.c | 5 ++--
3 files changed, 54 insertions(+), 24 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index e1e77225d634..2b1930a918b0 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1328,8 +1328,10 @@ struct cifs_io_subrequest {
__u64 offset;
ssize_t got_bytes;
unsigned int bytes;
+ unsigned int xid;
pid_t pid;
int result;
+ bool have_credits;
struct kvec iov[2];
struct TCP_Server_Info *server;
#ifdef CONFIG_CIFS_SMB_DIRECT
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 45510bd1f702..12663d9d1e51 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -3437,6 +3437,47 @@ int cifs_file_mmap(struct file *file, struct vm_area_struct *vma)
return rc;
}

+/*
+ * Split the read up according to how many credits we can get for each piece.
+ * It's okay to sleep here if we need to wait for more credit to become
+ * available.
+ *
+ * We also choose the server and allocate an operation ID to be cleaned up
+ * later.
+ */
+static bool cifs_clamp_length(struct netfs_io_subrequest *subreq)
+{
+ struct netfs_io_request *rreq = subreq->rreq;
+ struct TCP_Server_Info *server;
+ struct cifs_io_subrequest *rdata = container_of(subreq, struct cifs_io_subrequest, subreq);
+ struct cifs_io_request *req = container_of(subreq->rreq, struct cifs_io_request, rreq);
+ struct cifs_sb_info *cifs_sb = CIFS_SB(rreq->inode->i_sb);
+ unsigned int rsize = 0;
+ int rc;
+
+ rdata->xid = get_xid();
+
+ server = cifs_pick_channel(tlink_tcon(req->cfile->tlink)->ses);
+ rdata->server = server;
+
+ if (cifs_sb->ctx->rsize == 0)
+ cifs_sb->ctx->rsize =
+ server->ops->negotiate_rsize(tlink_tcon(req->cfile->tlink),
+ cifs_sb->ctx);
+
+
+ rc = server->ops->wait_mtu_credits(server, cifs_sb->ctx->rsize, &rsize,
+ &rdata->credits);
+ if (rc) {
+ subreq->error = rc;
+ return false;
+ }
+
+ rdata->have_credits = true;
+ subreq->len = min_t(size_t, subreq->len, rsize);
+ return true;
+}
+
/*
* Issue a read operation on behalf of the netfs helper functions. We're asked
* to make a read of a certain size at a point in the file. We are permitted
@@ -3446,24 +3487,17 @@ int cifs_file_mmap(struct file *file, struct vm_area_struct *vma)
static void cifs_req_issue_read(struct netfs_io_subrequest *subreq)
{
struct netfs_io_request *rreq = subreq->rreq;
- struct TCP_Server_Info *server;
struct cifs_io_subrequest *rdata = container_of(subreq, struct cifs_io_subrequest, subreq);
struct cifs_io_request *req = container_of(subreq->rreq, struct cifs_io_request, rreq);
struct cifs_sb_info *cifs_sb = CIFS_SB(rreq->inode->i_sb);
- unsigned int xid;
pid_t pid;
int rc = 0;
- unsigned int rsize;
-
- xid = get_xid();

if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_RWPIDFORWARD)
pid = req->cfile->pid;
else
pid = current->tgid; // Ummm... This may be a workqueue

- server = cifs_pick_channel(tlink_tcon(req->cfile->tlink)->ses);
-
cifs_dbg(FYI, "%s: op=%08x[%x] mapping=%p len=%zu/%zu\n",
__func__, rreq->debug_id, subreq->debug_index, rreq->mapping,
subreq->transferred, subreq->len);
@@ -3476,32 +3510,20 @@ static void cifs_req_issue_read(struct netfs_io_subrequest *subreq)
goto out;
}

- if (cifs_sb->ctx->rsize == 0)
- cifs_sb->ctx->rsize =
- server->ops->negotiate_rsize(tlink_tcon(req->cfile->tlink),
- cifs_sb->ctx);
-
- rc = server->ops->wait_mtu_credits(server, cifs_sb->ctx->rsize, &rsize,
- &rdata->credits);
- if (rc)
- goto out;
-
__set_bit(NETFS_SREQ_CLEAR_TAIL, &subreq->flags);
- rdata->server = server;
rdata->offset = subreq->start + subreq->transferred;
rdata->bytes = subreq->len - subreq->transferred;
rdata->pid = pid;

- rc = adjust_credits(server, &rdata->credits, rdata->bytes);
+ rc = adjust_credits(rdata->server, &rdata->credits, rdata->bytes);
if (!rc) {
if (rdata->req->cfile->invalidHandle)
rc = -EAGAIN;
else
- rc = server->ops->async_readv(rdata);
+ rc = rdata->server->ops->async_readv(rdata);
}

out:
- free_xid(xid);
if (rc)
netfs_subreq_terminated(subreq, rc, false);
}
@@ -3580,6 +3602,7 @@ static void cifs_free_subrequest(struct netfs_io_subrequest *subreq)
{
struct cifs_io_subrequest *rdata =
container_of(subreq, struct cifs_io_subrequest, subreq);
+ int rc;

if (rdata->subreq.source == NETFS_DOWNLOAD_FROM_SERVER) {
#ifdef CONFIG_CIFS_SMB_DIRECT
@@ -3589,7 +3612,10 @@ static void cifs_free_subrequest(struct netfs_io_subrequest *subreq)
}
#endif

- add_credits_and_wake_if(rdata->server, &rdata->credits, 0);
+ if (rdata->have_credits)
+ add_credits_and_wake_if(rdata->server, &rdata->credits, 0);
+ rc = subreq->error;
+ free_xid(rdata->xid);
}
}

@@ -3601,6 +3627,7 @@ const struct netfs_request_ops cifs_req_ops = {
.free_subrequest = cifs_free_subrequest,
.begin_cache_operation = cifs_begin_cache_operation,
.expand_readahead = cifs_expand_readahead,
+ .clamp_length = cifs_clamp_length,
.issue_read = cifs_req_issue_read,
.done = cifs_rreq_done,
};
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 6a8aaa003e54..952f242bee55 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -4174,7 +4174,7 @@ smb2_readv_callback(struct mid_q_entry *mid)
#endif
if (rdata->result && rdata->result != -ENODATA) {
cifs_stats_fail_inc(tcon, SMB2_READ_HE);
- trace_smb3_read_err(0 /* xid */,
+ trace_smb3_read_err(rdata->xid,
rdata->req->cfile->fid.persistent_fid,
tcon->tid, tcon->ses->Suid, rdata->offset,
rdata->bytes, rdata->result);
@@ -4193,6 +4193,7 @@ smb2_readv_callback(struct mid_q_entry *mid)
}
if (rdata->result == 0 || rdata->result == -EAGAIN)
iov_iter_advance(&rdata->subreq.iter, rdata->got_bytes);
+ rdata->have_credits = false;
netfs_subreq_terminated(&rdata->subreq,
(rdata->result == 0 || rdata->result == -EAGAIN) ?
rdata->got_bytes : rdata->result, true);
@@ -4259,7 +4260,7 @@ smb2_async_readv(struct cifs_io_subrequest *rdata)
&rdata->credits);
if (rc) {
cifs_stats_fail_inc(io_parms.tcon, SMB2_READ_HE);
- trace_smb3_read_err(0 /* xid */, io_parms.persistent_fid,
+ trace_smb3_read_err(rdata->xid, io_parms.persistent_fid,
io_parms.tcon->tid,
io_parms.tcon->ses->Suid,
io_parms.offset, io_parms.length, rc);