Re: [RFC PATCH 10/14] SCSI: use the dedicated admin queue to send admin commands

Ming Lei
2018-08-08T03:36:18Z

On Tue, Aug 07, 2018 at 11:33:15PM +0000, Bart Van Assche wrote:
> On Wed, 2018-08-08 at 01:44 +0800, Ming Lei wrote:
> > --- a/drivers/scsi/scsi_lib.c
> > +++ b/drivers/scsi/scsi_lib.c
> > @@ -278,16 +278,16 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
> > struct request *req;
> > struct scsi_request *rq;
> > int ret = DRIVER_ERROR << 24;
> > + struct request_queue *q = sdev->host->admin_q;
> >
> > - req = blk_get_request(sdev->request_queue,
> > + req = blk_get_request(q,
> > data_direction == DMA_TO_DEVICE ?
> > REQ_OP_SCSI_OUT : REQ_OP_SCSI_IN, BLK_MQ_REQ_PREEMPT);
>
> The above looks weird to me. Why are all RQF_PREEMPT requests sent to the admin
> queue instead of only RQF_PM requests?

The motivation is to use the dedicated admin queue for sending any admin
request, and that is why it isn't named as pm_queue, :-)

Also usually RQF_PREEMPT request is allowed when queue is quiesced, which
can be used in both system suspend and sending domain validation.

>
> > @@ -299,6 +299,8 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
> > req->cmd_flags |= flags;
> > req->rq_flags |= rq_flags | RQF_QUIET;
> >
> > + atomic_inc(&sdev->nr_admin_pending);
>
> Why has a new counter been introduced to keep track of admin requests instead of
> using q_usage_counter?

Just for making sure all admin requests sent to this scsi_device can be
completed before removing this scsi_device, given this patch switches to
use per-host admin queue to send admin requests to all scsi_devices in this host.

So we can't use the q_usage_counter of scsi_device's queue or the per-host
admin queue.

It won't be one big deal since scsi_execute() won't be run in fast path.

Thanks,
Ming