On 7/27/20 3:21 PM, Keith Busch wrote:
> On Mon, Jul 27, 2020 at 03:05:40PM -0600, Jens Axboe wrote:
>> +void blk_mq_quiesce_queue_wait(struct request_queue *q)
>> {
>> struct blk_mq_hw_ctx *hctx;
>> unsigned int i;
>> bool rcu = false;
>>
>> - blk_mq_quiesce_queue_nowait(q);
>> -
>> queue_for_each_hw_ctx(q, hctx, i) {
>> if (hctx->flags & BLK_MQ_F_BLOCKING)
>> synchronize_srcu(hctx->srcu);
>> else
>> rcu = true;
>> }
>> +
>> if (rcu)
>> synchronize_rcu();
>> }
>
> Either all the hctx's are blocking or none of them are: we don't need to
> iterate the hctx's to see which sync method to use. We can add at the
> very beginning (and get rid of 'bool rcu'):
>
> if (!(q->tag_set->flags & BLK_MQ_F_BLOCKING)) {
> synchronize_rcu();
> return;
> }
Agree, was just copy/pasting the existing code.
> But the issue Sagi is trying to address is quiescing a lot
> request queues sharing a tagset where synchronize_rcu() is too time
> consuming to do repeatedly. He wants to synchrnoize once for the entire
> tagset rather than per-request_queue, so I think he needs an API taking
> a 'struct blk_mq_tag_set' instead of a 'struct request_queue'.
Gotcha, yeah that won't work for multiple queues obviously.
Are all these queues sharing a tag set? If so, yes that seems like the
right abstraction. And the pointer addition is a much better idea than
including a full srcu/rcu struct.
--
Jens Axboe
