[PATCH] scsi_transport_fc: Hold queue lock while calling blk_run_queue_async()

[PATCH] scsi_transport_fc: Hold queue lock while calling blk_run_queue_async()

[Bart Van Assche]

It is required to hold the queue lock when calling blk_run_queue_async()
to avoid that a race between blk_run_queue_async() and
blk_cleanup_queue() is triggered. Additionally, remove the get_device()
and put_device() calls from fc_bsg_goose_queue. It is namely the
responsibility of the caller of fc_bsg_goose_queue() to ensure that
the bsg queue does not disappear while fc_bsg_goose_queue() is in
progress.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: James Smart <james.smart@avagotech.com>
---
 drivers/scsi/scsi_transport_fc.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 0f3a386..a32ab8e 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -3855,15 +3855,15 @@ fc_bsg_host_dispatch(struct request_queue *q, struct Scsi_Host *shost,
 static void
 fc_bsg_goose_queue(struct fc_rport *rport)
 {
-	if (!rport->rqst_q)
+	struct request_queue *q = rport->rqst_q;
+	unsigned long flags;
+
+	if (!q)
 		return;
 
-	/*
-	 * This get/put dance makes no sense
-	 */
-	get_device(&rport->dev);
-	blk_run_queue_async(rport->rqst_q);
-	put_device(&rport->dev);
+	spin_lock_irqsave(q->queue_lock, flags);
+	blk_run_queue_async(q);
+	spin_unlock_irqrestore(q->queue_lock, flags);
 }
 
 /**
-- 
2.10.1

Re: [PATCH] scsi_transport_fc: Hold queue lock while calling blk_run_queue_async()

[James Smart]

Reviewed-By: James Smart <james.smart@broadcom.com>

(Bart: fyi - note my change in email address)

-- james


On 11/11/2016 4:55 PM, Bart Van Assche wrote:
> It is required to hold the queue lock when calling blk_run_queue_async()
> to avoid that a race between blk_run_queue_async() and
> blk_cleanup_queue() is triggered. Additionally, remove the get_device()
> and put_device() calls from fc_bsg_goose_queue. It is namely the
> responsibility of the caller of fc_bsg_goose_queue() to ensure that
> the bsg queue does not disappear while fc_bsg_goose_queue() is in
> progress.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: James Smart <james.smart@avagotech.com>
> ---
>   drivers/scsi/scsi_transport_fc.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
> index 0f3a386..a32ab8e 100644
> --- a/drivers/scsi/scsi_transport_fc.c
> +++ b/drivers/scsi/scsi_transport_fc.c
> @@ -3855,15 +3855,15 @@ fc_bsg_host_dispatch(struct request_queue *q, struct Scsi_Host *shost,
>   static void
>   fc_bsg_goose_queue(struct fc_rport *rport)
>   {
> -	if (!rport->rqst_q)
> +	struct request_queue *q = rport->rqst_q;
> +	unsigned long flags;
> +
> +	if (!q)
>   		return;
>   
> -	/*
> -	 * This get/put dance makes no sense
> -	 */
> -	get_device(&rport->dev);
> -	blk_run_queue_async(rport->rqst_q);
> -	put_device(&rport->dev);
> +	spin_lock_irqsave(q->queue_lock, flags);
> +	blk_run_queue_async(q);
> +	spin_unlock_irqrestore(q->queue_lock, flags);
>   }
>   
>   /**

Re: [PATCH] scsi_transport_fc: Hold queue lock while calling blk_run_queue_async()

[Martin K. Petersen]

>>>>> "Bart" == Bart Van Assche <bart.vanassche@sandisk.com> writes:

Bart> It is required to hold the queue lock when calling
Bart> blk_run_queue_async() to avoid that a race between
Bart> blk_run_queue_async() and blk_cleanup_queue() is
Bart> triggered. Additionally, remove the get_device() and put_device()
Bart> calls from fc_bsg_goose_queue. It is namely the responsibility of
Bart> the caller of fc_bsg_goose_queue() to ensure that the bsg queue
Bart> does not disappear while fc_bsg_goose_queue() is in progress.

Applied to 4.10/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering