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. thanks, Ming Lei