I've found a few ways to optimize the release of pages in NFSD.
Please let me know if I'm abusing the release_pages() and pagevec
APIs.
---
Chuck Lever (4):
SUNRPC: Relocate svc_free_res_pages()
SUNRPC: Convert svc_xprt_release() to the release_pages() API
SUNRPC: Convert svc_tcp_restore_pages() to the release_pages() API
SUNRPC: Be even lazier about releasing pages
include/linux/sunrpc/svc.h | 12 +-----------
net/sunrpc/svc.c | 21 +++++++++++++++++++++
net/sunrpc/svc_xprt.c | 5 +----
net/sunrpc/svcsock.c | 12 ++++++------
4 files changed, 29 insertions(+), 21 deletions(-)
--
Chuck Lever
From: Chuck Lever <[email protected]>
A single RPC transaction that touches only a couple of pages means
rq_pvec will not be even close to full in svc_xpt_release(). This is
a common case.
Instead, just leave the pages in rq_pvec until it is completely
full. This improves the efficiency of the batch release mechanism
on workloads that involve small RPC messages.
The rq_pvec is also fully emptied just before thread exit.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/svc.c | 3 +++
net/sunrpc/svc_xprt.c | 3 ---
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index b982f802f2a0..26367cf4c17a 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -649,6 +649,8 @@ svc_rqst_alloc(struct svc_serv *serv, struct svc_pool *pool, int node)
if (!rqstp)
return rqstp;
+ pagevec_init(&rqstp->rq_pvec);
+
__set_bit(RQ_BUSY, &rqstp->rq_flags);
rqstp->rq_server = serv;
rqstp->rq_pool = pool;
@@ -894,6 +896,7 @@ void svc_rqst_release_pages(struct svc_rqst *rqstp)
void
svc_rqst_free(struct svc_rqst *rqstp)
{
+ pagevec_release(&rqstp->rq_pvec);
svc_release_buffer(rqstp);
if (rqstp->rq_scratch_page)
put_page(rqstp->rq_scratch_page);
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 533e08c4f319..e3952b690f54 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -541,7 +541,6 @@ static void svc_xprt_release(struct svc_rqst *rqstp)
kfree(rqstp->rq_deferred);
rqstp->rq_deferred = NULL;
- pagevec_release(&rqstp->rq_pvec);
svc_rqst_release_pages(rqstp);
rqstp->rq_res.page_len = 0;
rqstp->rq_res.page_base = 0;
@@ -667,8 +666,6 @@ static int svc_alloc_arg(struct svc_rqst *rqstp)
struct xdr_buf *arg = &rqstp->rq_arg;
unsigned long pages, filled, ret;
- pagevec_init(&rqstp->rq_pvec);
-
pages = (serv->sv_max_mesg + 2 * PAGE_SIZE) >> PAGE_SHIFT;
if (pages > RPCSVC_MAXPAGES) {
pr_warn_once("svc: warning: pages=%lu > RPCSVC_MAXPAGES=%lu\n",
From: Chuck Lever <[email protected]>
Instead of invoking put_page() one-at-a-time, pass the "response"
portion of rq_pages directly to release_pages() to reduce the number
of times each nfsd thread invokes a page allocator API.
Since svc_xprt_release() is not invoked while a client is waiting
for an RPC Reply, this is not expected to directly impact mean
request latencies on a lightly or moderately loaded server. However
as workload intensity increases, I expect somewhat better
scalability: the same number of server threads should be able to
handle more work.
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/svc.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index 0fc70cc405b2..b982f802f2a0 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -878,13 +878,12 @@ EXPORT_SYMBOL_GPL(svc_rqst_replace_page);
*/
void svc_rqst_release_pages(struct svc_rqst *rqstp)
{
- while (rqstp->rq_next_page != rqstp->rq_respages) {
- struct page **pp = --rqstp->rq_next_page;
+ int i, count = rqstp->rq_next_page - rqstp->rq_respages;
- if (*pp) {
- put_page(*pp);
- *pp = NULL;
- }
+ if (count) {
+ release_pages(rqstp->rq_respages, count);
+ for (i = 0; i < count; i++)
+ rqstp->rq_respages[i] = NULL;
}
}
On 15/04/2023 1:17 am, Chuck Lever wrote:
> I've found a few ways to optimize the release of pages in NFSD.
> Please let me know if I'm abusing the release_pages() and pagevec
> APIs.
>
> ---
>
> Chuck Lever (4):
> SUNRPC: Relocate svc_free_res_pages()
> SUNRPC: Convert svc_xprt_release() to the release_pages() API
> SUNRPC: Convert svc_tcp_restore_pages() to the release_pages() API
> SUNRPC: Be even lazier about releasing pages
>
>
> include/linux/sunrpc/svc.h | 12 +-----------
> net/sunrpc/svc.c | 21 +++++++++++++++++++++
> net/sunrpc/svc_xprt.c | 5 +----
> net/sunrpc/svcsock.c | 12 ++++++------
> 4 files changed, 29 insertions(+), 21 deletions(-)
>
> --
> Chuck Lever
>
Looks good to me, Chuck.
Reviewed-by: Calum Mackay <[email protected]>