Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: ToLkHBklhexdyVMjGimD5y2wXS7+5MeZ0dzP4K3QMrXsH8iWUS1gU+fpu0RdW/lMCnPpVy0XG+
 HP9SyzM9wjiReP6omhGQfAyzmfyf2c/kWwARzpjkFcCj8BEIOjT6X0DWtP5S2GXoewhiAkCyco
 +Hl5p1W/njihp2JmB55Hv/NM5rFMKIlz1lu0e0WJV7KfgXS6tK8eJ/kRtgKdSBCOPjEor4CqoY
 nTcXYGmuJNNux4JT0OGUsO4MHvewP6TEOwf7aDhrM/M2JgZHCys3BME63dZpPpGy2FPSip7Fnd
 V20=
From:   Damien Le Moal <Damien.LeMoal@wdc.com>
To:     Ming Lei <ming.lei@redhat.com>
CC:     Ming Lei <tom.leiming@gmail.com>,
        Changheun Lee <nanich.lee@samsung.com>,
        Johannes Thumshirn <Johannes.Thumshirn@wdc.com>,
        Jens Axboe <axboe@kernel.dk>,
        "jisoo2146.oh@samsung.com" <jisoo2146.oh@samsung.com>,
        "junho89.kim@samsung.com" <junho89.kim@samsung.com>,
        linux-block <linux-block@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "mj0123.lee@samsung.com" <mj0123.lee@samsung.com>,
        "seunghwan.hyun@samsung.com" <seunghwan.hyun@samsung.com>,
        "sookwan7.kim@samsung.com" <sookwan7.kim@samsung.com>,
        Tejun Heo <tj@kernel.org>,
        "yt0928.kim@samsung.com" <yt0928.kim@samsung.com>,
        "woosung2.lee@samsung.com" <woosung2.lee@samsung.com>
Subject: Re: [PATCH] bio: limit bio max size.
Thread-Topic: [PATCH] bio: limit bio max size.
Thread-Index: AQHW6Y5jqVlL4YJWRkeFSjyRBQwtnA==
Date:   Thu, 14 Jan 2021 04:00:33 +0000
Message-ID: <BL0PR04MB651470CA7B3AA843E57AF85EE7A80@BL0PR04MB6514.namprd04.prod.outlook.com>
References: <CGME20210113040146epcas1p230596c7c3760471dca442d1f7ce4dc55@epcas1p2.samsung.com>
 <CH2PR04MB65225EDDA7069CCD47A459A5E7A90@CH2PR04MB6522.namprd04.prod.outlook.com>
 <20210113034637.1382-1-nanich.lee@samsung.com>
 <CACVXFVMb0eE5-yo2k3KvnJjKN+aDLzOuT9rKQ7LY5-4WTgM3jw@mail.gmail.com>
 <CH2PR04MB65228D54F66068DA125CCE47E7A90@CH2PR04MB6522.namprd04.prod.outlook.com>
 <20210113102450.GA220440@T590>
 <CH2PR04MB6522CF231DAA4615DABA8457E7A90@CH2PR04MB6522.namprd04.prod.outlook.com>
 <20210113114722.GA233746@T590>
 <CH2PR04MB652295091E1D9455D2132BD8E7A90@CH2PR04MB6522.namprd04.prod.outlook.com>
 <20210114035242.GB237540@T590>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: ab17fd9c-c4f2-4bf0-d301-08d8b840f0d6
X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Jan 2021 04:00:33.0385
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 1Z1BjcG3/ZsJIohOCEgQht/4KprawSaUYOK/ukaFm9md3F7NAcfYYBaPNq2RiG3YEnXgH1rS7vLOa19WrYFmhA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL0PR04MB4851
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2021/01/14 12:53, Ming Lei wrote:=0A=
> On Wed, Jan 13, 2021 at 12:02:44PM +0000, Damien Le Moal wrote:=0A=
>> On 2021/01/13 20:48, Ming Lei wrote:=0A=
>>> On Wed, Jan 13, 2021 at 11:16:11AM +0000, Damien Le Moal wrote:=0A=
>>>> On 2021/01/13 19:25, Ming Lei wrote:=0A=
>>>>> On Wed, Jan 13, 2021 at 09:28:02AM +0000, Damien Le Moal wrote:=0A=
>>>>>> On 2021/01/13 18:19, Ming Lei wrote:=0A=
>>>>>>> On Wed, Jan 13, 2021 at 12:09 PM Changheun Lee <nanich.lee@samsung.=
com> wrote:=0A=
>>>>>>>>=0A=
>>>>>>>>> On 2021/01/12 21:14, Changheun Lee wrote:=0A=
>>>>>>>>>>> On 2021/01/12 17:52, Changheun Lee wrote:=0A=
>>>>>>>>>>>> From: "Changheun Lee" <nanich.lee@samsung.com>=0A=
>>>>>>>>>>>>=0A=
>>>>>>>>>>>> bio size can grow up to 4GB when muli-page bvec is enabled.=0A=
>>>>>>>>>>>> but sometimes it would lead to inefficient behaviors.=0A=
>>>>>>>>>>>> in case of large chunk direct I/O, - 64MB chunk read in user s=
pace -=0A=
>>>>>>>>>>>> all pages for 64MB would be merged to a bio structure if memor=
y address is=0A=
>>>>>>>>>>>> continued phsycally. it makes some delay to submit until merge=
 complete.=0A=
>>>>>>>>>>>> bio max size should be limited as a proper size.=0A=
>>>>>>>>>>>=0A=
>>>>>>>>>>> But merging physically contiguous pages into the same bvec + la=
ter automatic bio=0A=
>>>>>>>>>>> split on submit should give you better throughput for large IOs=
 compared to=0A=
>>>>>>>>>>> having to issue a bio chain of smaller BIOs that are arbitraril=
y sized and will=0A=
>>>>>>>>>>> likely need splitting anyway (because of DMA boundaries etc).=
=0A=
>>>>>>>>>>>=0A=
>>>>>>>>>>> Do you have a specific case where you see higher performance wi=
th this patch=0A=
>>>>>>>>>>> applied ? On Intel, BIO_MAX_SIZE would be 1MB... That is arbitr=
ary and too small=0A=
>>>>>>>>>>> considering that many hardware can execute larger IOs than that=
.=0A=
>>>>>>>>>>>=0A=
>>>>>>>>>>=0A=
>>>>>>>>>> When I tested 32MB chunk read with O_DIRECT in android, all page=
s of 32MB=0A=
>>>>>>>>>> is merged into a bio structure.=0A=
>>>>>>>>>> And elapsed time to merge complete was about 2ms.=0A=
>>>>>>>>>> It means first bio-submit is after 2ms.=0A=
>>>>>>>>>> If bio size is limited with 1MB with this patch, first bio-submi=
t is about=0A=
>>>>>>>>>> 100us by bio_full operation.=0A=
>>>>>>>>>=0A=
>>>>>>>>> bio_submit() will split the large BIO case into multiple requests=
 while the=0A=
>>>>>>>>> small BIO case will likely result one or two requests only. That =
likely explain=0A=
>>>>>>>>> the time difference here. However, for the large case, the 2ms wi=
ll issue ALL=0A=
>>>>>>>>> requests needed for processing the entire 32MB user IO while the =
1MB bio case=0A=
>>>>>>>>> will need 32 different bio_submit() calls. So what is the actual =
total latency=0A=
>>>>>>>>> difference for the entire 32MB user IO ? That is I think what nee=
ds to be=0A=
>>>>>>>>> compared here.=0A=
>>>>>>>>>=0A=
>>>>>>>>> Also, what is your device max_sectors_kb and max queue depth ?=0A=
>>>>>>>>>=0A=
>>>>>>>>=0A=
>>>>>>>> 32MB total latency is about 19ms including merge time without this=
 patch.=0A=
>>>>>>>> But with this patch, total latency is about 17ms including merge t=
ime too.=0A=
>>>>>>>=0A=
>>>>>>> 19ms looks too big just for preparing one 32MB sized bio, which isn=
't=0A=
>>>>>>> supposed to=0A=
>>>>>>> take so long.  Can you investigate where the 19ms is taken just for=
=0A=
>>>>>>> preparing one=0A=
>>>>>>> 32MB sized bio?=0A=
>>>>>>=0A=
>>>>>> Changheun mentioned that the device side IO latency is 16.7ms out of=
 the 19ms=0A=
>>>>>> total. So the BIO handling, submission+completion takes about 2.3ms,=
 and=0A=
>>>>>> Changheun points above to 2ms for the submission part.=0A=
>>>>>=0A=
>>>>> OK, looks I misunderstood the data.=0A=
>>>>>=0A=
>>>>>>=0A=
>>>>>>>=0A=
>>>>>>> It might be iov_iter_get_pages() for handling page fault. If yes, o=
ne suggestion=0A=
>>>>>>> is to enable THP(Transparent HugePage Support) in your application.=
=0A=
>>>>>>=0A=
>>>>>> But if that was due to page faults, the same large-ish time would be=
 taken for=0A=
>>>>>> the preparing the size-limited BIOs too, no ? No matter how the BIOs=
 are diced,=0A=
>>>>>> all 32MB of pages of the user IO are referenced...=0A=
>>>>>=0A=
>>>>> If bio size is reduced to 1MB, just 256 pages need to be faulted befo=
re submitting this=0A=
>>>>> bio, instead of 256*32 pages, that is why the following words are men=
tioned:=0A=
>>>>>=0A=
>>>>> 	It means first bio-submit is after 2ms.=0A=
>>>>> 	If bio size is limited with 1MB with this patch, first bio-submit is=
 about=0A=
>>>>> 	100us by bio_full operation.=0A=
>>>>=0A=
>>>> Yes, but eventually, all pages for the 32MB IO will be faulted in, jus=
t not in=0A=
>>>> one go. Overall number of page faults is likely the same as with the l=
arge BIO=0A=
>>>> preparation. So I think we are back to my previous point, that is, red=
ucing the=0A=
>>>> device idle time by starting a BIO more quickly, even a small one, lea=
ds to=0A=
>>>> overlap between CPU time needed for the next BIO preparation and previ=
ous BIO=0A=
>>>> execution, reducing overall the latency for the entire 32MB user IO.=
=0A=
>>>=0A=
>>> When bio size is reduced from 32M to 1M:=0A=
>>>=0A=
>>> 1MB/(P(1M) + D(1M)) may become bigger than 32MB/(P(1M) + D(1M)), so=0A=
>>> throughput is improved.=0A=
>>=0A=
>> I think that the reason is that P(1M) < D(1M) and so there is overlap be=
tween P=0A=
>> and D: P of the next BIO is done on the CPU while D of the previous BIO =
is=0A=
>> ongoing on the device, assuming there is no plugging.=0A=
> =0A=
> Looks you are talking about AIO. IMO, if AIO is used in Changheun's=0A=
> test, the UFS controller pipeline can be saturated easily by many=0A=
> enough(> 8 or more) 32M requests(preparing each takes 2ms, and device nee=
d=0A=
> 16ms to handle 32MB req), then there shouldn't be such issue.=0A=
> =0A=
> So I guess Changheun uses sync dio, and the 2ms preparing time is added=
=0A=
> to bio submission delay every time.=0A=
> =0A=
> Changheun, can you talk about your 32MB block size direct IO test in a=0A=
> bit detail? AIO or sync dio? Do you have fio command line to reproduce=0A=
> this issue?=0A=
=0A=
Maybe also provide a blktrace output of for one 32MB IO execution ?=0A=
=0A=
> =0A=
> =0A=
> Thanks, =0A=
> Ming=0A=
> =0A=
> =0A=
=0A=
=0A=
-- =0A=
Damien Le Moal=0A=
Western Digital Research=0A=