diff mbox series

[RFC,4/4] btrfs: sysfs: export supported checksums

Message ID e377ded65e4f2799776596ead308658710e4c8c1.1564046812.git.jthumshirn@suse.de (mailing list archive)
State New, archived
Headers show
Series Support xxhash64 checksums | expand

Commit Message

Johannes Thumshirn July 25, 2019, 9:33 a.m. UTC
From: David Sterba <dsterba@suse.com>

Export supported checksum algorithms via sysfs.

Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/sysfs.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

Comments

David Sterba July 30, 2019, 5:19 p.m. UTC | #1
On Thu, Jul 25, 2019 at 11:33:51AM +0200, Johannes Thumshirn wrote:
> From: David Sterba <dsterba@suse.com>
> 
> Export supported checksum algorithms via sysfs.

I wonder if we should also export the implementation that would be used.
This could be crross referenced with /proc/crypto, but having it in one
place would be IMHO convenient.  Also for the case when the kernel
module is missing.

Currently the hash names are printed as comma separated values so we'd
need bit something structured:

crc32c: crc32c-intel
xxhash64: xxhash-generic

or if there's some other format in common use.
Johannes Thumshirn July 31, 2019, 8:06 a.m. UTC | #2
On Tue, Jul 30, 2019 at 07:19:04PM +0200, David Sterba wrote:
> On Thu, Jul 25, 2019 at 11:33:51AM +0200, Johannes Thumshirn wrote:
> > From: David Sterba <dsterba@suse.com>
> > 
> > Export supported checksum algorithms via sysfs.
> 
> I wonder if we should also export the implementation that would be used.
> This could be crross referenced with /proc/crypto, but having it in one
> place would be IMHO convenient.  Also for the case when the kernel
> module is missing.
> 
> Currently the hash names are printed as comma separated values so we'd
> need bit something structured:
> 
> crc32c: crc32c-intel
> xxhash64: xxhash-generic
> 
> or if there's some other format in common use.

Sounds good, will do.
Johannes Thumshirn Aug. 7, 2019, 2:10 p.m. UTC | #3
On Tue, Jul 30, 2019 at 07:19:04PM +0200, David Sterba wrote:
> On Thu, Jul 25, 2019 at 11:33:51AM +0200, Johannes Thumshirn wrote:
> > From: David Sterba <dsterba@suse.com>
> > 
> > Export supported checksum algorithms via sysfs.
> 
> I wonder if we should also export the implementation that would be used.
> This could be crross referenced with /proc/crypto, but having it in one
> place would be IMHO convenient.  Also for the case when the kernel
> module is missing.
> 
> Currently the hash names are printed as comma separated values so we'd
> need bit something structured:
> 
> crc32c: crc32c-intel
> xxhash64: xxhash-generic

I thought a bit more about it and it's not quite that easy as I would need to
have access to the respective "struct crypto_shash". For the currently used
checksum this isn't much of a problem, as I get it via "struct btrfs_fs_info".

But this sysfs attribute lists all the checksumming algorithms the current
btrfs version is able to use irrespectively of the currently mounted
file-systems.

So yes I can do it but I think this is for another sysfs attribute which shows
the used checksumming algorithm for this FS, not the supported checksumming
algorithms.

Byte,
	Johannes
Nikolay Borisov Aug. 12, 2019, 9:19 a.m. UTC | #4
On 25.07.19 г. 12:33 ч., Johannes Thumshirn wrote:
> From: David Sterba <dsterba@suse.com>
> 
> Export supported checksum algorithms via sysfs.
> 
> Signed-off-by: David Sterba <dsterba@suse.com>
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  fs/btrfs/sysfs.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> index 9539f8143b7a..920282a3452b 100644
> --- a/fs/btrfs/sysfs.c
> +++ b/fs/btrfs/sysfs.c
> @@ -182,6 +182,30 @@ static umode_t btrfs_feature_visible(struct kobject *kobj,
>  	return mode;
>  }
>  
> +static ssize_t btrfs_checksums_show(struct kobject *kobj,
> +				       struct kobj_attribute *a, char *buf)
> +{
> +	ssize_t ret = 0;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(btrfs_csums); i++) {
> +		ret += snprintf(buf + ret, PAGE_SIZE, "%s%s",
> +				(i == 0 ? "" : ", "),
> +				btrfs_csums[i].name);
> +
> +	}
> +
> +	ret += snprintf(buf + ret, PAGE_SIZE, "\n");
> +	return ret;
> +}
> +
> +static ssize_t btrfs_checksums_store(struct kobject *kobj,
> +					struct kobj_attribute *a,
> +					const char *buf, size_t count)
> +{
> +	return -EPERM;
> +}
> +
>  BTRFS_FEAT_ATTR_INCOMPAT(mixed_backref, MIXED_BACKREF);
>  BTRFS_FEAT_ATTR_INCOMPAT(default_subvol, DEFAULT_SUBVOL);
>  BTRFS_FEAT_ATTR_INCOMPAT(mixed_groups, MIXED_GROUPS);
> @@ -195,6 +219,14 @@ BTRFS_FEAT_ATTR_INCOMPAT(no_holes, NO_HOLES);
>  BTRFS_FEAT_ATTR_INCOMPAT(metadata_uuid, METADATA_UUID);
>  BTRFS_FEAT_ATTR_COMPAT_RO(free_space_tree, FREE_SPACE_TREE);
>  
> +static struct btrfs_feature_attr btrfs_attr_features_checksums_name = {
> +	.kobj_attr = __INIT_KOBJ_ATTR(checksums, S_IRUGO,
> +				      btrfs_checksums_show,
> +				      btrfs_checksums_store),

Since we won't ever support writing to this sysfs just kill
btrfs_checksums_store and simply pass NULL as the last argument to
INIT_KOBJ_ATTR.

> +	.feature_set	= FEAT_INCOMPAT,
> +	.feature_bit	= 0,
> +};
> +
>  static struct attribute *btrfs_supported_feature_attrs[] = {
>  	BTRFS_FEAT_ATTR_PTR(mixed_backref),
>  	BTRFS_FEAT_ATTR_PTR(default_subvol),
> @@ -208,6 +240,9 @@ static struct attribute *btrfs_supported_feature_attrs[] = {
>  	BTRFS_FEAT_ATTR_PTR(no_holes),
>  	BTRFS_FEAT_ATTR_PTR(metadata_uuid),
>  	BTRFS_FEAT_ATTR_PTR(free_space_tree),
> +
> +	&btrfs_attr_features_checksums_name.kobj_attr.attr,
> +
>  	NULL
>  };
>  
>
Johannes Thumshirn Aug. 19, 2019, 9:15 a.m. UTC | #5
On Mon, Aug 12, 2019 at 12:19:13PM +0300, Nikolay Borisov wrote:
> > +static struct btrfs_feature_attr btrfs_attr_features_checksums_name = {
> > +	.kobj_attr = __INIT_KOBJ_ATTR(checksums, S_IRUGO,
> > +				      btrfs_checksums_show,
> > +				      btrfs_checksums_store),
> 
> Since we won't ever support writing to this sysfs just kill
> btrfs_checksums_store and simply pass NULL as the last argument to
> INIT_KOBJ_ATTR.
> 

Yup will do.
David Sterba Aug. 21, 2019, 4:19 p.m. UTC | #6
On Wed, Aug 07, 2019 at 04:10:05PM +0200, Johannes Thumshirn wrote:
> On Tue, Jul 30, 2019 at 07:19:04PM +0200, David Sterba wrote:
> > On Thu, Jul 25, 2019 at 11:33:51AM +0200, Johannes Thumshirn wrote:
> > > From: David Sterba <dsterba@suse.com>
> > > 
> > > Export supported checksum algorithms via sysfs.
> > 
> > I wonder if we should also export the implementation that would be used.
> > This could be crross referenced with /proc/crypto, but having it in one
> > place would be IMHO convenient.  Also for the case when the kernel
> > module is missing.
> > 
> > Currently the hash names are printed as comma separated values so we'd
> > need bit something structured:
> > 
> > crc32c: crc32c-intel
> > xxhash64: xxhash-generic
> 
> I thought a bit more about it and it's not quite that easy as I would need to
> have access to the respective "struct crypto_shash". For the currently used
> checksum this isn't much of a problem, as I get it via "struct btrfs_fs_info".
> 
> But this sysfs attribute lists all the checksumming algorithms the current
> btrfs version is able to use irrespectively of the currently mounted
> file-systems.
> 
> So yes I can do it but I think this is for another sysfs attribute which shows
> the used checksumming algorithm for this FS, not the supported checksumming
> algorithms.

I see. We can't know which implementation would be used until we
actually try to use it, so two sysfs files would make more sense. Plain
list for all supported algos and one with the implementation for a given
mounted filesystem.
diff mbox series

Patch

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 9539f8143b7a..920282a3452b 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -182,6 +182,30 @@  static umode_t btrfs_feature_visible(struct kobject *kobj,
 	return mode;
 }
 
+static ssize_t btrfs_checksums_show(struct kobject *kobj,
+				       struct kobj_attribute *a, char *buf)
+{
+	ssize_t ret = 0;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(btrfs_csums); i++) {
+		ret += snprintf(buf + ret, PAGE_SIZE, "%s%s",
+				(i == 0 ? "" : ", "),
+				btrfs_csums[i].name);
+
+	}
+
+	ret += snprintf(buf + ret, PAGE_SIZE, "\n");
+	return ret;
+}
+
+static ssize_t btrfs_checksums_store(struct kobject *kobj,
+					struct kobj_attribute *a,
+					const char *buf, size_t count)
+{
+	return -EPERM;
+}
+
 BTRFS_FEAT_ATTR_INCOMPAT(mixed_backref, MIXED_BACKREF);
 BTRFS_FEAT_ATTR_INCOMPAT(default_subvol, DEFAULT_SUBVOL);
 BTRFS_FEAT_ATTR_INCOMPAT(mixed_groups, MIXED_GROUPS);
@@ -195,6 +219,14 @@  BTRFS_FEAT_ATTR_INCOMPAT(no_holes, NO_HOLES);
 BTRFS_FEAT_ATTR_INCOMPAT(metadata_uuid, METADATA_UUID);
 BTRFS_FEAT_ATTR_COMPAT_RO(free_space_tree, FREE_SPACE_TREE);
 
+static struct btrfs_feature_attr btrfs_attr_features_checksums_name = {
+	.kobj_attr = __INIT_KOBJ_ATTR(checksums, S_IRUGO,
+				      btrfs_checksums_show,
+				      btrfs_checksums_store),
+	.feature_set	= FEAT_INCOMPAT,
+	.feature_bit	= 0,
+};
+
 static struct attribute *btrfs_supported_feature_attrs[] = {
 	BTRFS_FEAT_ATTR_PTR(mixed_backref),
 	BTRFS_FEAT_ATTR_PTR(default_subvol),
@@ -208,6 +240,9 @@  static struct attribute *btrfs_supported_feature_attrs[] = {
 	BTRFS_FEAT_ATTR_PTR(no_holes),
 	BTRFS_FEAT_ATTR_PTR(metadata_uuid),
 	BTRFS_FEAT_ATTR_PTR(free_space_tree),
+
+	&btrfs_attr_features_checksums_name.kobj_attr.attr,
+
 	NULL
 };