Skip to contentSkip to conversation

FAILED: patch "[PATCH] scsi: scsi_debug: Fix qc_lock use in sdebug_blk_mq_poll()" failed to apply to 5.17-stable tree

PATCH [email protected] 2 participants 2 messages Apr 1, 2022 – Apr 4, 2022

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 5.17-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 3fd07aecb75003fbcb0b7c3124d12f71ffd360d8 Mon Sep 17 00:00:00 2001
From: Damien Le Moal <[email protected]>
Date: Tue, 1 Mar 2022 20:30:09 +0900
Subject: [PATCH] scsi: scsi_debug: Fix qc_lock use in sdebug_blk_mq_poll()

The use of the 'locked' boolean variable to control locking and unlocking of the qc_lock spinlock of struct sdebug_queue confuses sparse, leading to a warning about an unexpected unlock. Simplify the qc_lock lock/unlock handling code of this function to avoid this warning by removing the 'locked' boolean variable. This change also fixes unlocked access to the in_use_bm bitmap with the find_first_bit() function.

Fixes b05d4e481eff ("scsi: scsi_debug: Refine sdebug_blk_mq_poll()")
drivers/scsi/scsi_debug.c +7 -9
index f4e97f2224b2..25fa8e93f5a8 100644
@@ -7509,7 +7509,6 @@ static int sdebug_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num)
 {
 	bool first;
 	bool retiring = false;
-	bool locked = false;
 	int num_entries = 0;
 	unsigned int qc_idx = 0;
 	unsigned long iflags;
@@ -7525,11 +7524,9 @@ static int sdebug_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num)
 	if (qc_idx >= sdebug_max_queue)
 		return 0;
 
+	spin_lock_irqsave(&sqp->qc_lock, iflags);
+
 	for (first = true; first || qc_idx + 1 < sdebug_max_queue; )   {
-		if (!locked) {
-			spin_lock_irqsave(&sqp->qc_lock, iflags);
-			locked = true;
-		}
 		if (first) {
 			first = false;
 			if (!test_bit(qc_idx, sqp->in_use_bm))
@@ -7586,14 +7583,15 @@ static int sdebug_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num)
 		}
 		WRITE_ONCE(sd_dp->defer_t, SDEB_DEFER_NONE);
 		spin_unlock_irqrestore(&sqp->qc_lock, iflags);
-		locked = false;
 		scsi_done(scp); /* callback to mid level */
 		num_entries++;
+		spin_lock_irqsave(&sqp->qc_lock, iflags);
 		if (find_first_bit(sqp->in_use_bm, sdebug_max_queue) >= sdebug_max_queue)
-			break;	/* if no more then exit without retaking spinlock */
+			break;
 	}
-	if (locked)
-		spin_unlock_irqrestore(&sqp->qc_lock, iflags);
+
+	spin_unlock_irqrestore(&sqp->qc_lock, iflags);
+
 	if (num_entries > 0)
 		atomic_add(num_entries, &sdeb_mq_poll_count);
 	return num_entries;
#1 lore raw
Quoted 9 lines · On 4/2/22 00:30, [email protected] wrote:

The patch below does not apply to the 5.17-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

Greg,

b05d4e481eff ("scsi: scsi_debug: Refine sdebug_blk_mq_poll()") which this patch fixes is not in 5.17 kernel, so there is no need for this backport.

Thanks.

Quoted 68 lines

------------------ original commit in Linus's tree ------------------

From 3fd07aecb75003fbcb0b7c3124d12f71ffd360d8 Mon Sep 17 00:00:00 2001
From: Damien Le Moal <[email protected]>
Date: Tue, 1 Mar 2022 20:30:09 +0900
Subject: [PATCH] scsi: scsi_debug: Fix qc_lock use in sdebug_blk_mq_poll()

The use of the 'locked' boolean variable to control locking and unlocking of the qc_lock spinlock of struct sdebug_queue confuses sparse, leading to a warning about an unexpected unlock. Simplify the qc_lock lock/unlock handling code of this function to avoid this warning by removing the 'locked' boolean variable. This change also fixes unlocked access to the in_use_bm bitmap with the find_first_bit() function.

Fixes b05d4e481eff ("scsi: scsi_debug: Refine sdebug_blk_mq_poll()")
drivers/scsi/scsi_debug.c +7 -9
index f4e97f2224b2..25fa8e93f5a8 100644
@@ -7509,7 +7509,6 @@ static int sdebug_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num)
  {
  	bool first;
  	bool retiring = false;
-	bool locked = false;
  	int num_entries = 0;
  	unsigned int qc_idx = 0;
  	unsigned long iflags;
@@ -7525,11 +7524,9 @@ static int sdebug_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num)
  	if (qc_idx >= sdebug_max_queue)
  		return 0;
 
+	spin_lock_irqsave(&sqp->qc_lock, iflags);
+
  	for (first = true; first || qc_idx + 1 < sdebug_max_queue; )   {
-		if (!locked) {
-			spin_lock_irqsave(&sqp->qc_lock, iflags);
-			locked = true;
-		}
  		if (first) {
  			first = false;
  			if (!test_bit(qc_idx, sqp->in_use_bm))
@@ -7586,14 +7583,15 @@ static int sdebug_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num)
  		}
  		WRITE_ONCE(sd_dp->defer_t, SDEB_DEFER_NONE);
  		spin_unlock_irqrestore(&sqp->qc_lock, iflags);
-		locked = false;
  		scsi_done(scp); /* callback to mid level */
  		num_entries++;
+		spin_lock_irqsave(&sqp->qc_lock, iflags);
  		if (find_first_bit(sqp->in_use_bm, sdebug_max_queue) >= sdebug_max_queue)
-			break;	/* if no more then exit without retaking spinlock */
+			break;
  	}
-	if (locked)
-		spin_unlock_irqrestore(&sqp->qc_lock, iflags);
+
+	spin_unlock_irqrestore(&sqp->qc_lock, iflags);
+
  	if (num_entries > 0)
  		atomic_add(num_entries, &sdeb_mq_poll_count);
  	return num_entries;
Signature · 2 lines
Damien Le Moal
Western Digital Research