On Wed, Aug 8, 2018 at 3:34 PM, jianchao.wang
<jianchao.w.wang@xxxxxxxxxx> wrote:
>
>
> On 08/08/2018 03:11 PM, Ming Lei wrote:
>> On Wed, Aug 8, 2018 at 1:57 PM, jianchao.wang
>> <jianchao.w.wang@xxxxxxxxxx> wrote:
>>> Hi Ming
>>>
>>> On 08/08/2018 01:44 AM, Ming Lei wrote:
>>>>
>>>> +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.
>>
>>> 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.
>>
>>>
>>> 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
