Message ID | 1255432719-19615-2-git-send-email-apw@canonical.com |
---|---|
State | Accepted |
Commit | 6830be77b871fa5220a52f733f644de5b2b1a670 |
Headers | show |
Having spent quite some time listening to that problem, I believe it is sane. ;-) Andy Whitcroft wrote: > BugLink: http://bugs.launchpad.net/bugs/444915 > > When a readahead request is injected into the elevator we construct a > bio as below, treating it as fastfail: > > void init_request_from_bio(struct request *req, struct bio *bio) > [...] > if (bio_rw_ahead(bio)) > req->cmd_flags |= (REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT | > REQ_FAILFAST_DRIVER); > [...] > > However when merging requests in the elevator we only allow merges of > new bios with matching fastfail attributes: > > int elv_rq_merge_ok(struct request *rq, struct bio *bio) > [...] > if (!bio_failfast_dev(bio) != !blk_failfast_dev(rq) || > !bio_failfast_transport(bio) != !blk_failfast_transport(rq) || > !bio_failfast_driver(bio) != !blk_failfast_driver(rq)) > [...] > > This check occurs against the original bio fastfail bits ignoring the > override and thus preventing merge of an existing readahead request with > one in the queue. > > Modify the merge check to take account of the fact that all readahead > bios will be treated as fastfail. > > Signed-off-by: Andy Whitcroft <apw@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > block/elevator.c | 14 ++++++++++---- > 1 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/block/elevator.c b/block/elevator.c > index 2d511f9..0353d80 100644 > --- a/block/elevator.c > +++ b/block/elevator.c > @@ -101,16 +101,22 @@ int elv_rq_merge_ok(struct request *rq, struct bio *bio) > return 0; > > /* > - * Don't merge if failfast settings don't match. > + * Don't merge if failfast settings don't match. Note readahead > + * requests are always mapped to failfast. > * > * FIXME: The negation in front of each condition is necessary > * because bio and request flags use different bit positions > * and the accessors return those bits directly. This > * ugliness will soon go away. > */ > - if (!bio_failfast_dev(bio) != !blk_failfast_dev(rq) || > - !bio_failfast_transport(bio) != !blk_failfast_transport(rq) || > - !bio_failfast_driver(bio) != !blk_failfast_driver(rq)) > + if (!(bio_rw_ahead(bio) || bio_failfast_dev(bio)) != > + !blk_failfast_dev(rq)) > + return 0; > + if (!(bio_rw_ahead(bio) || bio_failfast_transport(bio)) != > + !blk_failfast_transport(rq)) > + return 0; > + if (!(bio_rw_ahead(bio) || bio_failfast_driver(bio)) != > + !blk_failfast_driver(rq)) > return 0; > > if (!elv_iosched_allow_merge(rq, bio))
diff --git a/block/elevator.c b/block/elevator.c index 2d511f9..0353d80 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -101,16 +101,22 @@ int elv_rq_merge_ok(struct request *rq, struct bio *bio) return 0; /* - * Don't merge if failfast settings don't match. + * Don't merge if failfast settings don't match. Note readahead + * requests are always mapped to failfast. * * FIXME: The negation in front of each condition is necessary * because bio and request flags use different bit positions * and the accessors return those bits directly. This * ugliness will soon go away. */ - if (!bio_failfast_dev(bio) != !blk_failfast_dev(rq) || - !bio_failfast_transport(bio) != !blk_failfast_transport(rq) || - !bio_failfast_driver(bio) != !blk_failfast_driver(rq)) + if (!(bio_rw_ahead(bio) || bio_failfast_dev(bio)) != + !blk_failfast_dev(rq)) + return 0; + if (!(bio_rw_ahead(bio) || bio_failfast_transport(bio)) != + !blk_failfast_transport(rq)) + return 0; + if (!(bio_rw_ahead(bio) || bio_failfast_driver(bio)) != + !blk_failfast_driver(rq)) return 0; if (!elv_iosched_allow_merge(rq, bio))
BugLink: http://bugs.launchpad.net/bugs/444915 When a readahead request is injected into the elevator we construct a bio as below, treating it as fastfail: void init_request_from_bio(struct request *req, struct bio *bio) [...] if (bio_rw_ahead(bio)) req->cmd_flags |= (REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT | REQ_FAILFAST_DRIVER); [...] However when merging requests in the elevator we only allow merges of new bios with matching fastfail attributes: int elv_rq_merge_ok(struct request *rq, struct bio *bio) [...] if (!bio_failfast_dev(bio) != !blk_failfast_dev(rq) || !bio_failfast_transport(bio) != !blk_failfast_transport(rq) || !bio_failfast_driver(bio) != !blk_failfast_driver(rq)) [...] This check occurs against the original bio fastfail bits ignoring the override and thus preventing merge of an existing readahead request with one in the queue. Modify the merge check to take account of the fact that all readahead bios will be treated as fastfail. Signed-off-by: Andy Whitcroft <apw@canonical.com> --- block/elevator.c | 14 ++++++++++---- 1 files changed, 10 insertions(+), 4 deletions(-)