On Mon, Aug 19, 2024 at 10:05:51AM -0700, Kuniyuki Iwashima wrote:
> When commit 390390240145 ("nfsd: don't allow nfsd threads to be
> signalled.") is backported to 5.10, it was adjusted considering commit
> 3feac2b55293 ("sunrpc: exclude from freezer when waiting for requests:").
>
> However, 3feac2b55293 is based on commit f6e70aab9dfe ("SUNRPC: refresh
> rq_pages using a bulk page allocator"), which converted page-by-page
> allocation to a batch allocation, so schedule_timeout() is placed
> un-nested.
>
> As a result, the backported commit 7229200f6866 ("nfsd: don't allow nfsd
> threads to be signalled.") placed freezable_schedule_timeout() in the wrong
> place.
>
> Now, freezable_schedule_timeout() is called after every successful page
> allocation, and we see 30%+ performance regression on 5.10.220 in our
> test suite.
>
> Let's move it to the correct place so that freezable_schedule_timeout()
> is called only when page allocation fails.
>
> Fixes: 7229200f6866 ("nfsd: don't allow nfsd threads to be signalled.")
> Reported-by: Hughdan Liu <hughliu@xxxxxxxxxx>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@xxxxxxxxxx>
> ---
> net/sunrpc/svc_xprt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index d1eacf3358b8..60782504ad3e 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -679,8 +679,8 @@ static int svc_alloc_arg(struct svc_rqst *rqstp)
> set_current_state(TASK_RUNNING);
> return -EINTR;
> }
> + freezable_schedule_timeout(msecs_to_jiffies(500));
> }
> - freezable_schedule_timeout(msecs_to_jiffies(500));
> rqstp->rq_pages[i] = p;
> }
> rqstp->rq_page_end = &rqstp->rq_pages[i];
> --
> 2.30.2
>
Reviewed-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
--
Chuck Lever
