[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <20081011170124N.fujita.tomonori@lab.ntt.co.jp>
Date: Sat, 11 Oct 2008 17:01:48 +0900
From: FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>
To: jens.axboe@...cle.com
Cc: fujita.tomonori@....ntt.co.jp, linux-kernel@...r.kernel.org,
linux-scsi@...r.kernel.org, James.Bottomley@...senPartnership.com,
knikanth@...e.de
Subject: Re: [PATCH] block: fix nr_phys_segments miscalculation bug
On Sat, 11 Oct 2008 09:04:03 +0200
Jens Axboe <jens.axboe@...cle.com> wrote:
> On Sat, Oct 11 2008, FUJITA Tomonori wrote:
> > This is against the latest git (b922df7383749a1c0b7ea64c50fa839263d3816b).
> >
> > =
> > From: FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>
> > Subject: [PATCH] block: fix nr_phys_segments miscalculation bug
> >
> > This fixes the bug reported by Nikanth Karthikesan <knikanth@...e.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).
>
> This looks really good, just like I imagined. I'll give it a fuller
> review later today and do a bit of targetted testing, if it goes as
> planned it'll go in soonish. Thanks a lot!
Thanks,
One thing that I thought about fixing is that we could falsely
increase bi_seg_front_size and bi_seg_back_size in
ll_merge_requests_fn() though I chose the same way in which we did for
hw merging.
We might update bi_seg_front_size and bi_seg_back_size if
blk_phys_contig_segment() succeeds. But if total_phys_segments check
fails after blk_phys_contig_segment(), we could falsely increase
bi_seg_front_size and bi_seg_back_size.
But falsely increasing bi_seg_front_size and bi_seg_back_size doesn't
cause any bug. It just means we have less segments. So I let it
alone.
Oh, I forgot to say, I was able to reproduce the bug easily and wow
this patch seems to fix the bug for me.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists