linux-kernel - [PATCH RFC] scsi: core: remove unsed 'restarts' from scsi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 18 Nov 2022 19:30:52 +0800
From:   Yu Kuai <yukuai1@...weicloud.com>
To:     ming.lei@...hat.com, jejb@...ux.ibm.com, martin.petersen@...cle.com
Cc:     linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
        yukuai3@...wei.com, yukuai1@...weicloud.com, yi.zhang@...wei.com
Subject: [PATCH RFC] scsi: core: remove unsed 'restarts' from scsi_device

From: Yu Kuai <yukuai3@...wei.com>

During code review, I found that 'restarts' is not useful anymore after
the following commits:

1) commit ab3cee3762e5 ("blk-mq: In blk_mq_dispatch_rq_list() "no budget"
is a reason to kick")
2) commit d3b38596875d ("blk-mq: run queue no matter whether the request
is the last request")
3) commit 673235f91531 ("scsi: core: Fix race between handling STS_RESOURCE
and completion")

Now that if get budget ever failed, block layer will make sure to
trigger new run queue for the hctx. Hence there is no need to run queue
from scsi layer in this case.

Signed-off-by: Yu Kuai <yukuai3@...wei.com>
---
 drivers/scsi/scsi_lib.c    | 35 -----------------------------------
 include/scsi/scsi_device.h |  1 -
 2 files changed, 36 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 56f641ba1261..f6325a0f80fb 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -507,24 +507,6 @@ static void scsi_run_queue_async(struct scsi_device *sdev)
 	if (scsi_target(sdev)->single_lun ||
 	    !list_empty(&sdev->host->starved_list)) {
 		kblockd_schedule_work(&sdev->requeue_work);
-	} else {
-		/*
-		 * smp_mb() present in sbitmap_queue_clear() or implied in
-		 * .end_io is for ordering writing .device_busy in
-		 * scsi_device_unbusy() and reading sdev->restarts.
-		 */
-		int old = atomic_read(&sdev->restarts);
-
-		/*
-		 * ->restarts has to be kept as non-zero if new budget
-		 *  contention occurs.
-		 *
-		 *  No need to run queue when either another re-run
-		 *  queue wins in updating ->restarts or a new budget
-		 *  contention occurs.
-		 */
-		if (old && atomic_cmpxchg(&sdev->restarts, old, 0) == old)
-			blk_mq_run_hw_queues(sdev->request_queue, true);
 	}
 }
 
@@ -1666,23 +1648,6 @@ static int scsi_mq_get_budget(struct request_queue *q)
 	if (token >= 0)
 		return token;
 
-	atomic_inc(&sdev->restarts);
-
-	/*
-	 * Orders atomic_inc(&sdev->restarts) and atomic_read(&sdev->device_busy).
-	 * .restarts must be incremented before .device_busy is read because the
-	 * code in scsi_run_queue_async() depends on the order of these operations.
-	 */
-	smp_mb__after_atomic();
-
-	/*
-	 * If all in-flight requests originated from this LUN are completed
-	 * before reading .device_busy, sdev->device_busy will be observed as
-	 * zero, then blk_mq_delay_run_hw_queues() will dispatch this request
-	 * soon. Otherwise, completion of one of these requests will observe
-	 * the .restarts flag, and the request queue will be run for handling
-	 * this request, see scsi_end_request().
-	 */
 	if (unlikely(scsi_device_busy(sdev) == 0 &&
 				!scsi_device_blocked(sdev)))
 		blk_mq_delay_run_hw_queues(sdev->request_queue, SCSI_QUEUE_DELAY);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 24bdbf7999ab..66345de80897 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -115,7 +115,6 @@ struct scsi_device {
 	struct sbitmap budget_map;
 	atomic_t device_blocked;	/* Device returned QUEUE_FULL. */
 
-	atomic_t restarts;
 	spinlock_t list_lock;
 	struct list_head starved_entry;
 	unsigned short queue_depth;	/* How deep of a queue we want */
-- 
2.31.1