Messages in this thread | | | From | Ming Lei <> | Date | Wed, 2 Nov 2016 15:56:41 +0800 | Subject | Re: [PATCH 09/60] dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments |
| |
On Wed, Nov 2, 2016 at 11:09 AM, Kent Overstreet <kent.overstreet@gmail.com> wrote: > On Mon, Oct 31, 2016 at 08:29:01AM -0700, Christoph Hellwig wrote: >> On Sat, Oct 29, 2016 at 04:08:08PM +0800, Ming Lei wrote: >> > Avoid to access .bi_vcnt directly, because it may be not what >> > the driver expected any more after supporting multipage bvec. >> > >> > Signed-off-by: Ming Lei <tom.leiming@gmail.com> >> >> It would be really nice to have a comment in the code why it's >> even checking for multiple segments. > > Or ideally refactor the code to not care about multiple segments at all.
The check on 'bio->bi_vcnt == 1' is introduced in commit de3ec86dff160(dm: don't start current request if it would've merged with the previous), which fixed one performance issue.[1]
Looks the idea of the patch is to delay dispatching the rq if it would've merged with previous request and the rq is small(single bvec). I guess the motivation is to try to increase chance of merging with the delay.
But why does the code check on 'bio->bi_vcnt == 1'? Once the bio is submitted, .bi_vcnt isn't changed any more and merging doesn't change it too. So should the check have been on blk_rq_bytes(rq)?
Mike, please correct me if my understanding is wrong.
[1] https://www.redhat.com/archives/dm-devel/2015-March/msg00014.html
thanks, Ming Lei
| |