On 2020/7/16 14:17, Hannes Reinecke wrote:

On 7/15/20 4:30 PM, colyli@suse.de wrote:

quoted

From: Coly Li <redacted>

A new sysfs file /sys/fs/bcache/<cache set UUID>/internal/feature_sets
is added by this patch, to display feature sets information of the cache
set.

Now only an incompat feature 'large_bucket' added in bcache, the sysfs
file content is:
    feature_incompat: [large_bucket]
string large_bucket means the running bcache drive supports incompat
feature 'large_bucket', the wrapping [] means the 'large_bucket' feature
is currently enabled on this cache set.

This patch is ready to display compat and ro_compat features, in future
once bcache code implements such feature sets, the according feature
strings will be displayed in this sysfs file too.

Signed-off-by: Coly Li <redacted>
---
  drivers/md/bcache/Makefile   |  2 +-
  drivers/md/bcache/features.c | 48 ++++++++++++++++++++++++++++++++++++
  drivers/md/bcache/features.h |  3 +++
  drivers/md/bcache/sysfs.c    |  6 +++++
  4 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/md/bcache/Makefile b/drivers/md/bcache/Makefile
index fd714628da6a..5b87e59676b8 100644
--- a/drivers/md/bcache/Makefile
+++ b/drivers/md/bcache/Makefile

@@ -4,4 +4,4 @@ obj-$(CONFIG_BCACHE)    += bcache.o

    bcache-y        := alloc.o bset.o btree.o closure.o debug.o
extents.o\
      io.o journal.o movinggc.o request.o stats.o super.o sysfs.o
trace.o\
-    util.o writeback.o
+    util.o writeback.o features.o

diff --git a/drivers/md/bcache/features.c b/drivers/md/bcache/features.c
index ba53944bb390..5c601635e11c 100644
--- a/drivers/md/bcache/features.c
+++ b/drivers/md/bcache/features.c

@@ -8,6 +8,7 @@

   */
  #include <linux/bcache.h>
  #include "bcache.h"
+#include "features.h"
    struct feature {
      int        compat;

@@ -20,3 +21,50 @@ static struct feature feature_list[] = {

          "large_bucket"},
      {0, 0, 0 },
  };
+
+#define compose_feature_string(type, head)                \
+({                                    \
+    struct feature *f;                        \
+    bool first = true;                        \
+                                    \
+    for (f = &feature_list[0]; f->compat != 0; f++) {        \
+        if (f->compat != BCH_FEATURE_ ## type)            \
+            continue;                    \
+        if (BCH_HAS_ ## type ## _FEATURE(&c->sb, f->mask)) {    \
+            if (first) {                    \
+                out += snprintf(out, buf + size - out,    \
+                        "%s%s", head, ": [");    \
+            } else {                    \
+                out += snprintf(out, buf + size - out,    \
+                        " [");            \
+            }                        \
+        } else {                        \
+            if (first)                    \
+                out += snprintf(out, buf + size - out,    \
+                        "%s%s", head, ": ");    \
+            else                        \
+                out += snprintf(out, buf + size - out,    \
+                        " ");            \
+        }                            \
+                                    \
+        out += snprintf(out, buf + size - out, "%s", f->string);\
+                                    \
+        if (BCH_HAS_ ## type ## _FEATURE(&c->sb, f->mask))    \
+            out += snprintf(out, buf + size - out, "]");    \
+                                    \
+        first = false;                        \
+    }                                \
+    if (!first)                            \
+        out += snprintf(out, buf + size - out, "\n");        \
+})
+
+int bch_print_cache_set_feature_set(struct cache_set *c, char *buf,
int size)
+{
+    char *out = buf;
+
+    compose_feature_string(COMPAT, "feature_compat");
+    compose_feature_string(RO_COMPAT, "feature_ro_compat");
+    compose_feature_string(INCOMPAT, "feature_incompat");
+
+    return out - buf;
+}

diff --git a/drivers/md/bcache/features.h b/drivers/md/bcache/features.h
index dca052cf5203..350a4f413136 100644
--- a/drivers/md/bcache/features.h
+++ b/drivers/md/bcache/features.h

@@ -78,4 +78,7 @@ static inline void bch_clear_feature_##name(struct

cache_sb *sb) \
  }
    BCH_FEATURE_INCOMPAT_FUNCS(large_bucket, LARGE_BUCKET);
+
+int bch_print_cache_set_feature_set(struct cache_set *c, char *buf,
int size);
+
  #endif

diff --git a/drivers/md/bcache/sysfs.c b/drivers/md/bcache/sysfs.c
index 0dadec5a78f6..e3633f06d43b 100644
--- a/drivers/md/bcache/sysfs.c
+++ b/drivers/md/bcache/sysfs.c

@@ -11,6 +11,7 @@

  #include "btree.h"
  #include "request.h"
  #include "writeback.h"
+#include "features.h"
    #include <linux/blkdev.h>
  #include <linux/sort.h>

@@ -88,6 +89,7 @@ read_attribute(btree_used_percent);

  read_attribute(average_key_size);
  read_attribute(dirty_data);
  read_attribute(bset_tree_stats);
+read_attribute(feature_sets);
    read_attribute(state);
  read_attribute(cache_read_races);

@@ -779,6 +781,9 @@ SHOW(__bch_cache_set)

      if (attr == &sysfs_bset_tree_stats)
          return bch_bset_print_stats(c, buf);
  +    if (attr == &sysfs_feature_sets)
+        return bch_print_cache_set_feature_set(c, buf, PAGE_SIZE);
+
      return 0;
  }
  SHOW_LOCKED(bch_cache_set)

@@ -987,6 +992,7 @@ static struct attribute

*bch_cache_set_internal_files[] = {
      &sysfs_io_disable,
      &sysfs_cutoff_writeback,
      &sysfs_cutoff_writeback_sync,
+    &sysfs_feature_sets,
      NULL
  };
  KTYPE(bch_cache_set_internal);

Tsk.

sysfs attributes should be one value only, not some string declaring
several things at once.
Why not adding two attributes (features_compat and features_incompat),
listing each feature?
That would even easier to implement.

Copied, will fix it in next version. Thanks.

Coly Li