From ab0fd1debe730ec9998678a0c53caefbd121ed10 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Fri, 3 Jul 2009 12:56:18 +0200 Subject: block: don't merge requests of different failfast settings Block layer used to merge requests and bios with different failfast settings. This caused regular IOs to fail prematurely when they were merged into failfast requests for readahead. Niel Lambrechts could trigger the problem semi-reliably on ext4 when resuming from STR. ext4 uses readahead when reading inodes and combined with the deterministic extra SATA PHY exception cycle during resume on the specific configuration, non-readahead inode read would fail causing ext4 errors. Please read the following thread for details. http://lkml.org/lkml/2009/5/23/21 This patch makes block layer reject merging if the failfast settings don't match. This is correct but likely to lower IO performance by preventing regular IOs from mingling into surrounding readahead requests. Changes to allow such mixed merges and handle errors correctly will be added later. Signed-off-by: Tejun Heo Reported-by: Niel Lambrechts Cc: Theodore Tso Signed-off-by: Jens Axboe --- block/elevator.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'block/elevator.c') diff --git a/block/elevator.c b/block/elevator.c index ca861927ba4..6f2375339a9 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -100,6 +100,14 @@ int elv_rq_merge_ok(struct request *rq, struct bio *bio) if (bio_integrity(bio) != blk_integrity_rq(rq)) return 0; + /* + * Don't merge if failfast settings don't match + */ + 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)) + return 0; + if (!elv_iosched_allow_merge(rq, bio)) return 0; -- cgit v1.2.3-70-g09d2 From 0a09f4319c6d88c732ed46735f8584bbb95cac65 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Thu, 16 Jul 2009 15:26:55 +0900 Subject: block: fix failfast merge testing in elv_rq_merge_ok() Commit ab0fd1debe730ec9998678a0c53caefbd121ed10 tries to prevent merge of requests with different failfast settings. In elv_rq_merge_ok(), it compares new bio's failfast flags against the merge target request's. However, the flag testing accessors for bio and blk don't return boolean but the tested bit value directly and FAILFAST on bio and blk don't match, so directly comparing them with == results in false negative unnecessary preventing merge of readahead requests. This patch convert the results to boolean by negating them before comparison. Signed-off-by: Tejun Heo Cc: Jens Axboe Cc: Boaz Harrosh Cc: FUJITA Tomonori Cc: James Bottomley Cc: Jeff Garzik --- block/elevator.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'block/elevator.c') diff --git a/block/elevator.c b/block/elevator.c index 6f2375339a9..2d511f9105e 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -101,11 +101,16 @@ 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. + * + * 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_failfast_dev(bio) != !blk_failfast_dev(rq) || + !bio_failfast_transport(bio) != !blk_failfast_transport(rq) || + !bio_failfast_driver(bio) != !blk_failfast_driver(rq)) return 0; if (!elv_iosched_allow_merge(rq, bio)) -- cgit v1.2.3-70-g09d2