| Message ID | 162575798916.2532.6898942885852856593.stgit@klimt.1015granger.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Bulk-release pages during NFSD read splice | expand |
On Thu, Jul 08, 2021 at 11:26:29AM -0400, Chuck Lever wrote: > @@ -256,6 +256,9 @@ struct svc_rqst { > struct page * *rq_next_page; /* next reply page to use */ > struct page * *rq_page_end; /* one past the last page */ > > + struct page *rq_relpages[16]; > + int rq_numrelpages; This is only one struct page away from being a pagevec ... ? > @@ -838,6 +839,33 @@ svc_set_num_threads_sync(struct svc_serv *serv, struct svc_pool *pool, int nrser > } > EXPORT_SYMBOL_GPL(svc_set_num_threads_sync); > > +static void svc_rqst_release_replaced_pages(struct svc_rqst *rqstp) > +{ > + release_pages(rqstp->rq_relpages, rqstp->rq_numrelpages); > + rqstp->rq_numrelpages = 0; This would be pagevec_release()
> On Jul 8, 2021, at 7:30 PM, Matthew Wilcox <willy@infradead.org> wrote: > > On Thu, Jul 08, 2021 at 11:26:29AM -0400, Chuck Lever wrote: >> @@ -256,6 +256,9 @@ struct svc_rqst { >> struct page * *rq_next_page; /* next reply page to use */ >> struct page * *rq_page_end; /* one past the last page */ >> >> + struct page *rq_relpages[16]; >> + int rq_numrelpages; > > This is only one struct page away from being a pagevec ... ? > >> @@ -838,6 +839,33 @@ svc_set_num_threads_sync(struct svc_serv *serv, struct svc_pool *pool, int nrser >> } >> EXPORT_SYMBOL_GPL(svc_set_num_threads_sync); >> >> +static void svc_rqst_release_replaced_pages(struct svc_rqst *rqstp) >> +{ >> + release_pages(rqstp->rq_relpages, rqstp->rq_numrelpages); >> + rqstp->rq_numrelpages = 0; > > This would be pagevec_release() 986 void __pagevec_release(struct pagevec *pvec) 987 { 988 if (!pvec->percpu_pvec_drained) { 989 lru_add_drain(); 990 pvec->percpu_pvec_drained = true; 991 } 992 release_pages(pvec->pages, pagevec_count(pvec)); 993 pagevec_reinit(pvec); 994 } Pretty much the same under the bonnet. Fair enough! -- Chuck Lever
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h index e91d51ea028b..68a9f51e2624 100644 --- a/include/linux/sunrpc/svc.h +++ b/include/linux/sunrpc/svc.h @@ -256,6 +256,9 @@ struct svc_rqst { struct page * *rq_next_page; /* next reply page to use */ struct page * *rq_page_end; /* one past the last page */ + struct page *rq_relpages[16]; + int rq_numrelpages; + struct kvec rq_vec[RPCSVC_MAXPAGES]; /* generally useful.. */ struct bio_vec rq_bvec[RPCSVC_MAXPAGES]; @@ -502,6 +505,8 @@ struct svc_rqst *svc_rqst_alloc(struct svc_serv *serv, struct svc_pool *pool, int node); struct svc_rqst *svc_prepare_thread(struct svc_serv *serv, struct svc_pool *pool, int node); +void svc_rqst_replace_page(struct svc_rqst *rqstp, + struct page *page); void svc_rqst_free(struct svc_rqst *); void svc_exit_thread(struct svc_rqst *); unsigned int svc_pool_map_get(void); diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c index 0de918cb3d90..3679830cf123 100644 --- a/net/sunrpc/svc.c +++ b/net/sunrpc/svc.c @@ -21,6 +21,7 @@ #include <linux/module.h> #include <linux/kthread.h> #include <linux/slab.h> +#include <linux/pagemap.h> #include <linux/sunrpc/types.h> #include <linux/sunrpc/xdr.h> @@ -838,6 +839,33 @@ svc_set_num_threads_sync(struct svc_serv *serv, struct svc_pool *pool, int nrser } EXPORT_SYMBOL_GPL(svc_set_num_threads_sync); +static void svc_rqst_release_replaced_pages(struct svc_rqst *rqstp) +{ + release_pages(rqstp->rq_relpages, rqstp->rq_numrelpages); + rqstp->rq_numrelpages = 0; +} + +/** + * svc_rqst_replace_page - Replace one page in rq_pages[] + * @rqstp: svc_rqst with pages to replace + * @page: replacement page + * + * When replacing a page in rq_pages, batch the release of the + * replaced pages to avoid hammering put_page(). + */ +void svc_rqst_replace_page(struct svc_rqst *rqstp, struct page *page) +{ + if (*rqstp->rq_next_page) { + if (rqstp->rq_numrelpages >= ARRAY_SIZE(rqstp->rq_relpages)) + svc_rqst_release_replaced_pages(rqstp); + rqstp->rq_relpages[rqstp->rq_numrelpages++] = *rqstp->rq_next_page; + } + + get_page(page); + *(rqstp->rq_next_page++) = page; +} +EXPORT_SYMBOL_GPL(svc_rqst_replace_page); + /* * Called from a server thread as it's exiting. Caller must hold the "service * mutex" for the service. @@ -846,6 +874,7 @@ void svc_rqst_free(struct svc_rqst *rqstp) { svc_release_buffer(rqstp); + svc_rqst_release_replaced_pages(rqstp); if (rqstp->rq_scratch_page) put_page(rqstp->rq_scratch_page); kfree(rqstp->rq_resp);
Replacing a page in rq_pages[] requires a get_page(), which is a bus-locked operation, and a put_page(), which can be even more costly. To reduce the cost of replacing a page, batch the put_page() operations by collecting "free" pages in an array, and then passing them to release_pages() when the array becomes full. Signed-off-by: Chuck Lever <chuck.lever@oracle.com> --- include/linux/sunrpc/svc.h | 5 +++++ net/sunrpc/svc.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+)