> Il giorno 22 dic 2016, alle ore 10:59, Paolo Valente <paolo.valente@xxxxxxxxxx> ha scritto:
>
>>
>> Il giorno 17 dic 2016, alle ore 01:12, Jens Axboe <axboe@xxxxxx> ha scritto:
>>
>> This adds a set of hooks that intercepts the blk-mq path of
>> allocating/inserting/issuing/completing requests, allowing
>> us to develop a scheduler within that framework.
>>
>> We reuse the existing elevator scheduler API on the registration
>> side, but augment that with the scheduler flagging support for
>> the blk-mq interfce, and with a separate set of ops hooks for MQ
>> devices.
>>
>> Schedulers can opt in to using shadow requests. Shadow requests
>> are internal requests that the scheduler uses for for the allocate
>> and insert part, which are then mapped to a real driver request
>> at dispatch time. This is needed to separate the device queue depth
>> from the pool of requests that the scheduler has to work with.
>>
>> Signed-off-by: Jens Axboe <axboe@xxxxxx>
>>
> ...
>
>> diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
>> new file mode 100644
>> index 000000000000..b7e1839d4785
>> --- /dev/null
>> +++ b/block/blk-mq-sched.c
>
>> ...
>> +static inline bool
>> +blk_mq_sched_allow_merge(struct request_queue *q, struct request *rq,
>> + struct bio *bio)
>> +{
>> + struct elevator_queue *e = q->elevator;
>> +
>> + if (e && e->type->ops.mq.allow_merge)
>> + return e->type->ops.mq.allow_merge(q, rq, bio);
>> +
>> + return true;
>> +}
>> +
>
> Something does not seem to add up here:
> e->type->ops.mq.allow_merge may be called only in
> blk_mq_sched_allow_merge, which, in its turn, may be called only in
> blk_mq_attempt_merge, which, finally, may be called only in
> blk_mq_merge_queue_io. Yet the latter may be called only if there is
> no elevator (line 1399 and 1507 in blk-mq.c).
>
> Therefore, e->type->ops.mq.allow_merge can never be called, both if
> there is and if there is not an elevator. Be patient if I'm missing
> something huge, but I thought it was worth reporting this.
>
Just another detail: if e->type->ops.mq.allow_merge does get invoked
from the above path, then it is invoked of course without the
scheduler lock held. In contrast, if this function gets invoked
from dd_bio_merge, then the scheduler lock is held.
To handle this opposite alternatives, I don't know whether checking if
the lock is held (and possibly taking it) from inside
e->type->ops.mq.allow_merge is a good solution. In any case, before
possibly trying it, I will wait for some feedback on the main problem,
i.e., on the fact that e->type->ops.mq.allow_merge
seems unreachable in the above path.
Thanks,
Paolo
> Paolo
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-block" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
