Re: [PATCH v6 00/28] btrfs: zoned block device support

From: David Sterba <dsterba@suse.cz>
To: Naohiro Aota <naohiro.aota@wdc.com>
Cc: linux-btrfs@vger.kernel.org, David Sterba <dsterba@suse.com>,
	Chris Mason <clm@fb.com>, Josef Bacik <josef@toxicpanda.com>,
	Nikolay Borisov <nborisov@suse.com>,
	Damien Le Moal <damien.lemoal@wdc.com>,
	Johannes Thumshirn <jthumshirn@suse.de>,
	Hannes Reinecke <hare@suse.com>,
	Anand Jain <anand.jain@oracle.com>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v6 00/28] btrfs: zoned block device support
Date: Thu, 19 Dec 2019 21:19:41 +0100	[thread overview]
Message-ID: <20191219201941.GR3929@twin.jikos.cz> (raw)
In-Reply-To: <20191213040915.3502922-1-naohiro.aota@wdc.com>

On Fri, Dec 13, 2019 at 01:08:47PM +0900, Naohiro Aota wrote:
> This series adds zoned block device support to btrfs.
> 
> Changes:
>  - Changed -EINVAL to -EOPNOTSUPP to reject incompatible features
>    within HMZONED mode (David)
>  - Use bitmap helpers (Johannes)
>  - Fix calculation of a string length
>  - Code cleanup
> 
> Userland series is unchaged with the last version:
> https://lore.kernel.org/linux-btrfs/20191204082513.857320-1-naohiro.aota@wdc.com/T/
> 
> * Patch series description
> 
> A zoned block device consists of a number of zones. Zones are either
> conventional and accepting random writes or sequential and requiring
> that writes be issued in LBA order from each zone write pointer
> position. This patch series ensures that the sequential write
> constraint of sequential zones is respected while fundamentally not
> changing BtrFS block and I/O management for block stored in
> conventional zones.

One more high-level comment: let's please call it 'zone' mode, without
the 'host managed' part. That term is not relevant for a filesystem. The
zone allocator, or zone append-only allocator or similar describe what
happens on the filesystem layer.

The constraint posed by device is to never overwrite in place, that's
fine for COW design and that's what should be kept in mind while adding
the limitations (no nocow/raid56/...) or exceptions into the code.

In some cases it's not possible to fold the zoned support into existing
helpers but we should do that wherever we can. While reading the code
the number of if (HMZONED) felt too intrusive. This needs to be
adjusted, but I think it's mostly cosmetic or basic refactoring, not
changing the core of the implementation.

So in particular: remove 'hm' everywhere, filename, identifiers. For
short I'd call it 'zone' mode but full description would be something
like 'zone aware append-only allocation mode'.

I'll do another review pass to point out what I think can be refactored
but I hope that with the above gives enough hint.