On 2021/3/15 下午7:59, Anand Jain wrote:

On 10/03/2021 17:08, Qu Wenruo wrote:

quoted

Add extra sysfs interface features/supported_ro_sectorsize and
features/supported_rw_sectorsize to indicate subpage support.

Currently for supported_rw_sectorsize all architectures only have their
PAGE_SIZE listed.

While for supported_ro_sectorsize, for systems with 64K page size, 4K
sectorsize is also supported.

This new sysfs interface would help mkfs.btrfs to do more accurate
warning.

Signed-off-by: Qu Wenruo <redacted>
---

Changes looks good. Nit below...
And maybe it is a good idea to wait for other comments before reroll.

quoted

  fs/btrfs/sysfs.c | 34 ++++++++++++++++++++++++++++++++++
  1 file changed, 34 insertions(+)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 6eb1c50fa98c..3ef419899472 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c

@@ -360,11 +360,45 @@ static ssize_t

supported_rescue_options_show(struct kobject *kobj,
  BTRFS_ATTR(static_feature, supported_rescue_options,
         supported_rescue_options_show);
+static ssize_t supported_ro_sectorsize_show(struct kobject *kobj,
+                        struct kobj_attribute *a,
+                        char *buf)
+{
+    ssize_t ret = 0;
+    int i = 0;

  Drop variable i, as ret can be used instead of 'i'.

quoted

+
+    /* For 64K page size, 4K sector size is supported */
+    if (PAGE_SIZE == SZ_64K) {
+        ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%u", SZ_4K);
+        i++;
+    }
+    /* Other than above subpage, only support PAGE_SIZE as sectorsize
yet */
+    ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%s%lu\n",

quoted

+             (i ? " " : ""), PAGE_SIZE);

                           ^ret

quoted

+    return ret;
+}
+BTRFS_ATTR(static_feature, supported_ro_sectorsize,
+       supported_ro_sectorsize_show);
+
+static ssize_t supported_rw_sectorsize_show(struct kobject *kobj,
+                        struct kobj_attribute *a,
+                        char *buf)
+{
+    ssize_t ret = 0;
+
+    /* Only PAGE_SIZE as sectorsize is supported */
+    ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%lu\n", PAGE_SIZE);
+    return ret;
+}
+BTRFS_ATTR(static_feature, supported_rw_sectorsize,
+       supported_rw_sectorsize_show);

  Why not merge supported_ro_sectorsize and supported_rw_sectorsize
  and show both in two lines...
  For example:
    cat supported_sectorsizes
    ro: 4096 65536
    rw: 65536

If merged, btrfs-progs needs to do line number check before doing string
matching.

Although I doubt the usefulness for supported_ro_sectorsize, as the
window for RO support without RW support should not be that large.
(Current RW passes most generic test cases, and the remaining failures
are very limited)

Thus I can merged them into supported_sectorsize, and only report
sectorsize we can do RW as supported.

Thanks,
Qu

quoted

  static struct attribute *btrfs_supported_static_feature_attrs[] = {
      BTRFS_ATTR_PTR(static_feature, rmdir_subvol),
      BTRFS_ATTR_PTR(static_feature, supported_checksums),
      BTRFS_ATTR_PTR(static_feature, send_stream_version),
      BTRFS_ATTR_PTR(static_feature, supported_rescue_options),
+    BTRFS_ATTR_PTR(static_feature, supported_ro_sectorsize),
+    BTRFS_ATTR_PTR(static_feature, supported_rw_sectorsize),
      NULL
  };