Skip to content

Commit

Permalink
block: fix nr_phys_segments miscalculation bug
Browse files Browse the repository at this point in the history
This fixes the bug reported by Nikanth Karthikesan <knikanth@suse.de>:

http://lkml.org/lkml/2008/10/2/203

The root cause of the bug is that blk_phys_contig_segment
miscalculates q->max_segment_size.

blk_phys_contig_segment checks:

req->biotail->bi_size + next_req->bio->bi_size > q->max_segment_size

But blk_recalc_rq_segments might expect that req->biotail and the
previous bio in the req are supposed be merged into one
segment. blk_recalc_rq_segments might also expect that next_req->bio
and the next bio in the next_req are supposed be merged into one
segment. In such case, we merge two requests that can't be merged
here. Later, blk_rq_map_sg gives more segments than it should.

We need to keep track of segment size in blk_recalc_rq_segments and
use it to see if two requests can be merged. This patch implements it
in the similar way that we used to do for hw merging (virtual
merging).

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
  • Loading branch information
FUJITA Tomonori authored and Jens Axboe committed Oct 17, 2008
1 parent 0fc71e3 commit 8677142
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 2 deletions.
20 changes: 18 additions & 2 deletions block/blk-merge.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,20 @@ void blk_recalc_rq_segments(struct request *rq)
continue;
}
new_segment:
if (nr_phys_segs == 1 && seg_size > rq->bio->bi_seg_front_size)
rq->bio->bi_seg_front_size = seg_size;

nr_phys_segs++;
bvprv = bv;
seg_size = bv->bv_len;
highprv = high;
}

if (nr_phys_segs == 1 && seg_size > rq->bio->bi_seg_front_size)
rq->bio->bi_seg_front_size = seg_size;
if (seg_size > rq->biotail->bi_seg_back_size)
rq->biotail->bi_seg_back_size = seg_size;

rq->nr_phys_segments = nr_phys_segs;
}

Expand All @@ -106,7 +114,8 @@ static int blk_phys_contig_segment(struct request_queue *q, struct bio *bio,
if (!test_bit(QUEUE_FLAG_CLUSTER, &q->queue_flags))
return 0;

if (bio->bi_size + nxt->bi_size > q->max_segment_size)
if (bio->bi_seg_back_size + nxt->bi_seg_front_size >
q->max_segment_size)
return 0;

if (!bio_has_data(bio))
Expand Down Expand Up @@ -309,6 +318,8 @@ static int ll_merge_requests_fn(struct request_queue *q, struct request *req,
struct request *next)
{
int total_phys_segments;
unsigned int seg_size =
req->biotail->bi_seg_back_size + next->bio->bi_seg_front_size;

/*
* First check if the either of the requests are re-queued
Expand All @@ -324,8 +335,13 @@ static int ll_merge_requests_fn(struct request_queue *q, struct request *req,
return 0;

total_phys_segments = req->nr_phys_segments + next->nr_phys_segments;
if (blk_phys_contig_segment(q, req->biotail, next->bio))
if (blk_phys_contig_segment(q, req->biotail, next->bio)) {
if (req->nr_phys_segments == 1)
req->bio->bi_seg_front_size = seg_size;
if (next->nr_phys_segments == 1)
next->biotail->bi_seg_back_size = seg_size;
total_phys_segments--;
}

if (total_phys_segments > q->max_phys_segments)
return 0;
Expand Down
7 changes: 7 additions & 0 deletions include/linux/bio.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,13 @@ struct bio {

unsigned int bi_size; /* residual I/O count */

/*
* To keep track of the max segment size, we account for the
* sizes of the first and last mergeable segments in this bio.
*/
unsigned int bi_seg_front_size;
unsigned int bi_seg_back_size;

unsigned int bi_max_vecs; /* max bvl_vecs we can hold */

unsigned int bi_comp_cpu; /* completion CPU */
Expand Down

0 comments on commit 8677142

Please sign in to comment.