On 2021/3/16 上午2:44, David Sterba wrote:
On Mon, Mar 15, 2021 at 08:39:31PM +0800, Qu Wenruo wrote:


On 2021/3/15 下午7:59, Anand Jain wrote:
On 10/03/2021 17:08, Qu Wenruo wrote:
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 <w...@suse.com>
---

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

   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'.

+
+    /* 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",

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

+    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.

The sysfs files should do one value per file.

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.

In that case one file with the list of supported values is a better
option. The main point is to have full RW support, until then it's
interesting only for developers and they know what to expect.


Indeed only full RW support makes sense.

BTW, any comment on the file name? If no problem I would just use
"supported_sectorsize" in next update.

Although I hope the sysfs interface can be merged separately early, so
that I can add the proper support in btrfs-progs.

Thanks,
Qu

Reply via email to