lkml.org 
[lkml]   [2016]   [Dec]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 6/8] blk-mq-sched: add framework for MQ capable IO schedulers
    From
    Date

    > Il giorno 22 dic 2016, alle ore 10:59, Paolo Valente <paolo.valente@linaro.org> ha scritto:
    >
    >>
    >> Il giorno 17 dic 2016, alle ore 01:12, Jens Axboe <axboe@fb.com> 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@fb.com>
    >>
    > ...
    >
    >> 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@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html

    \
     
     \ /
      Last update: 2016-12-22 12:15    [W:4.071 / U:0.788 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site