| Message ID | 161651220579.3977.8959177746864957646.stgit@klimt.1015granger.net (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | SUNRPC consumer for the bulk page allocator | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | warning | 5 maintainers not CCed: anna.schumaker@netapp.com trond.myklebust@hammerspace.com bfields@fieldses.org davem@davemloft.net kuba@kernel.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | fail | Errors and warnings before: 0 this patch: 6 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: Block comments use a trailing */ on a separate line |
| netdev/build_allmodconfig_warn | fail | Errors and warnings before: 0 this patch: 6 |
| netdev/header_inline | success | Link |
On Tue, Mar 23, 2021 at 11:10:05AM -0400, Chuck Lever wrote: > Reduce the rate at which nfsd threads hammer on the page allocator. > This improves throughput scalability by enabling the threads to run > more independently of each other. > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> I've picked up the series and merged the leader with the first patch because I think the array vs list data is interesting but I did change the patch. > + for (;;) { > + filled = alloc_pages_bulk_array(GFP_KERNEL, pages, > + rqstp->rq_pages); > + /* We assume that if the next array element is populated, > + * all the following elements are as well, thus we're done. */ > + if (filled == pages || rqstp->rq_pages[filled]) > + break; > + I altered this check because the implementation now returns a useful index. I know I had concerns about this but while the implementation cost is higher, the caller needs less knowledge of alloc_bulk_pages implementation. It might be unfortunate if new users all had to have their own optimisations around hole management so lets keep it simpler to start with. Version current in my tree is below but also available in git://git.kernel.org/pub/scm/linux/kernel/git/mel/linux.git mm-bulk-rebase-v6r5 ---8<--- SUNRPC: Refresh rq_pages using a bulk page allocator From: Chuck Lever <chuck.lever@oracle.com> Reduce the rate at which nfsd threads hammer on the page allocator. This improves throughput scalability by enabling the threads to run more independently of each other. [mgorman: Update interpretation of alloc_pages_bulk return value] Signed-off-by: Chuck Lever <chuck.lever@oracle.com> Signed-off-by: Mel Gorman <mgorman@techsingularity.net> --- net/sunrpc/svc_xprt.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c index 609bda97d4ae..0c27c3291ca1 100644 --- a/net/sunrpc/svc_xprt.c +++ b/net/sunrpc/svc_xprt.c @@ -643,30 +643,29 @@ static int svc_alloc_arg(struct svc_rqst *rqstp) { struct svc_serv *serv = rqstp->rq_server; struct xdr_buf *arg = &rqstp->rq_arg; - int pages; - int i; + unsigned long pages, filled; - /* now allocate needed pages. If we get a failure, sleep briefly */ pages = (serv->sv_max_mesg + 2 * PAGE_SIZE) >> PAGE_SHIFT; if (pages > RPCSVC_MAXPAGES) { - pr_warn_once("svc: warning: pages=%u > RPCSVC_MAXPAGES=%lu\n", + pr_warn_once("svc: warning: pages=%lu > RPCSVC_MAXPAGES=%lu\n", pages, RPCSVC_MAXPAGES); /* use as many pages as possible */ pages = RPCSVC_MAXPAGES; } - for (i = 0; i < pages ; i++) - while (rqstp->rq_pages[i] == NULL) { - struct page *p = alloc_page(GFP_KERNEL); - if (!p) { - set_current_state(TASK_INTERRUPTIBLE); - if (signalled() || kthread_should_stop()) { - set_current_state(TASK_RUNNING); - return -EINTR; - } - schedule_timeout(msecs_to_jiffies(500)); - } - rqstp->rq_pages[i] = p; + + for (;;) { + filled = alloc_pages_bulk_array(GFP_KERNEL, pages, + rqstp->rq_pages); + if (filled == pages) + break; + + set_current_state(TASK_INTERRUPTIBLE); + if (signalled() || kthread_should_stop()) { + set_current_state(TASK_RUNNING); + return -EINTR; } + schedule_timeout(msecs_to_jiffies(500)); + } rqstp->rq_page_end = &rqstp->rq_pages[pages]; rqstp->rq_pages[pages] = NULL; /* this might be seen in nfsd_splice_actor() */
> On Mar 23, 2021, at 3:56 PM, Mel Gorman <mgorman@techsingularity.net> wrote: > > On Tue, Mar 23, 2021 at 11:10:05AM -0400, Chuck Lever wrote: >> Reduce the rate at which nfsd threads hammer on the page allocator. >> This improves throughput scalability by enabling the threads to run >> more independently of each other. >> >> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > > I've picked up the series and merged the leader with the first patch > because I think the array vs list data is interesting but I did change > the patch. > >> + for (;;) { >> + filled = alloc_pages_bulk_array(GFP_KERNEL, pages, >> + rqstp->rq_pages); >> + /* We assume that if the next array element is populated, >> + * all the following elements are as well, thus we're done. */ >> + if (filled == pages || rqstp->rq_pages[filled]) >> + break; >> + > > I altered this check because the implementation now returns a useful > index. I know I had concerns about this but while the implementation > cost is higher, the caller needs less knowledge of alloc_bulk_pages > implementation. It might be unfortunate if new users all had to have > their own optimisations around hole management so lets keep it simpler > to start with. Agreed! Your version below looks like what I'm testing now -- the "rq_pages[filled]" test and the comment have been removed. > Version current in my tree is below but also available in > > git://git.kernel.org/pub/scm/linux/kernel/git/mel/linux.git mm-bulk-rebase-v6r5 > > ---8<--- > SUNRPC: Refresh rq_pages using a bulk page allocator > > From: Chuck Lever <chuck.lever@oracle.com> > > Reduce the rate at which nfsd threads hammer on the page allocator. > This improves throughput scalability by enabling the threads to run > more independently of each other. > > [mgorman: Update interpretation of alloc_pages_bulk return value] > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > Signed-off-by: Mel Gorman <mgorman@techsingularity.net> > --- > net/sunrpc/svc_xprt.c | 31 +++++++++++++++---------------- > 1 file changed, 15 insertions(+), 16 deletions(-) > > diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c > index 609bda97d4ae..0c27c3291ca1 100644 > --- a/net/sunrpc/svc_xprt.c > +++ b/net/sunrpc/svc_xprt.c > @@ -643,30 +643,29 @@ static int svc_alloc_arg(struct svc_rqst *rqstp) > { > struct svc_serv *serv = rqstp->rq_server; > struct xdr_buf *arg = &rqstp->rq_arg; > - int pages; > - int i; > + unsigned long pages, filled; > > - /* now allocate needed pages. If we get a failure, sleep briefly */ > pages = (serv->sv_max_mesg + 2 * PAGE_SIZE) >> PAGE_SHIFT; > if (pages > RPCSVC_MAXPAGES) { > - pr_warn_once("svc: warning: pages=%u > RPCSVC_MAXPAGES=%lu\n", > + pr_warn_once("svc: warning: pages=%lu > RPCSVC_MAXPAGES=%lu\n", > pages, RPCSVC_MAXPAGES); > /* use as many pages as possible */ > pages = RPCSVC_MAXPAGES; > } > - for (i = 0; i < pages ; i++) > - while (rqstp->rq_pages[i] == NULL) { > - struct page *p = alloc_page(GFP_KERNEL); > - if (!p) { > - set_current_state(TASK_INTERRUPTIBLE); > - if (signalled() || kthread_should_stop()) { > - set_current_state(TASK_RUNNING); > - return -EINTR; > - } > - schedule_timeout(msecs_to_jiffies(500)); > - } > - rqstp->rq_pages[i] = p; > + > + for (;;) { > + filled = alloc_pages_bulk_array(GFP_KERNEL, pages, > + rqstp->rq_pages); > + if (filled == pages) > + break; > + > + set_current_state(TASK_INTERRUPTIBLE); > + if (signalled() || kthread_should_stop()) { > + set_current_state(TASK_RUNNING); > + return -EINTR; > } > + schedule_timeout(msecs_to_jiffies(500)); > + } > rqstp->rq_page_end = &rqstp->rq_pages[pages]; > rqstp->rq_pages[pages] = NULL; /* this might be seen in nfsd_splice_actor() */ > -- Chuck Lever
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c index 609bda97d4ae..d2792b2bf006 100644 --- a/net/sunrpc/svc_xprt.c +++ b/net/sunrpc/svc_xprt.c @@ -643,30 +643,31 @@ static int svc_alloc_arg(struct svc_rqst *rqstp) { struct svc_serv *serv = rqstp->rq_server; struct xdr_buf *arg = &rqstp->rq_arg; - int pages; - int i; + unsigned long pages, filled; - /* now allocate needed pages. If we get a failure, sleep briefly */ pages = (serv->sv_max_mesg + 2 * PAGE_SIZE) >> PAGE_SHIFT; if (pages > RPCSVC_MAXPAGES) { - pr_warn_once("svc: warning: pages=%u > RPCSVC_MAXPAGES=%lu\n", + pr_warn_once("svc: warning: pages=%lu > RPCSVC_MAXPAGES=%lu\n", pages, RPCSVC_MAXPAGES); /* use as many pages as possible */ pages = RPCSVC_MAXPAGES; } - for (i = 0; i < pages ; i++) - while (rqstp->rq_pages[i] == NULL) { - struct page *p = alloc_page(GFP_KERNEL); - if (!p) { - set_current_state(TASK_INTERRUPTIBLE); - if (signalled() || kthread_should_stop()) { - set_current_state(TASK_RUNNING); - return -EINTR; - } - schedule_timeout(msecs_to_jiffies(500)); - } - rqstp->rq_pages[i] = p; + + for (;;) { + filled = alloc_pages_bulk_array(GFP_KERNEL, pages, + rqstp->rq_pages); + /* We assume that if the next array element is populated, + * all the following elements are as well, thus we're done. */ + if (filled == pages || rqstp->rq_pages[filled]) + break; + + set_current_state(TASK_INTERRUPTIBLE); + if (signalled() || kthread_should_stop()) { + set_current_state(TASK_RUNNING); + return -EINTR; } + schedule_timeout(msecs_to_jiffies(500)); + } rqstp->rq_page_end = &rqstp->rq_pages[pages]; rqstp->rq_pages[pages] = NULL; /* this might be seen in nfsd_splice_actor() */
Reduce the rate at which nfsd threads hammer on the page allocator. This improves throughput scalability by enabling the threads to run more independently of each other. Signed-off-by: Chuck Lever <chuck.lever@oracle.com> --- net/sunrpc/svc_xprt.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-)