On Mon, Aug 19, 2024 at 02:11:04PM -0400, Chuck Lever wrote:

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@amazon.com Signed-off-by: Kuniyuki Iwashima kuniyu@amazon.com

---

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@oracle.com

Now queued up, thanks.

greg k-h