On Tue, May 25, 2021 at 09:40:22AM +0000, Johannes Thumshirn wrote:

On 24/05/2021 01:05, Damien Le Moal wrote: 

quoted

quoted

quoted

+	if (fs_info && fs_info->max_zone_append_size)
+		max_bytes = ALIGN_DOWN(fs_info->max_zone_append_size,
+				       PAGE_SIZE);

Why is the alignment needed? Are the max zone append values expected to
be so random? Also it's using memory-related value for something that's
more hw related, or at least extent size (which ends up on disk).

I did the ALIGN_DOWN() call because we want to have complete pages added.

quoted

It is similar to max_hw_sectors: the hardware decides what the value is. So we
cannot assume anything about what max_zone_append_size is.

I think that Johannes patch here limits the extent size to the HW value to avoid
having to split the extent later one. That is efficient but indeed is a bit of a
layering violation here.

Damien just brought up a good idea: what about a function to lookup the max extent
size depending on the block group. For regular btrfs it'll for now just return 
BTRFS_MAX_EXTENT_SIZE, for zoned btrfs it'll return 
ALIGN_DOWN(fs_info->max_zone_append_size, PAGE_SIZE) and it also gives us some 
headroom for future improvements in this area.

Hm, right that sounds safer. I've grepped for BTRFS_MAX_EXTENT_SIZE and
it's used in many places so it's not just the one you fixed. If the
maximum extent size is really limited by max_zone_append it needs to be
used consistently everywhere, thus needing a helper.