public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.de>
To: Chuck Lever <chuck.lever@oracle.com>, Jeff Layton <jlayton@kernel.org>
Cc: linux-nfs@vger.kernel.org
Subject: [PATCH 06/12] SUNRPC: rename and refactor svc_get_next_xprt().
Date: Mon, 31 Jul 2023 16:48:33 +1000	[thread overview]
Message-ID: <20230731064839.7729-7-neilb@suse.de> (raw)
In-Reply-To: <20230731064839.7729-1-neilb@suse.de>

svc_get_next_xprt() does a lot more than just get an xprt.  It also
decides if it needs to sleep, depending not only on the availability of
xprts, but also on the need to exit or handle external work
(SP_TASK_PENDING).

So rename it to svc_rqst_wait_and_dequeue_work(), don't return the xprt
(which can easily be found in rqstp->rq_xprt), and restructure to make a
clear separation between waiting and dequeueing.

All the scheduling-related code like try_to_freeze() and
kthread_should_stop() is moved into svc_rqst_wait_and_dequeue_work().

Rather than calling svc_xprt_dequeue() twice (before and after deciding
to wait), it now calls rqst_should_sleep() twice.  If the first fails,
we skip all the waiting code completely.  In the waiting code we call
again after setting the task state in case we missed a wake-up.

We now only have one call to try_to_freeze() and one call to
svc_xprt_dequeue().  We still have two calls to kthread_should_stop() -
one in rqst_should_sleep() to avoid sleeping, and one afterwards to
avoid dequeueing any work (it previously came after dequeueing which
doesn't seem right).

Signed-off-by: NeilBrown <neilb@suse.de>
---
 net/sunrpc/svc_xprt.c | 62 +++++++++++++++++++++----------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 380fb3caea4c..67f2b34cb8e4 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -722,47 +722,51 @@ rqst_should_sleep(struct svc_rqst *rqstp)
 	return true;
 }
 
-static struct svc_xprt *svc_get_next_xprt(struct svc_rqst *rqstp)
+static void svc_rqst_wait_and_dequeue_work(struct svc_rqst *rqstp)
 {
 	struct svc_pool		*pool = rqstp->rq_pool;
+	bool slept = false;
 
 	/* rq_xprt should be clear on entry */
 	WARN_ON_ONCE(rqstp->rq_xprt);
 
-	rqstp->rq_xprt = svc_xprt_dequeue(pool);
-	if (rqstp->rq_xprt) {
-		trace_svc_pool_polled(rqstp);
-		goto out_found;
+	if (rqst_should_sleep(rqstp)) {
+		set_current_state(TASK_IDLE);
+		smp_mb__before_atomic();
+		clear_bit(SP_CONGESTED, &pool->sp_flags);
+		clear_bit(RQ_BUSY, &rqstp->rq_flags);
+		smp_mb__after_atomic();
+
+		/* Need to test again after setting task state */
+		if (likely(rqst_should_sleep(rqstp))) {
+			schedule();
+			slept = true;
+		} else {
+			__set_current_state(TASK_RUNNING);
+			cond_resched();
+		}
+		set_bit(RQ_BUSY, &rqstp->rq_flags);
+		smp_mb__after_atomic();
 	}
-
-	set_current_state(TASK_IDLE);
-	smp_mb__before_atomic();
-	clear_bit(SP_CONGESTED, &pool->sp_flags);
-	clear_bit(RQ_BUSY, &rqstp->rq_flags);
-	smp_mb__after_atomic();
-
-	if (likely(rqst_should_sleep(rqstp)))
-		schedule();
-	else
-		__set_current_state(TASK_RUNNING);
-
 	try_to_freeze();
 
-	set_bit(RQ_BUSY, &rqstp->rq_flags);
-	smp_mb__after_atomic();
+	if (kthread_should_stop())
+		return;
+
 	clear_bit(SP_TASK_PENDING, &pool->sp_flags);
 	rqstp->rq_xprt = svc_xprt_dequeue(pool);
 	if (rqstp->rq_xprt) {
-		trace_svc_pool_awoken(rqstp);
+		if (slept)
+			trace_svc_pool_awoken(rqstp);
+		else
+			trace_svc_pool_polled(rqstp);
 		goto out_found;
 	}
 
-	if (kthread_should_stop())
-		return NULL;
-	percpu_counter_inc(&pool->sp_threads_no_work);
-	return NULL;
+	if (slept)
+		percpu_counter_inc(&pool->sp_threads_no_work);
+	return;
 out_found:
-	clear_bit(SP_TASK_PENDING, &pool->sp_flags);
 	/* Normally we will wait up to 5 seconds for any required
 	 * cache information to be provided.
 	 */
@@ -770,7 +774,6 @@ static struct svc_xprt *svc_get_next_xprt(struct svc_rqst *rqstp)
 		rqstp->rq_chandle.thread_wait = 5*HZ;
 	else
 		rqstp->rq_chandle.thread_wait = 1*HZ;
-	return rqstp->rq_xprt;
 }
 
 static void svc_add_new_temp_xprt(struct svc_serv *serv, struct svc_xprt *newxpt)
@@ -854,12 +857,9 @@ void svc_recv(struct svc_rqst *rqstp)
 	if (!svc_alloc_arg(rqstp))
 		goto out;
 
-	try_to_freeze();
-	cond_resched();
-	if (kthread_should_stop())
-		goto out;
+	svc_rqst_wait_and_dequeue_work(rqstp);
 
-	xprt = svc_get_next_xprt(rqstp);
+	xprt = rqstp->rq_xprt;
 	if (!xprt)
 		goto out;
 
-- 
2.40.1


  parent reply	other threads:[~2023-07-31  6:52 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-31  6:48 [PATCH 00/12] SUNRPC: various thread management improvements NeilBrown
2023-07-31  6:48 ` [PATCH 01/12] SUNRPC: make rqst_should_sleep() idempotent() NeilBrown
2023-07-31 14:21   ` Chuck Lever
2023-07-31 22:05     ` NeilBrown
2023-07-31 22:31       ` Chuck Lever III
2023-07-31 14:33   ` Jeff Layton
2023-07-31  6:48 ` [PATCH 02/12] FIXUP: SUNRPC: Deduplicate thread wake-up code NeilBrown
2023-07-31  6:48 ` [PATCH 03/12] FIXUP: SUNRPC: call svc_process() from svc_recv() NeilBrown
2023-07-31 14:22   ` Chuck Lever
2023-07-31  6:48 ` [PATCH 04/12] nfsd: Simplify code around svc_exit_thread() call in nfsd() NeilBrown
2023-07-31  6:48 ` [PATCH 05/12] nfsd: separate nfsd_last_thread() from nfsd_put() NeilBrown
2023-07-31 14:23   ` Chuck Lever
2023-07-31  6:48 ` NeilBrown [this message]
2023-07-31 23:16   ` [PATCH 06/12] SUNRPC: rename and refactor svc_get_next_xprt() Chuck Lever
2023-08-01 22:46     ` Chuck Lever
2023-08-02  5:00     ` NeilBrown
2023-07-31  6:48 ` [PATCH 07/12] SUNRPC: move all of xprt handling into svc_xprt_handle() NeilBrown
2023-07-31  6:48 ` [PATCH 08/12] SUNRPC: move task-dequeueing code into svc_recv() NeilBrown
2023-07-31  6:48 ` [PATCH 09/12] SUNRPC: integrate back-channel processing with svc_recv() NeilBrown
2023-07-31  6:48 ` [PATCH 10/12] SUNRPC: change how svc threads are asked to exit NeilBrown
2023-07-31  6:48 ` [PATCH 11/12] SUNRPC: add list of idle threads NeilBrown
2023-07-31  6:48 ` [PATCH 12/12] SUNRPC: discard SP_CONGESTED NeilBrown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230731064839.7729-7-neilb@suse.de \
    --to=neilb@suse.de \
    --cc=chuck.lever@oracle.com \
    --cc=jlayton@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox