Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: [PATCH 07/14] SUNRPC: refactor svc_recv()
From:   NeilBrown <neilb@suse.de>
To:     Chuck Lever <chuck.lever@oracle.com>,
        Jeff Layton <jlayton@kernel.org>
Cc:     linux-nfs@vger.kernel.org
Date:   Tue, 18 Jul 2023 16:38:08 +1000
Message-ID: <168966228864.11075.17318657609206358910.stgit@noble.brown>
In-Reply-To: <168966227838.11075.2974227708495338626.stgit@noble.brown>
References: <168966227838.11075.2974227708495338626.stgit@noble.brown>
User-Agent: StGit/1.5
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk

svc_get_next_xprt() does a lot more than get an xprt.  It mostly waits.

So rename to svc_wait_for_work() and don't bother returning a value.
The xprt can be found in ->rq_xprt.

Also move all the code to handle ->rq_xprt into a single if branch, so
that other handlers can be added there if other work is found.

Remove the call to svc_xprt_dequeue() that is before we set TASK_IDLE.
If there is still something to dequeue will still get it after a few
more checks - no sleeping.  This was an unnecessary optimisation which
muddles the code.

Drop a call to kthread_should_stop().  There are enough of those in
svc_wait_for_work().

(This patch is best viewed with "-b")

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

diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 44a33b1f542f..c7095ff7d5fd 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -735,19 +735,10 @@ rqst_should_sleep(struct svc_rqst *rqstp)
 	return true;
 }
 
-static struct svc_xprt *svc_get_next_xprt(struct svc_rqst *rqstp)
+static void svc_wait_for_work(struct svc_rqst *rqstp)
 {
 	struct svc_pool		*pool = rqstp->rq_pool;
 
-	/* 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;
-	}
-
 	set_current_state(TASK_IDLE);
 	smp_mb__before_atomic();
 	clear_bit(SP_CONGESTED, &pool->sp_flags);
@@ -769,10 +760,9 @@ static struct svc_xprt *svc_get_next_xprt(struct svc_rqst *rqstp)
 		goto out_found;
 	}
 
-	if (kthread_should_stop())
-		return NULL;
-	percpu_counter_inc(&pool->sp_threads_no_work);
-	return NULL;
+	if (!kthread_should_stop())
+		percpu_counter_inc(&pool->sp_threads_no_work);
+	return;
 out_found:
 	/* Normally we will wait up to 5 seconds for any required
 	 * cache information to be provided.
@@ -781,7 +771,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)
@@ -855,45 +844,40 @@ static int svc_handle_xprt(struct svc_rqst *rqstp, struct svc_xprt *xprt)
  */
 void svc_recv(struct svc_rqst *rqstp)
 {
-	struct svc_xprt		*xprt = NULL;
-	struct svc_serv		*serv = rqstp->rq_server;
-	int			len;
-
 	if (!svc_alloc_arg(rqstp))
-		goto out;
+		return;
 
 	try_to_freeze();
 	cond_resched();
-	if (kthread_should_stop())
-		goto out;
 
-	xprt = svc_get_next_xprt(rqstp);
-	if (!xprt)
-		goto out;
+	svc_wait_for_work(rqstp);
 
-	len = svc_handle_xprt(rqstp, xprt);
+	if (rqstp->rq_xprt) {
+		struct svc_serv	*serv = rqstp->rq_server;
+		struct svc_xprt *xprt = rqstp->rq_xprt;
+		int len;
 
-	/* No data, incomplete (TCP) read, or accept() */
-	if (len <= 0)
-		goto out_release;
+		len = svc_handle_xprt(rqstp, xprt);
 
-	trace_svc_xdr_recvfrom(&rqstp->rq_arg);
+		/* No data, incomplete (TCP) read, or accept() */
+		if (len <= 0) {
+			rqstp->rq_res.len = 0;
+			svc_xprt_release(rqstp);
+		} else {
 
-	clear_bit(XPT_OLD, &xprt->xpt_flags);
+			trace_svc_xdr_recvfrom(&rqstp->rq_arg);
 
-	rqstp->rq_chandle.defer = svc_defer;
+			clear_bit(XPT_OLD, &xprt->xpt_flags);
 
-	if (serv->sv_stats)
-		serv->sv_stats->netcnt++;
-	percpu_counter_inc(&rqstp->rq_pool->sp_messages_arrived);
-	rqstp->rq_stime = ktime_get();
-	svc_process(rqstp);
-	return;
-out_release:
-	rqstp->rq_res.len = 0;
-	svc_xprt_release(rqstp);
-out:
-	return;
+			rqstp->rq_chandle.defer = svc_defer;
+
+			if (serv->sv_stats)
+				serv->sv_stats->netcnt++;
+			percpu_counter_inc(&rqstp->rq_pool->sp_messages_arrived);
+			rqstp->rq_stime = ktime_get();
+			svc_process(rqstp);
+		}
+	}
 }
 EXPORT_SYMBOL_GPL(svc_recv);