Re: [PATCH V4 7/7] SCSI: don't hold device refcount in IO path

[Bart Van Assche <bvanassche@acm.org>]

On Thu, 2019-04-04 at 16:43 +0800, Ming Lei wrote:
> scsi_device's refcount is always grabbed in IO path.
> 
> Turns out it isn't necessary, because blk_queue_cleanup() will
> drain any in-flight IOs, then cancel timeout/requeue work, and
> SCSI's requeue_work is canceled too in __scsi_remove_device().
> 
> Also scsi_device won't go away until blk_cleanup_queue() is done.
> 
> So don't hold the refcount in IO path, especially the refcount isn't
> required in IO path since blk_queue_enter() / blk_queue_exit()
> is introduced in the legacy block layer.
> 
> Cc: Dongli Zhang <dongli.zhang@oracle.com>
> Cc: James Smart <james.smart@broadcom.com>
> Cc: Bart Van Assche <bart.vanassche@wdc.com>
> Cc: linux-scsi@vger.kernel.org,
> Cc: Martin K . Petersen <martin.petersen@oracle.com>,
> Cc: Christoph Hellwig <hch@lst.de>,
> Cc: James E . J . Bottomley <jejb@linux.vnet.ibm.com>,
> Cc: jianchao wang <jianchao.w.wang@oracle.com>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  drivers/scsi/scsi_lib.c | 28 ++--------------------------
>  1 file changed, 2 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 601b9f1de267..3369d66911eb 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -141,8 +141,6 @@ scsi_set_blocked(struct scsi_cmnd *cmd, int reason)
>  
>  static void scsi_mq_requeue_cmd(struct scsi_cmnd *cmd)
>  {
> -       struct scsi_device *sdev = cmd->device;
> -
>         if (cmd->request->rq_flags & RQF_DONTPREP) {
>                 cmd->request->rq_flags &= ~RQF_DONTPREP;
>                 scsi_mq_uninit_cmd(cmd);
> @@ -150,7 +148,6 @@ static void scsi_mq_requeue_cmd(struct scsi_cmnd *cmd)
>                 WARN_ON_ONCE(true);
>         }
>         blk_mq_requeue_request(cmd->request, true);
> -       put_device(&sdev->sdev_gendev);
>  }
>  
>  /**
> @@ -189,19 +186,7 @@ static void __scsi_queue_insert(struct scsi_cmnd *cmd, int reason, bool unbusy)
>          */
>         cmd->result = 0;
>  
> -       /*
> -        * Before a SCSI command is dispatched,
> -        * get_device(&sdev->sdev_gendev) is called and the host,
> -        * target and device busy counters are increased. Since
> -        * requeuing a request causes these actions to be repeated and
> -        * since scsi_device_unbusy() has already been called,
> -        * put_device(&device->sdev_gendev) must still be called. Call
> -        * put_device() after blk_mq_requeue_request() to avoid that
> -        * removal of the SCSI device can start before requeueing has
> -        * happened.
> -        */
>         blk_mq_requeue_request(cmd->request, true);
> -       put_device(&device->sdev_gendev);
>  }
>  
>  /*
> @@ -619,7 +604,6 @@ static bool scsi_end_request(struct request *req, blk_status_t error,
>                 blk_mq_run_hw_queues(q, true);
>  
>         percpu_ref_put(&q->q_usage_counter);
> -       put_device(&sdev->sdev_gendev);
>         return false;
>  }
>  
> @@ -1613,7 +1597,6 @@ static void scsi_mq_put_budget(struct blk_mq_hw_ctx *hctx)
>         struct scsi_device *sdev = q->queuedata;
>  
>         atomic_dec(&sdev->device_busy);
> -       put_device(&sdev->sdev_gendev);
>  }
>  
>  static bool scsi_mq_get_budget(struct blk_mq_hw_ctx *hctx)
> @@ -1621,16 +1604,9 @@ static bool scsi_mq_get_budget(struct blk_mq_hw_ctx *hctx)
>         struct request_queue *q = hctx->queue;
>         struct scsi_device *sdev = q->queuedata;
>  
> -       if (!get_device(&sdev->sdev_gendev))
> -               goto out;
> -       if (!scsi_dev_queue_ready(q, sdev))
> -               goto out_put_device;
> -
> -       return true;
> +       if (scsi_dev_queue_ready(q, sdev))
> +               return true;
>  
> -out_put_device:
> -       put_device(&sdev->sdev_gendev);
> -out:
>         if (atomic_read(&sdev->device_busy) == 0 && !scsi_device_blocked(sdev))
>                 blk_mq_delay_run_hw_queue(hctx, SCSI_QUEUE_DELAY);
>         return false;

Reviewed-by: Bart Van Assche <bvanassche@acm.org>