>On 2021/01/14 12:53, Ming Lei wrote:
>> On Wed, Jan 13, 2021 at 12:02:44PM +0000, Damien Le Moal wrote:
>>> On 2021/01/13 20:48, Ming Lei wrote:
>>>> On Wed, Jan 13, 2021 at 11:16:11AM +0000, Damien Le Moal wrote:
>>>>> On 2021/01/13 19:25, Ming Lei wrote:
>>>>>> On Wed, Jan 13, 2021 at 09:28:02AM +0000, Damien Le Moal wrote:
>>>>>>> On 2021/01/13 18:19, Ming Lei wrote:
>>>>>>>> On Wed, Jan 13, 2021 at 12:09 PM Changheun Lee
>>>>>>>> <nanich....@samsung.com> wrote:
>>>>>>>>>
>>>>>>>>>> On 2021/01/12 21:14, Changheun Lee wrote:
>>>>>>>>>>>> On 2021/01/12 17:52, Changheun Lee wrote:
>>>>>>>>>>>>> From: "Changheun Lee" <nanich....@samsung.com>
>>>>>>>>>>>>>
>>>>>>>>>>>>> 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?
>>>>>>>
>>>>>>> Changheun mentioned that the device side IO latency is 16.7ms out of
>>>>>>> the 19ms
>>>>>>> total. So the BIO handling, submission+completion takes about 2.3ms, and
>>>>>>> Changheun points above to 2ms for the submission part.
>>>>>>
>>>>>> OK, looks I misunderstood the data.
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> 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.
>>>>>>>
>>>>>>> But if that was due to page faults, the same large-ish time would be
>>>>>>> taken for
>>>>>>> the preparing the size-limited BIOs too, no ? No matter how the BIOs
>>>>>>> are diced,
>>>>>>> all 32MB of pages of the user IO are referenced...
>>>>>>
>>>>>> If bio size is reduced to 1MB, just 256 pages need to be faulted before
>>>>>> submitting this
>>>>>> bio, instead of 256*32 pages, that is why the following words are
>>>>>> mentioned:
>>>>>>
>>>>>> 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.
>>>>>
>>>>> Yes, but eventually, all pages for the 32MB IO will be faulted in, just
>>>>> not in
>>>>> one go. Overall number of page faults is likely the same as with the
>>>>> large BIO
>>>>> preparation. So I think we are back to my previous point, that is,
>>>>> reducing the
>>>>> device idle time by starting a BIO more quickly, even a small one, leads
>>>>> to
>>>>> overlap between CPU time needed for the next BIO preparation and previous
>>>>> BIO
>>>>> execution, reducing overall the latency for the entire 32MB user IO.
>>>>
>>>> When bio size is reduced from 32M to 1M:
>>>>
>>>> 1MB/(P(1M) + D(1M)) may become bigger than 32MB/(P(1M) + D(1M)), so
>>>> throughput is improved.
>>>
>>> I think that the reason is that P(1M) < D(1M) and so there is overlap
>>> between P
>>> and D: P of the next BIO is done on the CPU while D of the previous BIO is
>>> ongoing on the device, assuming there is no plugging.
>>
>> Looks you are talking about AIO. IMO, if AIO is used in Changheun's
>> test, the UFS controller pipeline can be saturated easily by many
>> enough(> 8 or more) 32M requests(preparing each takes 2ms, and device need
>> 16ms to handle 32MB req), then there shouldn't be such issue.
>>
>> So I guess Changheun uses sync dio, and the 2ms preparing time is added
>> to bio submission delay every time.
>>
>> Changheun, can you talk about your 32MB block size direct IO test in a
>> bit detail? AIO or sync dio? Do you have fio command line to reproduce
>> this issue?
>
>Maybe also provide a blktrace output of for one 32MB IO execution ?
When 32MB chunk read with direct I/O option is comming from userspace,
kernel behavior is below now. it's timeline.
| bio merge for 32MB. total 8,192 pages are merged.
| total elapsed time is over 2ms.
|------------------ ... ----------------------->|
| 8,192 pages merged a bio.
| at this time, first bio
submit is done.
| 1 bio is split to 32 read
request and issue.
|--------------->
|--------------->
|--------------->
......
|--------------->
|--------------->|
|
total 19ms elapsed to complete 32MB read done from
device. |
If bio max size is limited with 1MB, behavior is changed below.
| bio merge for 1MB. 256 pages are merged for each bio.
| total 32 bio will be made.
| total elapsed time is over 2ms. it's same.
| but, first bio submit timing is fast. about 100us.
|--->|--->|--->|---> ... -->|--->|--->|--->|--->|
| 256 pages merged a bio.
| at this time, first bio submit is done.
| and 1 read request is issued for 1 bio.
|--------------->
|--------------->
|--------------->
......
|--------------->
|--------------->|
|
total 17ms elapsed to complete 32MB read done from device. |
As a result, read request issue timing is faster if bio max size is limited.
Current kernel behavior with multipage bvec, super large bio can be created.
And it lead to delay first I/O request issue.
>
>>
>>
>> Thanks,
>> Ming
>>
>>
>
>
>--
>Damien Le Moal
>Western Digital Research
>
