[v3,14/16] bcache: add sysfs file to display feature sets information of cache set
diff mbox series

Message ID 20200715143015.14957-15-colyli@suse.de
State New
Headers show
Series
  • bcache: extend bucket size to 32bit width
Related show

Commit Message

Coly Li July 15, 2020, 2:30 p.m. UTC
From: Coly Li <colyli@suse.de>

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 <colyli@suse.de>
---
 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(-)

Comments

Hannes Reinecke July 16, 2020, 6:17 a.m. UTC | #1
On 7/15/20 4:30 PM, colyli@suse.de wrote:
> From: Coly Li <colyli@suse.de>
> 
> 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 <colyli@suse.de>
> ---
>   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.

Cheers,

Hannes
Coly Li July 16, 2020, 6:20 a.m. UTC | #2
On 2020/7/16 14:17, Hannes Reinecke wrote:
> On 7/15/20 4:30 PM, colyli@suse.de wrote:
>> From: Coly Li <colyli@suse.de>
>>
>> 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 <colyli@suse.de>
>> ---
>>   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

Patch
diff mbox series

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);