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: wacTIo5+t2FzwH+88WMUNPbNzlw24Ma2+BLE2JOfs/SN5wL1g87mv8qstej3ydKpOsOpxj9fdW
 E8O7LYLePbhNjzLWfXERSjOwNtE8xmGR9s8LBDvN3cdi3432n/lpv6s/73kRyzp/I66LTVoLn+
 AX/c/QqpkiDArgVZhCpndV5gn1Do+9h+asmqV4bDVB+6mDATsFjJ2AFeRo//ez5Xopjs506GMe
 i+RGWJfTylxNgb7LHECVKtW63SYJUtoljONJFcT6NdhQ4QRv4T0Mpyx0y7TLDY/h5+MQd3dAV8
 UtA=
From:   Damien Le Moal <Damien.LeMoal@wdc.com>
To:     Ming Lei <tom.leiming@gmail.com>,
        Changheun Lee <nanich.lee@samsung.com>
CC:     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:   Wed, 13 Jan 2021 09:28:02 +0000
Message-ID: <CH2PR04MB65228D54F66068DA125CCE47E7A90@CH2PR04MB6522.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>
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: 79ccc1da-5896-4daf-77bb-08d8b7a5865c
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Jan 2021 09:28:02.3274
 (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: 5CGTxPTo+ENnxXJBPD6xbiP0BIQQO03RgcMdVCPg98wobUIoQJ4C3EV1YtXEw3vHTua5XiGCaFhpZH1rDkvWZg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR04MB6981
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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> w=
rote:=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 space -=
=0A=
>>>>>> all pages for 64MB would be merged to a bio structure if memory addr=
ess is=0A=
>>>>>> continued phsycally. it makes some delay to submit until merge compl=
ete.=0A=
>>>>>> bio max size should be limited as a proper size.=0A=
>>>>>=0A=
>>>>> But merging physically contiguous pages into the same bvec + later au=
tomatic bio=0A=
>>>>> split on submit should give you better throughput for large IOs compa=
red to=0A=
>>>>> having to issue a bio chain of smaller BIOs that are arbitrarily size=
d 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 with thi=
s patch=0A=
>>>>> applied ? On Intel, BIO_MAX_SIZE would be 1MB... That is arbitrary an=
d 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 pages of 3=
2MB=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-submit is a=
bout=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 will iss=
ue ALL=0A=
>>> requests needed for processing the entire 32MB user IO while the 1MB bi=
o 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 needs 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 time to=
o.=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 19=
ms=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=
> =0A=
> It might be iov_iter_get_pages() for handling page fault. If yes, one sug=
gestion=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 di=
ced,=0A=
all 32MB of pages of the user IO are referenced...=0A=
=0A=
> =0A=
> =0A=
=0A=
=0A=
-- =0A=
Damien Le Moal=0A=
Western Digital Research=0A=