[PATCH 1/1] UBUNTU: [Upstream] elevator: fix fastfail checks to allow merge of readahead requests

[Stefan Bader]

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 at canonical.com>

Acked-by: Stefan Bader <stefan.bader at 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))