On 2021/10/27 15:28, Johannes Thumshirn wrote:

On 26/10/2021 18:59, Stefan Roesch wrote:

[...]

quoted

+/*
+ * Compute stripe size depending on block type.
+ */
+static u64 compute_stripe_size(struct btrfs_fs_info *info, u64 flags)
+{
+	if (flags & BTRFS_BLOCK_GROUP_DATA) {
+		return SZ_1G;
+	} else if (flags & BTRFS_BLOCK_GROUP_METADATA) {
+		/* For larger filesystems, use larger metadata chunks */
+		return info->fs_devices->total_rw_bytes > 50ULL * SZ_1G
+			? 5ULL * SZ_1G
+			: SZ_256M;
+	} else if (flags & BTRFS_BLOCK_GROUP_SYSTEM) {
+		return SZ_32M;
+	}
+
+	BUG();
+}
+
+/*
+ * Compute chunk size depending on block type and stripe size.
+ */
+static u64 compute_chunk_size(u64 flags, u64 max_stripe_size)
+{
+	if (flags & BTRFS_BLOCK_GROUP_DATA)
+		return BTRFS_MAX_DATA_CHUNK_SIZE;
+	else if (flags & BTRFS_BLOCK_GROUP_METADATA)
+		return max_stripe_size;
+	else if (flags & BTRFS_BLOCK_GROUP_SYSTEM)
+		return 2 * max_stripe_size;
+
+	BUG();
+}
+
+/*
+ * Update maximum stripe size and chunk size.
+ *
+ */
+void btrfs_update_space_info_max_alloc_sizes(struct btrfs_space_info *space_info,
+					     u64 flags, u64 max_stripe_size)
+{
+	spin_lock(&space_info->lock);
+	space_info->max_stripe_size = max_stripe_size;
+	space_info->max_chunk_size = compute_chunk_size(flags,
+						space_info->max_stripe_size);
+	spin_unlock(&space_info->lock);
+}
+
 static int create_space_info(struct btrfs_fs_info *info, u64 flags)
 {
 

@@ -203,6 +251,10 @@ static int create_space_info(struct btrfs_fs_info *info, u64 flags)
 	INIT_LIST_HEAD(&space_info->priority_tickets);
 	space_info->clamp = 1;
 
+	space_info->max_stripe_size = compute_stripe_size(info, flags);
+	space_info->max_chunk_size = compute_chunk_size(flags,
+						space_info->max_stripe_size);
+
 	ret = btrfs_sysfs_add_space_info_type(info, space_info);
 	if (ret)
 		return ret;

[...]

  

quoted

+/*
+ * Return space info stripe size.
+ */
+static ssize_t btrfs_stripe_size_show(struct kobject *kobj,
+				      struct kobj_attribute *a, char *buf)
+{
+	struct btrfs_space_info *sinfo = to_space_info(kobj);
+
+	return btrfs_show_u64(&sinfo->max_stripe_size, &sinfo->lock, buf);
+}

This will return the wrong values for a fs on a zoned device.
What you could do is:

static ssize_t btrfs_stripe_size_show(struct kobject *kobj,
				struct kobj_attribute *a, char *buf)

{
	struct btrfs_space_info *sinfo = to_space_info(kobj);
	struct btrfs_fs_info *fs_info = to_fs_info(get_btrfs_kobj(kobj));
	u64 max_stripe_size;

	spin_lock(&sinfo->lock);
	if (btrfs_is_zoned(fs_info))
		max_stripe_size = fs_info->zone_size;
	else
		max_stripe_size = sinfo->max_stripe_size;
	spin_unlock(&sinfo->lock);

This will not work once we have stripped zoned volume though, won't it ?
Why is not max_stripe_size set to zone size for a simple zoned btrfs volume ?

	return btrfs_show_u64(&max_stripe_size, NULL, buf);
}

[...]

quoted

+	if (fs_info->fs_devices->chunk_alloc_policy == BTRFS_CHUNK_ALLOC_ZONED)
+		return -EINVAL;

Nit: As we can't mix zoned and non-zoned devices 'if (btrfs_is_zoned(fs_info))'
should do the trick here as well and is way more readable. 

-- 
Damien Le Moal
Western Digital Research