Re: [PATCH 11/17] cifs: Move the 'pid' from the subreq to the req

Steve French
2024-06-20T20:30:09Z

[-- Attachment #1: Type: text/plain, Size: 6336 bytes --]

minor update to the patch. Fix function ‘cifs_req_issue_read’ to
remove the warning:
/home/smfrench/cifs-2.6/fs/smb/client/file.c:180:30: warning: unused
variable ‘cifs_sb
See attached.

Merged this and two more from the series tentatively into cifs-2.6.git
for-next pending more testing/review

969b3010cbfc cifs: Only pick a channel once per read request
ce5291e56081 cifs: Defer read completion

On Thu, Jun 20, 2024 at 12:33 PM David Howells <dhowells@redhat.com> wrote:
>
> Move the reference pid from the cifs_io_subrequest struct to the
> cifs_io_request struct as it's the same for all subreqs of a particular
> request.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Steve French <sfrench@samba.org>
> cc: Paulo Alcantara <pc@manguebit.com>
> cc: Jeff Layton <jlayton@kernel.org>
> cc: linux-cifs@vger.kernel.org
> cc: netfs@lists.linux.dev
> cc: linux-fsdevel@vger.kernel.org
> ---
> fs/smb/client/cifsglob.h | 2 +-
> fs/smb/client/cifssmb.c | 8 ++++----
> fs/smb/client/file.c | 10 +++-------
> fs/smb/client/smb2pdu.c | 4 ++--
> 4 files changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> index b48d3f5e8889..bbcc552c07be 100644
> --- a/fs/smb/client/cifsglob.h
> +++ b/fs/smb/client/cifsglob.h
> @@ -1495,6 +1495,7 @@ struct cifs_io_request {
> struct netfs_io_request rreq;
> struct cifsFileInfo *cfile;
> struct TCP_Server_Info *server;
> + pid_t pid;
> };
>
> /* asynchronous read support */
> @@ -1505,7 +1506,6 @@ struct cifs_io_subrequest {
> struct cifs_io_request *req;
> };
> ssize_t got_bytes;
> - pid_t pid;
> unsigned int xid;
> int result;
> bool have_xid;
> diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
> index 25e9ab947c17..595c4b673707 100644
> --- a/fs/smb/client/cifssmb.c
> +++ b/fs/smb/client/cifssmb.c
> @@ -1345,8 +1345,8 @@ cifs_async_readv(struct cifs_io_subrequest *rdata)
> if (rc)
> return rc;
>
> - smb->hdr.Pid = cpu_to_le16((__u16)rdata->pid);
> - smb->hdr.PidHigh = cpu_to_le16((__u16)(rdata->pid >> 16));
> + smb->hdr.Pid = cpu_to_le16((__u16)rdata->req->pid);
> + smb->hdr.PidHigh = cpu_to_le16((__u16)(rdata->req->pid >> 16));
>
> smb->AndXCommand = 0xFF; /* none */
> smb->Fid = rdata->req->cfile->fid.netfid;
> @@ -1689,8 +1689,8 @@ cifs_async_writev(struct cifs_io_subrequest *wdata)
> if (rc)
> goto async_writev_out;
>
> - smb->hdr.Pid = cpu_to_le16((__u16)wdata->pid);
> - smb->hdr.PidHigh = cpu_to_le16((__u16)(wdata->pid >> 16));
> + smb->hdr.Pid = cpu_to_le16((__u16)wdata->req->pid);
> + smb->hdr.PidHigh = cpu_to_le16((__u16)(wdata->req->pid >> 16));
>
> smb->AndXCommand = 0xFF; /* none */
> smb->Fid = wdata->req->cfile->fid.netfid;
> diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
> index 16fa1ac1ed2d..45c860f0e7fd 100644
> --- a/fs/smb/client/file.c
> +++ b/fs/smb/client/file.c
> @@ -178,14 +178,8 @@ static void cifs_req_issue_read(struct netfs_io_subrequest *subreq)
> 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);
> - pid_t pid;
> int rc = 0;
>
> - if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_RWPIDFORWARD)
> - pid = req->cfile->pid;
> - else
> - pid = current->tgid; // Ummm... This may be a workqueue
> -
> 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);
> @@ -199,7 +193,6 @@ static void cifs_req_issue_read(struct netfs_io_subrequest *subreq)
> }
>
> __set_bit(NETFS_SREQ_CLEAR_TAIL, &subreq->flags);
> - rdata->pid = pid;
>
> rc = rdata->server->ops->async_readv(rdata);
> out:
> @@ -236,12 +229,15 @@ static int cifs_init_request(struct netfs_io_request *rreq, struct file *file)
>
> rreq->rsize = cifs_sb->ctx->rsize;
> rreq->wsize = cifs_sb->ctx->wsize;
> + req->pid = current->tgid; // Ummm... This may be a workqueue
>
> if (file) {
> open_file = file->private_data;
> rreq->netfs_priv = file->private_data;
> req->cfile = cifsFileInfo_get(open_file);
> req->server = cifs_pick_channel(tlink_tcon(req->cfile->tlink)->ses);
> + if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_RWPIDFORWARD)
> + req->pid = req->cfile->pid;
> } else if (rreq->origin != NETFS_WRITEBACK) {
> WARN_ON_ONCE(1);
> return -EIO;
> diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> index e213cecd5094..2ae2dbb6202b 100644
> --- a/fs/smb/client/smb2pdu.c
> +++ b/fs/smb/client/smb2pdu.c
> @@ -4621,7 +4621,7 @@ smb2_async_readv(struct cifs_io_subrequest *rdata)
> io_parms.length = rdata->subreq.len;
> io_parms.persistent_fid = rdata->req->cfile->fid.persistent_fid;
> io_parms.volatile_fid = rdata->req->cfile->fid.volatile_fid;
> - io_parms.pid = rdata->pid;
> + io_parms.pid = rdata->req->pid;
>
> rc = smb2_new_read_req(
> (void **) &buf, &total_len, &io_parms, rdata, 0, 0);
> @@ -4873,7 +4873,7 @@ smb2_async_writev(struct cifs_io_subrequest *wdata)
> .length = wdata->subreq.len,
> .persistent_fid = wdata->req->cfile->fid.persistent_fid,
> .volatile_fid = wdata->req->cfile->fid.volatile_fid,
> - .pid = wdata->pid,
> + .pid = wdata->req->pid,
> };
> io_parms = &_io_parms;
>
>

--
Thanks,

Steve

[-- Attachment #2: 0001-cifs-Move-the-pid-from-the-subreq-to-the-req.patch --]
[-- Type: text/x-patch, Size: 4993 bytes --]

From 3f59138580bf8006fa99641b5803d0f683709f10 Mon Sep 17 00:00:00 2001
From: David Howells <dhowells@redhat.com>
Date: Thu, 20 Jun 2024 18:31:29 +0100
Subject: [PATCH] cifs: Move the 'pid' from the subreq to the req

Move the reference pid from the cifs_io_subrequest struct to the
cifs_io_request struct as it's the same for all subreqs of a particular
request.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Paulo Alcantara <pc@manguebit.com>
cc: Jeff Layton <jlayton@kernel.org>
cc: linux-cifs@vger.kernel.org
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>
---
fs/smb/client/cifsglob.h | 2 +-
fs/smb/client/cifssmb.c | 8 ++++----
fs/smb/client/file.c | 11 +++--------
fs/smb/client/smb2pdu.c | 4 ++--
4 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 0978997ddfa6..557b68e99d0a 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -1495,6 +1495,7 @@ struct cifs_io_request {
struct netfs_io_request rreq;
struct cifsFileInfo *cfile;
struct TCP_Server_Info *server;
+ pid_t pid;
};

/* asynchronous read support */
@@ -1505,7 +1506,6 @@ struct cifs_io_subrequest {
struct cifs_io_request *req;
};
ssize_t got_bytes;
- pid_t pid;
unsigned int xid;
int result;
bool have_xid;
diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index 25e9ab947c17..595c4b673707 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -1345,8 +1345,8 @@ cifs_async_readv(struct cifs_io_subrequest *rdata)
if (rc)
return rc;

- smb->hdr.Pid = cpu_to_le16((__u16)rdata->pid);
- smb->hdr.PidHigh = cpu_to_le16((__u16)(rdata->pid >> 16));
+ smb->hdr.Pid = cpu_to_le16((__u16)rdata->req->pid);
+ smb->hdr.PidHigh = cpu_to_le16((__u16)(rdata->req->pid >> 16));

smb->AndXCommand = 0xFF; /* none */
smb->Fid = rdata->req->cfile->fid.netfid;
@@ -1689,8 +1689,8 @@ cifs_async_writev(struct cifs_io_subrequest *wdata)
if (rc)
goto async_writev_out;

- smb->hdr.Pid = cpu_to_le16((__u16)wdata->pid);
- smb->hdr.PidHigh = cpu_to_le16((__u16)(wdata->pid >> 16));
+ smb->hdr.Pid = cpu_to_le16((__u16)wdata->req->pid);
+ smb->hdr.PidHigh = cpu_to_le16((__u16)(wdata->req->pid >> 16));

smb->AndXCommand = 0xFF; /* none */
smb->Fid = wdata->req->cfile->fid.netfid;
diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index 4dbd80168a2b..f1f2573bb18d 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -177,15 +177,8 @@ static void cifs_req_issue_read(struct netfs_io_subrequest *subreq)
struct netfs_io_request *rreq = subreq->rreq;
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);
- pid_t pid;
int rc = 0;

- if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_RWPIDFORWARD)
- pid = req->cfile->pid;
- else
- pid = current->tgid; // Ummm... This may be a workqueue
-
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);
@@ -199,7 +192,6 @@ static void cifs_req_issue_read(struct netfs_io_subrequest *subreq)
}

__set_bit(NETFS_SREQ_CLEAR_TAIL, &subreq->flags);
- rdata->pid = pid;

rc = rdata->server->ops->async_readv(rdata);
out:
@@ -236,12 +228,15 @@ static int cifs_init_request(struct netfs_io_request *rreq, struct file *file)

rreq->rsize = cifs_sb->ctx->rsize;
rreq->wsize = cifs_sb->ctx->wsize;
+ req->pid = current->tgid; // Ummm... This may be a workqueue

if (file) {
open_file = file->private_data;
rreq->netfs_priv = file->private_data;
req->cfile = cifsFileInfo_get(open_file);
req->server = cifs_pick_channel(tlink_tcon(req->cfile->tlink)->ses);
+ if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_RWPIDFORWARD)
+ req->pid = req->cfile->pid;
} else if (rreq->origin != NETFS_WRITEBACK) {
WARN_ON_ONCE(1);
return -EIO;
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index e213cecd5094..2ae2dbb6202b 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -4621,7 +4621,7 @@ smb2_async_readv(struct cifs_io_subrequest *rdata)
io_parms.length = rdata->subreq.len;
io_parms.persistent_fid = rdata->req->cfile->fid.persistent_fid;
io_parms.volatile_fid = rdata->req->cfile->fid.volatile_fid;
- io_parms.pid = rdata->pid;
+ io_parms.pid = rdata->req->pid;

rc = smb2_new_read_req(
(void **) &buf, &total_len, &io_parms, rdata, 0, 0);
@@ -4873,7 +4873,7 @@ smb2_async_writev(struct cifs_io_subrequest *wdata)
.length = wdata->subreq.len,
.persistent_fid = wdata->req->cfile->fid.persistent_fid,
.volatile_fid = wdata->req->cfile->fid.volatile_fid,
- .pid = wdata->pid,
+ .pid = wdata->req->pid,
};
io_parms = &_io_parms;

--
2.43.0