On Wed, Aug 8, 2018 at 3:34 PM, jianchao.wang
[off-list ref] wrote:

On 08/08/2018 03:11 PM, Ming Lei wrote:

quoted

On Wed, Aug 8, 2018 at 1:57 PM, jianchao.wang
[off-list ref] wrote:

quoted

Hi Ming

On 08/08/2018 01:44 AM, Ming Lei wrote:

quoted

+static struct request_queue *scsi_mq_alloc_admin_queue(struct Scsi_Host *shost)
+{
+     struct request_queue *q = __blk_mq_init_queue(&shost->tag_set,
+                     QUEUE_FLAG_MQ_NO_SCHED_DEFAULT);
+     if (IS_ERR(q))
+             return NULL;
+
+     q->mq_ops = &scsi_mq_admin_ops;
+
+     __scsi_init_queue(shost, q);
+
+     return q;
+}

In your patch set, the logical adminq per host is standalone request_queue which share
the tagset with other request_queue.

Due to the hctx_may_queue,
If only one LUN, the adminq will take away half of the driver tags when
the adminq is active.

Most of times, the admin queue is inactive, so it shouldn't be a big deal.

quoted

And when multiple LUNs, all of the LUNs have to share the limited budget of tags
of the adminq.
This is unacceptable.

That can be solved easily, such as, let __blk_mq_tag_busy() not take
account of admin queue, will do it in V2.

quoted

And also, not all the admin request is send out through scsi_execute, maybe SG_IO.
So the admin queue here looks more like the pm queue ?

SG_IO should be covered by IO queue in which SCSI_PASSTHROUGH
is enabled. It is reasonable too since SG_IO can be normal IO from
userspace.

I just concern too much complexity would be introduced by this logical admin queue. ;)

About this concern, I am pretty sure the logic is simple, :-)

If no one objects the admin queue approach.  I plan to rename
the queue flag of NO_SCHED into ADMIN in V2, then we can simply
not increase/decrease the counter of 'active_queues' in case of one
ADMIN queue.

As you saw, the complexity is reduced a lot about implementing
runtime PM and SCSI quiesce with this patch.

Thanks,
Ming Lei