On Fri, Aug 25, 2017 at 10:23:09PM +0000, Bart Van Assche wrote:
> On Thu, 2017-08-24 at 14:52 +0800, Ming Lei wrote:
> > On Tue, Aug 22, 2017 at 09:55:57PM +0000, Bart Van Assche wrote:
> > > On Sat, 2017-08-05 at 14:56 +0800, Ming Lei wrote:
> > > > + /*
> > > > + * if there is q->queue_depth, all hw queues share
> > > > + * this queue depth limit
> > > > + */
> > > > + if (q->queue_depth) {
> > > > + queue_for_each_hw_ctx(q, hctx, i)
> > > > + hctx->flags |= BLK_MQ_F_SHARED_DEPTH;
> > > > + }
> > > > +
> > > > + if (!q->elevator)
> > > > + goto exit;
> > >
> > > Hello Ming,
> > >
> > > It seems very fragile to me to set BLK_MQ_F_SHARED_DEPTH if and only if
> > > q->queue_depth != 0. Wouldn't it be better to let the block driver tell the
> > > block layer whether or not there is a queue depth limit across hardware
> > > queues, e.g. through a tag set flag?
> >
> > One reason for not doing in that way is because q->queue_depth can be
> > changed via sysfs interface.
>
> Only the SCSI core allows the queue depth to be changed through sysfs. The
> other block drivers I am familiar with set the queue depth when the block
> layer queue is created and do not allow the queue depth to be changed later
> on.
Actually SCSI core is the only user of q->queue_depth, and it supports
to change it via sysfs.
>
> > Another reason is that better to not exposing this flag to drivers since
> > it isn't necessary, that said it is an internal flag actually.
>
> As far as I know only the SCSI core can create request queues that have a
> queue depth that is lower than the number of tags in the tag set. So for all
> block drivers except the SCSI core it is OK to dispatch all requests at once
> to which a hardware tag has been assigned. My proposal is either to let
> drivers like the SCSI core set BLK_MQ_F_SHARED_DEPTH or to modify
> blk_mq_sched_dispatch_requests() such that it flushes all requests if the
> request queue depth is not lower than the hardware tag set size instead of if
> q->queue_depth == 0.
As I mentioned, SCSI core is the only user of q->queue_depth, so no
affect on other drivers.
--
Ming
