Re: [PATCH RFC] block: fix bio merge checks when virt_boundary is set

From	Ming Lei <tom.leiming@gmail.com>
Newsgroups	linux.kernel
Subject	Re: [PATCH RFC] block: fix bio merge checks when virt_boundary is set
Date	2016-03-30 15:10 +0200
Message-ID	<rio2K-5RV-7@gated-at.bofh.it> (permalink)
References	(2 earlier) <rdmuC-2UO-7@gated-at.bofh.it> <rdsgG-71y-19@gated-at.bofh.it> <rdEhQ-6LW-21@gated-at.bofh.it> <rdJhx-1oX-43@gated-at.bofh.it> <rdSNQ-7PY-1@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

On Fri, Mar 18, 2016 at 10:59 AM, Ming Lei <tom.leiming@gmail.com> wrote:
> On Fri, Mar 18, 2016 at 12:39 AM, Keith Busch <keith.busch@intel.com> wrote:
>> On Thu, Mar 17, 2016 at 12:20:28PM +0100, Vitaly Kuznetsov wrote:
>>> Keith Busch <keith.busch@intel.com> writes:
>>> > been combined. In any case, I think you can get what you're after just
>>> > by moving the gap check after BIOVEC_PHYS_MERGABLE. Does the following
>>> > look ok to you?
>>> >
>>>
>>> Thanks, it does.
>>
>> Cool, thanks for confirming.
>>
>>> Will you send it or would you like me to do that with your Suggested-by?
>>
>> I'm not confident yet this doesn't break anything, particularly since
>> we moved the gap check after the length check. Just wanted to confirm
>> the concept addressed your concern, but still need to take a closer look
>> and test before submitting.
>
> IMO, the change on blk_bio_segment_split() is correct, because actually it
> is a sg gap and the check should have been done between segments
> instead of bvecs. So it is reasonable to move the check just before populating
> a new segment.

Thinking of the 1st part change further, looks it is just correct in concept,
but wrong from current implementation. Because of bios/reqs merge,
blk_rq_map_sg() may end one segment in any bvec in theroy, so I guess
that is why each non-1st bvec need the check to make sure no sg gap.
Looks a very crazy limit, :-)

>
> But for the 2nd change in bio_will_gap(), which should fix Vitaly's problem, I
> am still not sure if it is completely correct. bio_will_gap() is used
> to check if two
> bios may be merged. Suppose two bios are continues physically, the last bvec
> in 1st bio and the first bvec in 2nd bio might not be in one same segment
> because of segment size limit.

How about the attached patch?


>
> The root cause might be from blkdev_writepage(), and I guess these small
> bios are from there.
>
> thanks,
> Ming Lei



-- 
Ming Lei

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Re: [PATCH RFC] block: fix bio merge checks when virt_boundary is  set Keith Busch <keith.busch@intel.com> - 2016-03-17 17:50 +0100
  Re: [PATCH RFC] block: fix bio merge checks when virt_boundary is set Ming Lei <tom.leiming@gmail.com> - 2016-03-18 04:00 +0100
    Re: [PATCH RFC] block: fix bio merge checks when virt_boundary is set Ming Lei <tom.leiming@gmail.com> - 2016-03-30 15:10 +0200

csiph-web