diff mbox

[v5,7/8] scsi: Set QUEUE_FLAG_PREEMPT_ONLY while quiesced

Message ID 20171002225218.18548-8-bart.vanassche@wdc.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bart Van Assche Oct. 2, 2017, 10:52 p.m. UTC 
Make the quiesce state visible to the block layer for the next
patch in this series.

Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/scsi/scsi_lib.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Christoph Hellwig Oct. 4, 2017, 7:06 a.m. UTC | #1 
> +	/*
> +	 * Ensure that the effect of blk_set_preempt_only() is globally
> +	 * visible before unfreezing the queue.
> +	 */
> +	if (err == 0)
> +		synchronize_rcu();

I don't understand why we'd need this.  The flag is set both under
a spinlock and a mutex that are unlocked right after.  That should
gurantee all the visibility we'll need.
Bart Van Assche Oct. 4, 2017, 3:44 p.m. UTC | #2 
On Wed, 2017-10-04 at 09:06 +0200, Christoph Hellwig wrote:
> > +	/*

> > +	 * Ensure that the effect of blk_set_preempt_only() is globally

> > +	 * visible before unfreezing the queue.

> > +	 */

> > +	if (err == 0)

> > +		synchronize_rcu();

> 

> I don't understand why we'd need this.  The flag is set both under

> a spinlock and a mutex that are unlocked right after.  That should

> gurantee all the visibility we'll need.


Hello Christoph,

That synchronize_rcu() call follows the example in the third figure in
https://lwn.net/Articles/573497/. I will verify whether that synchronize_rcu()
call can be left out.

Bart.
diff mbox

Patch

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index e0bef5c9dd14..534c81de0880 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2940,8 +2940,16 @@  scsi_device_quiesce(struct scsi_device *sdev)
 	if (freeze)
 		blk_mq_freeze_queue(q);
 	err = scsi_device_set_state(sdev, SDEV_QUIESCE);
+	if (err == 0)
+		blk_set_preempt_only(q);
 	mutex_unlock(&sdev->state_mutex);
 
+	/*
+	 * Ensure that the effect of blk_set_preempt_only() is globally
+	 * visible before unfreezing the queue.
+	 */
+	if (err == 0)
+		synchronize_rcu();
 	if (freeze)
 		blk_mq_unfreeze_queue(q);
 
@@ -2966,8 +2974,10 @@  void scsi_device_resume(struct scsi_device *sdev)
 	 */
 	mutex_lock(&sdev->state_mutex);
 	if (sdev->sdev_state == SDEV_QUIESCE &&
-	    scsi_device_set_state(sdev, SDEV_RUNNING) == 0)
+	    scsi_device_set_state(sdev, SDEV_RUNNING) == 0) {
+		blk_clear_preempt_only(sdev->request_queue);
 		scsi_run_queue(sdev->request_queue);
+	}
 	mutex_unlock(&sdev->state_mutex);
 }
 EXPORT_SYMBOL(scsi_device_resume);