Re: [PATCH 09/60] dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments

From	Ming Lei <tom.leiming@gmail.com>
Newsgroups	linux.kernel
Subject	Re: [PATCH 09/60] dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments
Date	2016-11-02 09:00 +0100
Message-ID	<syYCP-3oe-19@gated-at.bofh.it> (permalink)
References	<sxx1U-3GR-3@gated-at.bofh.it> <sxxlf-3Nw-9@gated-at.bofh.it> <symQO-4eD-31@gated-at.bofh.it> <syU65-zX-11@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

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

Thread

[PATCH 09/60] dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments Ming Lei <tom.leiming@gmail.com> - 2016-10-29 10:40 +0200
  Re: [PATCH 09/60] dm: dm.c: replace 'bio->bi_vcnt == 1' with  !bio_multiple_segments Christoph Hellwig <hch@infradead.org> - 2016-10-31 16:40 +0100
    Re: [PATCH 09/60] dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments Ming Lei <tom.leiming@gmail.com> - 2016-11-01 00:10 +0100
    Re: [PATCH 09/60] dm: dm.c: replace 'bio->bi_vcnt == 1' with  !bio_multiple_segments Kent Overstreet <kent.overstreet@gmail.com> - 2016-11-02 04:10 +0100
      Re: [PATCH 09/60] dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments Ming Lei <tom.leiming@gmail.com> - 2016-11-02 09:00 +0100
        Re: [PATCH 09/60] dm: dm.c: replace 'bio->bi_vcnt == 1' with  !bio_multiple_segments Mike Snitzer <snitzer@redhat.com> - 2016-11-02 15:30 +0100
          Re: [PATCH 09/60] dm: dm.c: replace 'bio->bi_vcnt == 1' with !bio_multiple_segments Ming Lei <tom.leiming@gmail.com> - 2016-11-03 00:50 +0100

csiph-web