| Message ID | 944b685765a68c3389888159d3fe228c2e78eb22.1564046812.git.jthumshirn@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support xxhash64 checksums | expand |
On 2019/7/25 5:33 PM, Johannes Thumshirn wrote: > Create a structure to encode the type and length for the known on-disk > checksums. Also add a table and a convenience macro for adding the > checksum types to the table. > > This makes it easier to add new checksums later. > > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> > --- > fs/btrfs/ctree.h | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > index da97ff10f421..099401f5dd47 100644 > --- a/fs/btrfs/ctree.h > +++ b/fs/btrfs/ctree.h > @@ -82,9 +82,15 @@ struct btrfs_ref; > */ > #define BTRFS_LINK_MAX 65535U > > -/* four bytes for CRC32 */ > -static const int btrfs_csum_sizes[] = { 4 }; > -static const char *btrfs_csum_names[] = { "crc32c" }; > +#define BTRFS_CHECKSUM_TYPE(_type, _size, _name) \ > + [_type] = { .size = _size, .name = _name } > + > +static const struct btrfs_csums { > + u16 size; > + const char *name; > +} btrfs_csums[] = { > + BTRFS_CHECKSUM_TYPE(BTRFS_CSUM_TYPE_CRC32, 4, "crc32c"), > +}; > How about: struct btrfs_csum { u16 size; const char *name; }; static const struct btrfs_csum btrfs_csums[] = { #define BTRFS_CHECKSUM_TYPE(_type, _size, _name) \ [_type] = { .size = _size, .name = _name } BTRFS_CHECKSUM_TYPE(BTRFS_CSUM_TYPE_CRC32, 4, "crc32c"), }; Since the macro BTRFS_CHECKSUM_TYPE is only used in array btrfs_csum btrfs_csums. And this makes the struct btrfs_csum clear. --- Su > #define BTRFS_EMPTY_DIR_SIZE 0 > > @@ -2373,13 +2379,13 @@ static inline int btrfs_super_csum_size(const struct btrfs_super_block *s) > /* > * csum type is validated at mount time > */ > - return btrfs_csum_sizes[t]; > + return btrfs_csums[t].size; > } > > static inline const char *btrfs_super_csum_name(u16 csum_type) > { > /* csum type is validated at mount time */ > - return btrfs_csum_names[csum_type]; > + return btrfs_csums[csum_type].name; > } > > /* >
On Thu, Jul 25, 2019 at 11:33:49AM +0200, Johannes Thumshirn wrote: > Create a structure to encode the type and length for the known on-disk > checksums. Also add a table and a convenience macro for adding the > checksum types to the table. > > This makes it easier to add new checksums later. > > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> > --- > fs/btrfs/ctree.h | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > index da97ff10f421..099401f5dd47 100644 > --- a/fs/btrfs/ctree.h > +++ b/fs/btrfs/ctree.h > @@ -82,9 +82,15 @@ struct btrfs_ref; > */ > #define BTRFS_LINK_MAX 65535U > > -/* four bytes for CRC32 */ > -static const int btrfs_csum_sizes[] = { 4 }; > -static const char *btrfs_csum_names[] = { "crc32c" }; > +#define BTRFS_CHECKSUM_TYPE(_type, _size, _name) \ > + [_type] = { .size = _size, .name = _name } I think the macro initializer might be an overkill here, there are only 3 items to initialize. > + > +static const struct btrfs_csums { > + u16 size; > + const char *name; > +} btrfs_csums[] = { > + BTRFS_CHECKSUM_TYPE(BTRFS_CSUM_TYPE_CRC32, 4, "crc32c"), > +}; > > #define BTRFS_EMPTY_DIR_SIZE 0 > > @@ -2373,13 +2379,13 @@ static inline int btrfs_super_csum_size(const struct btrfs_super_block *s) > /* > * csum type is validated at mount time > */ > - return btrfs_csum_sizes[t]; > + return btrfs_csums[t].size; > } > > static inline const char *btrfs_super_csum_name(u16 csum_type) > { > /* csum type is validated at mount time */ > - return btrfs_csum_names[csum_type]; > + return btrfs_csums[csum_type].name; > } This has been in the code already, but shouldn't the btrfs_csums table be declared in ctree.h and defined in eg. in ctree.c? As ctree.h is included by everything we have multiple copies of it in the .o files.
On 25.07.19 г. 12:33 ч., Johannes Thumshirn wrote: > Create a structure to encode the type and length for the known on-disk > checksums. Also add a table and a convenience macro for adding the > checksum types to the table. > > This makes it easier to add new checksums later. > > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> > --- > fs/btrfs/ctree.h | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > index da97ff10f421..099401f5dd47 100644 > --- a/fs/btrfs/ctree.h > +++ b/fs/btrfs/ctree.h > @@ -82,9 +82,15 @@ struct btrfs_ref; > */ > #define BTRFS_LINK_MAX 65535U > > -/* four bytes for CRC32 */ > -static const int btrfs_csum_sizes[] = { 4 }; > -static const char *btrfs_csum_names[] = { "crc32c" }; > +#define BTRFS_CHECKSUM_TYPE(_type, _size, _name) \ > + [_type] = { .size = _size, .name = _name } > + > +static const struct btrfs_csums { > + u16 size; > + const char *name; > +} btrfs_csums[] = { > + BTRFS_CHECKSUM_TYPE(BTRFS_CSUM_TYPE_CRC32, 4, "crc32c"), > +}; Considering we won't support more than 4-5 csums I'd rather you remove the macro. > > #define BTRFS_EMPTY_DIR_SIZE 0 > > @@ -2373,13 +2379,13 @@ static inline int btrfs_super_csum_size(const struct btrfs_super_block *s) > /* > * csum type is validated at mount time > */ > - return btrfs_csum_sizes[t]; > + return btrfs_csums[t].size; > } > > static inline const char *btrfs_super_csum_name(u16 csum_type) > { > /* csum type is validated at mount time */ > - return btrfs_csum_names[csum_type]; > + return btrfs_csums[csum_type].name; > } > > /* >
On Mon, Aug 12, 2019 at 12:07:44PM +0300, Nikolay Borisov wrote: > > > On 25.07.19 г. 12:33 ч., Johannes Thumshirn wrote: > > Create a structure to encode the type and length for the known on-disk > > checksums. Also add a table and a convenience macro for adding the > > checksum types to the table. > > > > This makes it easier to add new checksums later. > > > > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> > > --- > > fs/btrfs/ctree.h | 16 +++++++++++----- > > 1 file changed, 11 insertions(+), 5 deletions(-) > > > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > > index da97ff10f421..099401f5dd47 100644 > > --- a/fs/btrfs/ctree.h > > +++ b/fs/btrfs/ctree.h > > @@ -82,9 +82,15 @@ struct btrfs_ref; > > */ > > #define BTRFS_LINK_MAX 65535U > > > > -/* four bytes for CRC32 */ > > -static const int btrfs_csum_sizes[] = { 4 }; > > -static const char *btrfs_csum_names[] = { "crc32c" }; > > +#define BTRFS_CHECKSUM_TYPE(_type, _size, _name) \ > > + [_type] = { .size = _size, .name = _name } > > + > > +static const struct btrfs_csums { > > + u16 size; > > + const char *name; > > +} btrfs_csums[] = { > > + BTRFS_CHECKSUM_TYPE(BTRFS_CSUM_TYPE_CRC32, 4, "crc32c"), > > +}; > > > Considering we won't support more than 4-5 csums I'd rather you remove > the macro. Yeah already dropped it, though I thought it might help a bit on the readability/self-describing side.
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index da97ff10f421..099401f5dd47 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -82,9 +82,15 @@ struct btrfs_ref; */ #define BTRFS_LINK_MAX 65535U -/* four bytes for CRC32 */ -static const int btrfs_csum_sizes[] = { 4 }; -static const char *btrfs_csum_names[] = { "crc32c" }; +#define BTRFS_CHECKSUM_TYPE(_type, _size, _name) \ + [_type] = { .size = _size, .name = _name } + +static const struct btrfs_csums { + u16 size; + const char *name; +} btrfs_csums[] = { + BTRFS_CHECKSUM_TYPE(BTRFS_CSUM_TYPE_CRC32, 4, "crc32c"), +}; #define BTRFS_EMPTY_DIR_SIZE 0 @@ -2373,13 +2379,13 @@ static inline int btrfs_super_csum_size(const struct btrfs_super_block *s) /* * csum type is validated at mount time */ - return btrfs_csum_sizes[t]; + return btrfs_csums[t].size; } static inline const char *btrfs_super_csum_name(u16 csum_type) { /* csum type is validated at mount time */ - return btrfs_csum_names[csum_type]; + return btrfs_csums[csum_type].name; } /*
Create a structure to encode the type and length for the known on-disk checksums. Also add a table and a convenience macro for adding the checksum types to the table. This makes it easier to add new checksums later. Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> --- fs/btrfs/ctree.h | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)