* [PATCH] [CIFS]: Add clamp_length support
@ 2022-02-08 5:53 Rohith Surabattula
2022-02-16 21:25 ` David Howells
0 siblings, 1 reply; 9+ messages in thread
From: Rohith Surabattula @ 2022-02-08 5:53 UTC (permalink / raw)
To: Steve French, David Howells, Shyam Prasad N, ronnie sahlberg,
Paulo Alcantara, linux-cifs
[-- Attachment #1: Type: text/plain, Size: 370 bytes --]
Hi All,
Added clamp length handler to further slice the read requests based on
available credits.
This patch needs to be applied on top of Dave howells 7-patch
series(netfs integration for cifs).
@David Howells As the above 7-patch series is dependent on netfs lib
changes which are not merged yet. Please let us know the timelines for
mege process.
Regards,
Rohith
[-- Attachment #2: CIFS-Add-clamp_length-support.patch --]
[-- Type: application/octet-stream, Size: 4579 bytes --]
From e0eee5ff329273985e41e3ae9feb6c27dcfdb419 Mon Sep 17 00:00:00 2001
From: Rohith Surabattula <rohiths@microsoft.com>
Date: Tue, 8 Feb 2022 05:38:29 +0000
Subject: [PATCH] [CIFS]: Add clamp_length support
Added clamp length handler to slice the read requests further based on available credits.
Signed-off-by: Rohith Surabattula <rohiths@microsoft.com>
---
fs/cifs/file.c | 40 ++++++++++++++++++++++++++++++++--------
fs/cifs/smb2ops.c | 1 +
include/linux/netfs.h | 1 +
3 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 36559de02e37..8f4961960beb 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -3459,11 +3459,10 @@ static void cifs_req_issue_op(struct netfs_read_subrequest *subreq)
struct cifs_readdata *rdata;
struct cifsFileInfo *open_file = rreq->netfs_priv;
struct cifs_sb_info *cifs_sb = CIFS_SB(rreq->inode->i_sb);
- struct cifs_credits credits_on_stack, *credits = &credits_on_stack;
+ struct cifs_credits *credits;
unsigned int xid;
pid_t pid;
int rc = 0;
- unsigned int rsize;
xid = get_xid();
@@ -3478,18 +3477,18 @@ static void cifs_req_issue_op(struct netfs_read_subrequest *subreq)
__func__, rreq->debug_id, subreq->debug_index, rreq->mapping,
subreq->transferred, subreq->len);
+ credits = subreq->subreq_priv;
+
if (open_file->invalidHandle) {
do {
rc = cifs_reopen_file(open_file, true);
} while (rc == -EAGAIN);
- if (rc)
+ if (rc) {
+ add_credits_and_wake_if(server, credits, 0);
goto out;
+ }
}
- rc = server->ops->wait_mtu_credits(server, cifs_sb->ctx->rsize, &rsize, credits);
- if (rc)
- goto out;
-
rdata = cifs_readdata_alloc(NULL);
if (!rdata) {
add_credits_and_wake_if(server, credits, 0);
@@ -3504,7 +3503,7 @@ static void cifs_req_issue_op(struct netfs_read_subrequest *subreq)
rdata->offset = subreq->start + subreq->transferred;
rdata->bytes = subreq->len - subreq->transferred;
rdata->pid = pid;
- rdata->credits = credits_on_stack;
+ rdata->credits = *credits;
rdata->iter = subreq->iter;
rc = adjust_credits(server, &rdata->credits, rdata->bytes);
@@ -3526,6 +3525,7 @@ static void cifs_req_issue_op(struct netfs_read_subrequest *subreq)
out:
free_xid(xid);
+ kfree(credits);
if (rc)
netfs_subreq_terminated(subreq, rc, false);
}
@@ -3566,6 +3566,29 @@ static void cifs_expand_readahead(struct netfs_read_request *rreq)
rreq->len = i_size - rreq->start;
}
+static bool cifs_clamp_length(struct netfs_read_subrequest *subreq)
+{
+ struct cifs_sb_info *cifs_sb = CIFS_SB(subreq->rreq->inode->i_sb);
+ struct TCP_Server_Info *server;
+ struct cifsFileInfo *open_file = subreq->rreq->netfs_priv;
+ struct cifs_credits *credits;
+ unsigned int rsize;
+ int rc;
+
+ credits = kmalloc(sizeof(struct cifs_credits), GFP_KERNEL);
+ server = cifs_pick_channel(tlink_tcon(open_file->tlink)->ses);
+
+ rc = server->ops->wait_mtu_credits(server, cifs_sb->ctx->rsize,
+ &rsize, credits);
+ if (rc)
+ return false;
+
+ subreq->len = rsize;
+ subreq->subreq_priv = credits;
+ return true;
+}
+
+
static void cifs_rreq_done(struct netfs_read_request *rreq)
{
struct inode *inode = rreq->inode;
@@ -3586,6 +3609,7 @@ const struct netfs_request_ops cifs_req_ops = {
.init_rreq = cifs_init_rreq,
.expand_readahead = cifs_expand_readahead,
.issue_op = cifs_req_issue_op,
+ .clamp_length = cifs_clamp_length,
.done = cifs_rreq_done,
.cleanup = cifs_req_cleanup,
};
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index e1649ac194db..5faf45672891 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -4917,6 +4917,7 @@ handle_read_data(struct TCP_Server_Info *server, struct mid_q_entry *mid,
length = copy_to_iter(buf + data_offset, data_len, &rdata->iter);
if (length < 0)
return length;
+ rdata->got_bytes = data_len;
} else {
/* read response payload cannot be in both buf and pages */
WARN_ONCE(1, "buf can not contain only a part of read data");
diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index b05b7a7d90e6..e8fb1b3694c6 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -170,6 +170,7 @@ struct netfs_read_subrequest {
#define NETFS_SREQ_SHORT_READ 2 /* Set if there was a short read from the cache */
#define NETFS_SREQ_SEEK_DATA_READ 3 /* Set if ->read() should SEEK_DATA first */
#define NETFS_SREQ_NO_PROGRESS 4 /* Set if we didn't manage to read any data */
+ void *subreq_priv;
};
enum netfs_read_origin {
--
2.32.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] [CIFS]: Add clamp_length support
2022-02-08 5:53 [PATCH] [CIFS]: Add clamp_length support Rohith Surabattula
@ 2022-02-16 21:25 ` David Howells
2022-02-28 14:23 ` Rohith Surabattula
2022-02-28 14:39 ` David Howells
0 siblings, 2 replies; 9+ messages in thread
From: David Howells @ 2022-02-16 21:25 UTC (permalink / raw)
To: Rohith Surabattula
Cc: dhowells, Steve French, Shyam Prasad N, ronnie sahlberg,
Paulo Alcantara, linux-cifs
Rohith Surabattula <rohiths.msft@gmail.com> wrote:
> + credits = kmalloc(sizeof(struct cifs_credits), GFP_KERNEL);
> ...
> + subreq->subreq_priv = credits;
Would it be better if I made it so that the netfs could specify the size of
the netfs_read_subrequest struct to be allocated, thereby allowing it to tag
extra data on the end?
David
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [CIFS]: Add clamp_length support
2022-02-16 21:25 ` David Howells
@ 2022-02-28 14:23 ` Rohith Surabattula
2022-02-28 14:39 ` David Howells
1 sibling, 0 replies; 9+ messages in thread
From: Rohith Surabattula @ 2022-02-28 14:23 UTC (permalink / raw)
To: David Howells
Cc: Steve French, Shyam Prasad N, ronnie sahlberg, Paulo Alcantara,
linux-cifs
Hi David,
Do you mean the clamp handler in netfs should return the size of data
to be allocated instead of allocating itself ?
Regards,
Rohith
On Thu, Feb 17, 2022 at 2:56 AM David Howells <dhowells@redhat.com> wrote:
>
> Rohith Surabattula <rohiths.msft@gmail.com> wrote:
>
> > + credits = kmalloc(sizeof(struct cifs_credits), GFP_KERNEL);
> > ...
> > + subreq->subreq_priv = credits;
>
> Would it be better if I made it so that the netfs could specify the size of
> the netfs_read_subrequest struct to be allocated, thereby allowing it to tag
> extra data on the end?
>
> David
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [CIFS]: Add clamp_length support
2022-02-16 21:25 ` David Howells
2022-02-28 14:23 ` Rohith Surabattula
@ 2022-02-28 14:39 ` David Howells
2022-03-02 11:05 ` Rohith Surabattula
` (2 more replies)
1 sibling, 3 replies; 9+ messages in thread
From: David Howells @ 2022-02-28 14:39 UTC (permalink / raw)
To: Rohith Surabattula
Cc: dhowells, Steve French, Shyam Prasad N, ronnie sahlberg,
Paulo Alcantara, linux-cifs
Rohith Surabattula <rohiths.msft@gmail.com> wrote:
> > Rohith Surabattula <rohiths.msft@gmail.com> wrote:
> >
> > > + credits = kmalloc(sizeof(struct cifs_credits), GFP_KERNEL);
> > > ...
> > > + subreq->subreq_priv = credits;
> >
> > Would it be better if I made it so that the netfs could specify the size of
> > the netfs_read_subrequest struct to be allocated, thereby allowing it to tag
> > extra data on the end?
>
> Do you mean the clamp handler in netfs should return the size of data
> to be allocated instead of allocating itself ?
No, I was thinking of putting a size_t in struct netfs_request_ops that
indicates how big the subrequest struct should be:
struct netfs_request_ops {
...
size_t subrequest_size;
};
and then:
struct netfs_read_subrequest *netfs_alloc_subrequest(
struct netfs_read_request *rreq)
{
struct netfs_read_subrequest *subreq;
subreq = kzalloc(rreq->ops->subrequest_size, GFP_KERNEL);
if (subreq) {
INIT_LIST_HEAD(&subreq->rreq_link);
refcount_set(&subreq->usage, 2);
subreq->rreq = rreq;
netfs_get_read_request(rreq);
netfs_stat(&netfs_n_rh_sreq);
}
return subreq;
}
This would allow you to do, for instance:
struct cifs_subrequest {
struct netfs_read_subrequest subreq;
struct cifs_credits credits;
};
then:
const struct netfs_request_ops cifs_req_ops = {
.subrequest_size = sizeof(struct cifs_subrequest),
.init_rreq = cifs_init_rreq,
.expand_readahead = cifs_expand_readahead,
.clamp_length = cifs_clamp_length,
.issue_op = cifs_req_issue_op,
.done = cifs_rreq_done,
.cleanup = cifs_req_cleanup,
};
and then:
static bool cifs_clamp_length(struct netfs_read_subrequest *subreq)
{
struct cifs_subrequest *cifs_subreq =
container_of(subreq, struct cifs_subrequest, subreq);
struct cifs_sb_info *cifs_sb = CIFS_SB(subreq->rreq->inode->i_sb);
struct TCP_Server_Info *server;
struct cifsFileInfo *open_file = subreq->rreq->netfs_priv;
struct cifs_credits *credits = &cifs_subreq->credits;
unsigned int rsize;
int rc;
server = cifs_pick_channel(tlink_tcon(open_file->tlink)->ses);
rc = server->ops->wait_mtu_credits(server, cifs_sb->ctx->rsize,
&rsize, credits);
if (rc)
return false;
subreq->len = rsize;
return true;
}
David
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [CIFS]: Add clamp_length support
2022-02-28 14:39 ` David Howells
@ 2022-03-02 11:05 ` Rohith Surabattula
2022-03-02 15:16 ` David Howells
2022-03-09 12:58 ` David Howells
2 siblings, 0 replies; 9+ messages in thread
From: Rohith Surabattula @ 2022-03-02 11:05 UTC (permalink / raw)
To: David Howells
Cc: Steve French, Shyam Prasad N, ronnie sahlberg, Paulo Alcantara,
linux-cifs
Yes, It will look better. Agree with your idea. I will work on it.
Regards,
Rohith
On Mon, Feb 28, 2022 at 8:09 PM David Howells <dhowells@redhat.com> wrote:
>
> Rohith Surabattula <rohiths.msft@gmail.com> wrote:
>
> > > Rohith Surabattula <rohiths.msft@gmail.com> wrote:
> > >
> > > > + credits = kmalloc(sizeof(struct cifs_credits), GFP_KERNEL);
> > > > ...
> > > > + subreq->subreq_priv = credits;
> > >
> > > Would it be better if I made it so that the netfs could specify the size of
> > > the netfs_read_subrequest struct to be allocated, thereby allowing it to tag
> > > extra data on the end?
> >
> > Do you mean the clamp handler in netfs should return the size of data
> > to be allocated instead of allocating itself ?
>
> No, I was thinking of putting a size_t in struct netfs_request_ops that
> indicates how big the subrequest struct should be:
>
> struct netfs_request_ops {
> ...
> size_t subrequest_size;
> };
>
> and then:
>
> struct netfs_read_subrequest *netfs_alloc_subrequest(
> struct netfs_read_request *rreq)
> {
> struct netfs_read_subrequest *subreq;
>
> subreq = kzalloc(rreq->ops->subrequest_size, GFP_KERNEL);
> if (subreq) {
> INIT_LIST_HEAD(&subreq->rreq_link);
> refcount_set(&subreq->usage, 2);
> subreq->rreq = rreq;
> netfs_get_read_request(rreq);
> netfs_stat(&netfs_n_rh_sreq);
> }
>
> return subreq;
> }
>
> This would allow you to do, for instance:
>
> struct cifs_subrequest {
> struct netfs_read_subrequest subreq;
> struct cifs_credits credits;
> };
>
> then:
>
> const struct netfs_request_ops cifs_req_ops = {
> .subrequest_size = sizeof(struct cifs_subrequest),
> .init_rreq = cifs_init_rreq,
> .expand_readahead = cifs_expand_readahead,
> .clamp_length = cifs_clamp_length,
> .issue_op = cifs_req_issue_op,
> .done = cifs_rreq_done,
> .cleanup = cifs_req_cleanup,
> };
>
> and then:
>
> static bool cifs_clamp_length(struct netfs_read_subrequest *subreq)
> {
> struct cifs_subrequest *cifs_subreq =
> container_of(subreq, struct cifs_subrequest, subreq);
> struct cifs_sb_info *cifs_sb = CIFS_SB(subreq->rreq->inode->i_sb);
> struct TCP_Server_Info *server;
> struct cifsFileInfo *open_file = subreq->rreq->netfs_priv;
> struct cifs_credits *credits = &cifs_subreq->credits;
> unsigned int rsize;
> int rc;
>
> server = cifs_pick_channel(tlink_tcon(open_file->tlink)->ses);
>
> rc = server->ops->wait_mtu_credits(server, cifs_sb->ctx->rsize,
> &rsize, credits);
> if (rc)
> return false;
>
> subreq->len = rsize;
> return true;
> }
>
> David
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [CIFS]: Add clamp_length support
2022-02-28 14:39 ` David Howells
2022-03-02 11:05 ` Rohith Surabattula
@ 2022-03-02 15:16 ` David Howells
2022-03-02 15:26 ` Rohith Surabattula
2022-03-02 15:47 ` David Howells
2022-03-09 12:58 ` David Howells
2 siblings, 2 replies; 9+ messages in thread
From: David Howells @ 2022-03-02 15:16 UTC (permalink / raw)
To: Rohith Surabattula
Cc: dhowells, Steve French, Shyam Prasad N, ronnie sahlberg,
Paulo Alcantara, linux-cifs
Rohith Surabattula <rohiths.msft@gmail.com> wrote:
> > struct netfs_read_subrequest *netfs_alloc_subrequest(
Btw, note the patches I just posted that rename these structs and some of the
functions.
David
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [CIFS]: Add clamp_length support
2022-03-02 15:16 ` David Howells
@ 2022-03-02 15:26 ` Rohith Surabattula
2022-03-02 15:47 ` David Howells
1 sibling, 0 replies; 9+ messages in thread
From: Rohith Surabattula @ 2022-03-02 15:26 UTC (permalink / raw)
To: David Howells
Cc: Steve French, Shyam Prasad N, ronnie sahlberg, Paulo Alcantara,
linux-cifs
Hi David,
Patches which you posted today are on top of 7-patch series "[RFC][RFC
PATCH 0/7] cifs: In-progress conversion to use iov_iters and netfslib"
which you published earlier?
or this 7-patch series needs to be reworked ?
Regards,
Rohith
On Wed, Mar 2, 2022 at 8:46 PM David Howells <dhowells@redhat.com> wrote:
>
> Rohith Surabattula <rohiths.msft@gmail.com> wrote:
>
> > > struct netfs_read_subrequest *netfs_alloc_subrequest(
>
> Btw, note the patches I just posted that rename these structs and some of the
> functions.
>
> David
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [CIFS]: Add clamp_length support
2022-03-02 15:16 ` David Howells
2022-03-02 15:26 ` Rohith Surabattula
@ 2022-03-02 15:47 ` David Howells
1 sibling, 0 replies; 9+ messages in thread
From: David Howells @ 2022-03-02 15:47 UTC (permalink / raw)
To: Rohith Surabattula
Cc: dhowells, Steve French, Shyam Prasad N, ronnie sahlberg,
Paulo Alcantara, linux-cifs
Rohith Surabattula <rohiths.msft@gmail.com> wrote:
> Patches which you posted today are on top of 7-patch series "[RFC][RFC
> PATCH 0/7] cifs: In-progress conversion to use iov_iters and netfslib"
> which you published earlier?
They're a subset of the patches on my netfs-lib branch on top of which the
7-patch series was based.
> or this 7-patch series needs to be reworked ?
Yeah. I'm going to have a look at that rebasing my netfs-lib and
cifs-experimental branches tomorrow.
David
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [CIFS]: Add clamp_length support
2022-02-28 14:39 ` David Howells
2022-03-02 11:05 ` Rohith Surabattula
2022-03-02 15:16 ` David Howells
@ 2022-03-09 12:58 ` David Howells
2 siblings, 0 replies; 9+ messages in thread
From: David Howells @ 2022-03-09 12:58 UTC (permalink / raw)
To: Rohith Surabattula
Cc: dhowells, Steve French, Shyam Prasad N, ronnie sahlberg,
Paulo Alcantara, linux-cifs
Rohith Surabattula <rohiths.msft@gmail.com> wrote:
> > No, I was thinking of putting a size_t in struct netfs_request_ops that
> > indicates how big the subrequest struct should be:
> ...
> Yes, It will look better. Agree with your idea. I will work on it.
See the patches I've just added to my cifs-experimental branch.
David
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-03-09 12:59 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-08 5:53 [PATCH] [CIFS]: Add clamp_length support Rohith Surabattula
2022-02-16 21:25 ` David Howells
2022-02-28 14:23 ` Rohith Surabattula
2022-02-28 14:39 ` David Howells
2022-03-02 11:05 ` Rohith Surabattula
2022-03-02 15:16 ` David Howells
2022-03-02 15:26 ` Rohith Surabattula
2022-03-02 15:47 ` David Howells
2022-03-09 12:58 ` David Howells
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).