Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Damien Le Moal <Damien.LeMoal@wdc.com>
To:     Josef Bacik <josef@toxicpanda.com>,
        Naohiro Aota <Naohiro.Aota@wdc.com>
CC:     "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
        David Sterba <dsterba@suse.com>, Chris Mason <clm@fb.com>,
        Qu Wenruo <wqu@suse.com>, Nikolay Borisov <nborisov@suse.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Hannes Reinecke <hare@suse.com>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        =?iso-8859-1?Q?Matias_Bj=F8rling?= <mb@lightnvm.io>,
        Johannes Thumshirn <jthumshirn@suse.de>,
        Bart Van Assche <bvanassche@acm.org>
Subject: Re: [PATCH 11/19] btrfs: introduce submit buffer
Thread-Topic: [PATCH 11/19] btrfs: introduce submit buffer
Thread-Index: AQHVHTKKjg778rmXIkyNFIGQA7/Xbg==
Date:   Mon, 17 Jun 2019 03:16:05 +0000
Message-ID: <BYAPR04MB5816E9FC012A289CA438E794E7EB0@BYAPR04MB5816.namprd04.prod.outlook.com>
References: <20190607131025.31996-1-naohiro.aota@wdc.com>
 <20190607131025.31996-12-naohiro.aota@wdc.com>
 <20190613141457.jws5ca63wfgjf7da@MacBook-Pro-91.local>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 45f42baf-75ad-4e76-7f31-08d6f2d222cd
X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Jun 2019 03:16:05.8391
 (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: Damien.LeMoal@wdc.com
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR04MB5991
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Josef,=0A=
=0A=
On 2019/06/13 23:15, Josef Bacik wrote:=0A=
> On Fri, Jun 07, 2019 at 10:10:17PM +0900, Naohiro Aota wrote:=0A=
>> Sequential allocation is not enough to maintain sequential delivery of=
=0A=
>> write IOs to the device. Various features (async compress, async checksu=
m,=0A=
>> ...) of btrfs affect ordering of the IOs. This patch introduces submit=
=0A=
>> buffer to sort WRITE bios belonging to a block group and sort them out=
=0A=
>> sequentially in increasing block address to achieve sequential write=0A=
>> sequences with __btrfs_map_bio().=0A=
>>=0A=
>> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>=0A=
> =0A=
> I hate everything about this.  Can't we just use the plugging infrastruct=
ure for=0A=
> this and then make sure it re-orders the bios before submitting them?  Al=
so=0A=
> what's to prevent the block layer scheduler from re-arranging these io's?=
=0A=
> Thanks,=0A=
=0A=
The block I/O scheduler reorders requests in LBA order, but that happens fo=
r a=0A=
newly inserted request against pending requests. If there are no pending=0A=
requests because all requests were already issued, no ordering happen, and =
even=0A=
worse, if the drive queue is not full yet (e.g. there are free tags), then =
the=0A=
newly inserted request will be dispatched almost immediately, preventing=0A=
reordering with subsequent incoming write requests to happen.=0A=
=0A=
The other problem is that the mq-deadline scheduler does not track zone WP=
=0A=
position. Write request issuing is done regardless of the current WP value,=
=0A=
solely based on LBA ordering. This means that mq-deadline will not prevent=
=0A=
out-of-order, or rather, unaligned write requests. These will not be detect=
ed=0A=
and dispatched whenever possible. The reasons for this are that:=0A=
1) the disk user (the FS) has to manage zone WP positions anyway. So duplic=
ating=0A=
that management at the block IO scheduler level is inefficient.=0A=
2) Adding zone WP management at the block IO scheduler level would also nee=
d a=0A=
write error processing path to resync the WP value in case of failed writes=
. But=0A=
the user/FS also needs that anyway. Again duplicated functionalities.=0A=
3) The block layer will need a timeout to force issue or cancel pending=0A=
unaligned write requests. This is necessary in case the drive user stops is=
suing=0A=
writes (for whatever reasons) or the scheduler is being switched. This woul=
d=0A=
unnecessarily cause write I/O errors or cause deadlocks if the request queu=
e=0A=
quiesce mode is entered at the wrong time (and I do not see a good way to d=
eal=0A=
with that).=0A=
=0A=
blk-mq is already complicated enough. Adding this to the block IO scheduler=
 will=0A=
unnecessarily complicate things further for no real benefits. I would like =
to=0A=
point out the dm-zoned device mapper and f2fs which are both already dealin=
g=0A=
with write ordering and write error processing directly. Both are fairly=0A=
straightforward but completely different and each optimized for their own s=
tructure.=0A=
=0A=
Naohiro changes to btrfs IO scheduler have the same intent, that is, effici=
ently=0A=
integrate and handle write ordering "a la btrfs". Would creating a differen=
t=0A=
"hmzoned" btrfs IO scheduler help address your concerns ?=0A=
=0A=
Best regards.=0A=
=0A=
=0A=
-- =0A=
Damien Le Moal=0A=
Western Digital Research=0A=