[PATCH] cifs: pick channels for individual subrequests

[PATCH] cifs: pick channels for individual subrequests

[nspmangalore]

From: Shyam Prasad N <sprasad@microsoft.com>

The netfs library could break down a read request into
multiple subrequests. When multichannel is used, there is
potential to improve performance when each of these
subrequests pick a different channel.

Today we call cifs_pick_channel when the main read request
is initialized in cifs_init_request. This change moves this to
cifs_prepare_read, which is the right place to pick channel since
it gets called for each subrequest.

Interestingly cifs_prepare_write already does channel selection
for individual subreq, but looks like it was missed for read.
This is especially important when multichannel is used with
increased rasize.

In my test setup, with rasize set to 8MB, a sequential read
of large file was taking 11.5s without this change. With the
change, it completed in 9s. The difference is even more signigicant
with bigger rasize.

Cc: <stable@vger.kernel.org>
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
---
 fs/smb/client/cifsglob.h | 1 -
 fs/smb/client/file.c     | 7 ++++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index a68434ad744a..243e4881528c 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -1508,7 +1508,6 @@ struct cifs_io_parms {
 struct cifs_io_request {
 	struct netfs_io_request		rreq;
 	struct cifsFileInfo		*cfile;
-	struct TCP_Server_Info		*server;
 	pid_t				pid;
 };
 
diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
index 79de2f2f9c41..8582cf61242c 100644
--- a/fs/smb/client/file.c
+++ b/fs/smb/client/file.c
@@ -147,7 +147,7 @@ static int cifs_prepare_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 TCP_Server_Info *server = req->server;
+	struct TCP_Server_Info *server;
 	struct cifs_sb_info *cifs_sb = CIFS_SB(rreq->inode->i_sb);
 	size_t size;
 	int rc = 0;
@@ -156,6 +156,8 @@ static int cifs_prepare_read(struct netfs_io_subrequest *subreq)
 		rdata->xid = get_xid();
 		rdata->have_xid = true;
 	}
+
+	server = cifs_pick_channel(tlink_tcon(req->cfile->tlink)->ses);
 	rdata->server = server;
 
 	if (cifs_sb->ctx->rsize == 0)
@@ -198,7 +200,7 @@ static void cifs_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 TCP_Server_Info *server = req->server;
+	struct TCP_Server_Info *server = rdata->server;
 	int rc = 0;
 
 	cifs_dbg(FYI, "%s: op=%08x[%x] mapping=%p len=%zu/%zu\n",
@@ -266,7 +268,6 @@ static int cifs_init_request(struct netfs_io_request *rreq, struct file *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) {
-- 
2.43.0

Re: [PATCH] cifs: pick channels for individual subrequests

[Steve French]

tentatively merged to cifs-2.6.git for-next pending more testing/review

On Tue, Feb 11, 2025 at 4:01 AM <nspmangalore@gmail.com> wrote:
>
> From: Shyam Prasad N <sprasad@microsoft.com>
>
> The netfs library could break down a read request into
> multiple subrequests. When multichannel is used, there is
> potential to improve performance when each of these
> subrequests pick a different channel.
>
> Today we call cifs_pick_channel when the main read request
> is initialized in cifs_init_request. This change moves this to
> cifs_prepare_read, which is the right place to pick channel since
> it gets called for each subrequest.
>
> Interestingly cifs_prepare_write already does channel selection
> for individual subreq, but looks like it was missed for read.
> This is especially important when multichannel is used with
> increased rasize.
>
> In my test setup, with rasize set to 8MB, a sequential read
> of large file was taking 11.5s without this change. With the
> change, it completed in 9s. The difference is even more signigicant
> with bigger rasize.
>
> Cc: <stable@vger.kernel.org>
> Cc: David Howells <dhowells@redhat.com>
> Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
> ---
>  fs/smb/client/cifsglob.h | 1 -
>  fs/smb/client/file.c     | 7 ++++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> index a68434ad744a..243e4881528c 100644
> --- a/fs/smb/client/cifsglob.h
> +++ b/fs/smb/client/cifsglob.h
> @@ -1508,7 +1508,6 @@ struct cifs_io_parms {
>  struct cifs_io_request {
>         struct netfs_io_request         rreq;
>         struct cifsFileInfo             *cfile;
> -       struct TCP_Server_Info          *server;
>         pid_t                           pid;
>  };
>
> diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
> index 79de2f2f9c41..8582cf61242c 100644
> --- a/fs/smb/client/file.c
> +++ b/fs/smb/client/file.c
> @@ -147,7 +147,7 @@ static int cifs_prepare_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 TCP_Server_Info *server = req->server;
> +       struct TCP_Server_Info *server;
>         struct cifs_sb_info *cifs_sb = CIFS_SB(rreq->inode->i_sb);
>         size_t size;
>         int rc = 0;
> @@ -156,6 +156,8 @@ static int cifs_prepare_read(struct netfs_io_subrequest *subreq)
>                 rdata->xid = get_xid();
>                 rdata->have_xid = true;
>         }
> +
> +       server = cifs_pick_channel(tlink_tcon(req->cfile->tlink)->ses);
>         rdata->server = server;
>
>         if (cifs_sb->ctx->rsize == 0)
> @@ -198,7 +200,7 @@ static void cifs_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 TCP_Server_Info *server = req->server;
> +       struct TCP_Server_Info *server = rdata->server;
>         int rc = 0;
>
>         cifs_dbg(FYI, "%s: op=%08x[%x] mapping=%p len=%zu/%zu\n",
> @@ -266,7 +268,6 @@ static int cifs_init_request(struct netfs_io_request *rreq, struct file *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) {
> --
> 2.43.0
>


-- 
Thanks,

Steve