Re: Re: [PATCH] bio: limit bio max size.

On Wed, Jan 13, 2021 at 12:09 PM Changheun Lee [off-list ref] wrote:

quoted

On 2021/01/12 21:14, Changheun Lee wrote:

quoted

quoted

On 2021/01/12 17:52, Changheun Lee wrote:

quoted

From: "Changheun Lee" <redacted>

bio size can grow up to 4GB when muli-page bvec is enabled.
but sometimes it would lead to inefficient behaviors.
in case of large chunk direct I/O, - 64MB chunk read in user space -
all pages for 64MB would be merged to a bio structure if memory address is
continued phsycally. it makes some delay to submit until merge complete.
bio max size should be limited as a proper size.

But merging physically contiguous pages into the same bvec + later automatic bio
split on submit should give you better throughput for large IOs compared to
having to issue a bio chain of smaller BIOs that are arbitrarily sized and will
likely need splitting anyway (because of DMA boundaries etc).

Do you have a specific case where you see higher performance with this patch
applied ? On Intel, BIO_MAX_SIZE would be 1MB... That is arbitrary and too small
considering that many hardware can execute larger IOs than that.

When I tested 32MB chunk read with O_DIRECT in android, all pages of 32MB
is merged into a bio structure.
And elapsed time to merge complete was about 2ms.
It means first bio-submit is after 2ms.
If bio size is limited with 1MB with this patch, first bio-submit is about
100us by bio_full operation.

bio_submit() will split the large BIO case into multiple requests while the
small BIO case will likely result one or two requests only. That likely explain
the time difference here. However, for the large case, the 2ms will issue ALL
requests needed for processing the entire 32MB user IO while the 1MB bio case
will need 32 different bio_submit() calls. So what is the actual total latency
difference for the entire 32MB user IO ? That is I think what needs to be
compared here.

Also, what is your device max_sectors_kb and max queue depth ?

32MB total latency is about 19ms including merge time without this patch.
But with this patch, total latency is about 17ms including merge time too.

19ms looks too big just for preparing one 32MB sized bio, which isn't
supposed to
take so long.  Can you investigate where the 19ms is taken just for
preparing one
32MB sized bio?

It might be iov_iter_get_pages() for handling page fault. If yes, one suggestion
is to enable THP(Transparent HugePage Support) in your application.


-- 
Ming Lei