linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/17] Add support for SHA-256 checksums
@ 2019-05-10 11:15 Johannes Thumshirn
  2019-05-10 11:15 ` [PATCH 01/17] btrfs: use btrfs_csum_data() instead of directly calling crc32c Johannes Thumshirn
                   ` (17 more replies)
  0 siblings, 18 replies; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

This patchset add support for adding new checksum types in BTRFS.

Currently BTRFS only supports CRC32C as data and metadata checksum, which is
good if you only want to detect errors due to data corruption in hardware.

But CRC32C isn't able cover other use-cases like de-duplication or
cryptographically save data integrity guarantees.

The following properties made SHA-256 interesting for these use-cases:
- Still considered cryptographically sound
- Reasonably well understood by the security industry
- Result fits into the 32Byte/256Bit we have for the checksum in the on-disk
  format
- Small enough collision space to make it feasible for data de-duplication
- Fast enough to calculate and offloadable to crypto hardware via the kernel's
  crypto_shash framework.

The patchset also provides mechanisms for plumbing in different hash
algorithms relatively easy.

Unfortunately this patchset also partially reverts commit: 
9678c54388b6 ("btrfs: Remove custom crc32c init code")

Patches 1 - 16 are preparation patches to make the change of the checksum
algorithm easy and patch 17 then finally adds SHA-256 as a new checksum
algorithm.

Only patch 17/17 is dependent on mkfs changes.

Johannes Thumshirn (17):
  btrfs: use btrfs_csum_data() instead of directly calling crc32c
  btrfs: resurrect btrfs_crc32c()
  btrfs: use btrfs_crc32c() instead of btrfs_extref_hash()
  btrfs: use btrfs_crc32c() instead of btrfs_name_hash()
  btrfs: don't assume ordered sums to be 4 bytes
  btrfs: dont assume compressed_bio sums to be 4 bytes
  btrfs: use btrfs_crc32c{,_final}() in for free space cache
  btrfs: format checksums according to type for printing
  btrfs: add common checksum type validation
  btrfs: check for supported superblock checksum type before checksum
    validation
  btrfs: Simplify btrfs_check_super_csum() and get rid of size
    assumptions
  btrfs: add boilerplate code for directly including the crypto
    framework
  btrfs: pass in an fs_info to btrfs_csum_{data,final}()
  btrfs: directly call into crypto framework for checsumming
  btrfs: remove assumption about csum type form
    btrfs_csum_{data,final}()
  btrfs: remove assumption about csum type form
    btrfs_print_data_csum_error()
  btrfs: add sha256 as another checksum algorithm

 fs/btrfs/btrfs_inode.h          |  33 ++++++--
 fs/btrfs/check-integrity.c      |   6 +-
 fs/btrfs/compression.c          |  34 +++++----
 fs/btrfs/compression.h          |   2 +-
 fs/btrfs/ctree.h                |  30 +++++---
 fs/btrfs/dir-item.c             |  10 +--
 fs/btrfs/disk-io.c              | 164 +++++++++++++++++++++++++++++-----------
 fs/btrfs/disk-io.h              |   5 +-
 fs/btrfs/extent-tree.c          |   6 +-
 fs/btrfs/file-item.c            |  35 ++++-----
 fs/btrfs/free-space-cache.c     |  10 +--
 fs/btrfs/inode-item.c           |   6 +-
 fs/btrfs/inode.c                |  21 +++--
 fs/btrfs/ordered-data.c         |  13 ++--
 fs/btrfs/ordered-data.h         |   4 +-
 fs/btrfs/props.c                |   5 +-
 fs/btrfs/scrub.c                |  22 +++---
 fs/btrfs/send.c                 |   5 +-
 fs/btrfs/tree-checker.c         |   3 +-
 fs/btrfs/tree-log.c             |   6 +-
 include/uapi/linux/btrfs_tree.h |   1 +
 21 files changed, 276 insertions(+), 145 deletions(-)

-- 
2.16.4


^ permalink raw reply	[flat|nested] 60+ messages in thread

* [PATCH 01/17] btrfs: use btrfs_csum_data() instead of directly calling crc32c
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
@ 2019-05-10 11:15 ` Johannes Thumshirn
  2019-05-10 16:16   ` Nikolay Borisov
  2019-05-10 11:15 ` [PATCH 02/17] btrfs: resurrect btrfs_crc32c() Johannes Thumshirn
                   ` (16 subsequent siblings)
  17 siblings, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

btrfsic_test_for_metadata() directly calls the crc32c() library function
for calculating the CRC32C checksum, but then uses btrfs_csum_final() to
invert the result.

To ease further refactoring and development around checksumming in BTRFS
convert to calling btrfs_csum_data(), which is a wrapper around crc32c().

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/check-integrity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index b0c8094528d1..85774e2fa3e5 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -1728,7 +1728,7 @@ static int btrfsic_test_for_metadata(struct btrfsic_state *state,
 		size_t sublen = i ? PAGE_SIZE :
 				    (PAGE_SIZE - BTRFS_CSUM_SIZE);
 
-		crc = crc32c(crc, data, sublen);
+		crc = btrfs_csum_data(data, crc, sublen);
 	}
 	btrfs_csum_final(crc, csum);
 	if (memcmp(csum, h->csum, state->csum_size))
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 02/17] btrfs: resurrect btrfs_crc32c()
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
  2019-05-10 11:15 ` [PATCH 01/17] btrfs: use btrfs_csum_data() instead of directly calling crc32c Johannes Thumshirn
@ 2019-05-10 11:15 ` Johannes Thumshirn
  2019-05-10 16:16   ` Nikolay Borisov
  2019-05-10 11:15 ` [PATCH 03/17] btrfs: use btrfs_crc32c() instead of btrfs_extref_hash() Johannes Thumshirn
                   ` (15 subsequent siblings)
  17 siblings, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

Commit 9678c54388b6 ("btrfs: Remove custom crc32c init code") removed the
btrfs_crc32c() function, because it was a duplicate of the crc32c() library
function we already have in the kernel.

Resurrect it as a shim wrapper over crc32c() to make following
transformations of the checksumming code in btrfs easier.

Also provide a btrfs_crc32_final() to ease following transformations.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/ctree.h       | 12 ++++++++++++
 fs/btrfs/extent-tree.c |  6 +++---
 fs/btrfs/send.c        |  2 +-
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index b81c331b28fa..d85541f13f65 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -19,6 +19,7 @@
 #include <linux/kobject.h>
 #include <trace/events/btrfs.h>
 #include <asm/kmap_types.h>
+#include <asm/unaligned.h>
 #include <linux/pagemap.h>
 #include <linux/btrfs.h>
 #include <linux/btrfs_tree.h>
@@ -2642,6 +2643,17 @@ BTRFS_SETGET_STACK_FUNCS(stack_dev_replace_cursor_right,
 	((unsigned long)(BTRFS_LEAF_DATA_OFFSET + \
 	btrfs_item_offset_nr(leaf, slot)))
 
+
+static inline u32 btrfs_crc32c(u32 crc, const void *address, unsigned length)
+{
+	return crc32c(crc, address, length);
+}
+
+static inline void btrfs_crc32c_final(u32 crc, u8 *result)
+{
+	put_unaligned_le32(~crc, result);
+}
+
 static inline u64 btrfs_name_hash(const char *name, int len)
 {
        return crc32c((u32)~1, name, len);
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index f79e477a378e..06a30f2cd2e0 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -1119,11 +1119,11 @@ static u64 hash_extent_data_ref(u64 root_objectid, u64 owner, u64 offset)
 	__le64 lenum;
 
 	lenum = cpu_to_le64(root_objectid);
-	high_crc = crc32c(high_crc, &lenum, sizeof(lenum));
+	high_crc = btrfs_crc32c(high_crc, &lenum, sizeof(lenum));
 	lenum = cpu_to_le64(owner);
-	low_crc = crc32c(low_crc, &lenum, sizeof(lenum));
+	low_crc = btrfs_crc32c(low_crc, &lenum, sizeof(lenum));
 	lenum = cpu_to_le64(offset);
-	low_crc = crc32c(low_crc, &lenum, sizeof(lenum));
+	low_crc = btrfs_crc32c(low_crc, &lenum, sizeof(lenum));
 
 	return ((u64)high_crc << 31) ^ (u64)low_crc;
 }
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index dd38dfe174df..c029ca6d5eba 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -686,7 +686,7 @@ static int send_cmd(struct send_ctx *sctx)
 	hdr->len = cpu_to_le32(sctx->send_size - sizeof(*hdr));
 	hdr->crc = 0;
 
-	crc = crc32c(0, (unsigned char *)sctx->send_buf, sctx->send_size);
+	crc = btrfs_crc32c(0, (unsigned char *)sctx->send_buf, sctx->send_size);
 	hdr->crc = cpu_to_le32(crc);
 
 	ret = write_buf(sctx->send_filp, sctx->send_buf, sctx->send_size,
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 03/17] btrfs: use btrfs_crc32c() instead of btrfs_extref_hash()
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
  2019-05-10 11:15 ` [PATCH 01/17] btrfs: use btrfs_csum_data() instead of directly calling crc32c Johannes Thumshirn
  2019-05-10 11:15 ` [PATCH 02/17] btrfs: resurrect btrfs_crc32c() Johannes Thumshirn
@ 2019-05-10 11:15 ` Johannes Thumshirn
  2019-05-10 13:03   ` Nikolay Borisov
  2019-05-10 11:15 ` [PATCH 04/17] btrfs: use btrfs_crc32c() instead of btrfs_name_hash() Johannes Thumshirn
                   ` (14 subsequent siblings)
  17 siblings, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

Like btrfs_crc32c() btrfs_extref_hash() is only a shim wrapper over the
crc32c() library function. So we can just use btrfs_crc32c() instead of
btrfs_extref_hash().

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/ctree.h      | 9 ---------
 fs/btrfs/inode-item.c | 6 +++---
 fs/btrfs/tree-log.c   | 6 +++---
 3 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index d85541f13f65..a3479a4e4f4d 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2659,15 +2659,6 @@ static inline u64 btrfs_name_hash(const char *name, int len)
        return crc32c((u32)~1, name, len);
 }
 
-/*
- * Figure the key offset of an extended inode ref
- */
-static inline u64 btrfs_extref_hash(u64 parent_objectid, const char *name,
-                                   int len)
-{
-       return (u64) crc32c(parent_objectid, name, len);
-}
-
 static inline bool btrfs_mixed_space_info(struct btrfs_space_info *space_info)
 {
 	return ((space_info->flags & BTRFS_BLOCK_GROUP_METADATA) &&
diff --git a/fs/btrfs/inode-item.c b/fs/btrfs/inode-item.c
index 30d62ef918b9..d2b2a64a1553 100644
--- a/fs/btrfs/inode-item.c
+++ b/fs/btrfs/inode-item.c
@@ -91,7 +91,7 @@ btrfs_lookup_inode_extref(struct btrfs_trans_handle *trans,
 
 	key.objectid = inode_objectid;
 	key.type = BTRFS_INODE_EXTREF_KEY;
-	key.offset = btrfs_extref_hash(ref_objectid, name, name_len);
+	key.offset = (u64) btrfs_crc32c(ref_objectid, name, name_len);
 
 	ret = btrfs_search_slot(trans, root, &key, path, ins_len, cow);
 	if (ret < 0)
@@ -123,7 +123,7 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans,
 
 	key.objectid = inode_objectid;
 	key.type = BTRFS_INODE_EXTREF_KEY;
-	key.offset = btrfs_extref_hash(ref_objectid, name, name_len);
+	key.offset = (u64) btrfs_crc32c(ref_objectid, name, name_len);
 
 	path = btrfs_alloc_path();
 	if (!path)
@@ -272,7 +272,7 @@ static int btrfs_insert_inode_extref(struct btrfs_trans_handle *trans,
 
 	key.objectid = inode_objectid;
 	key.type = BTRFS_INODE_EXTREF_KEY;
-	key.offset = btrfs_extref_hash(ref_objectid, name, name_len);
+	key.offset = (u64) btrfs_crc32c(ref_objectid, name, name_len);
 
 	path = btrfs_alloc_path();
 	if (!path)
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 6adcd8a2c5c7..f01c6e1a15ed 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -1111,7 +1111,7 @@ static inline int __add_inode_ref(struct btrfs_trans_handle *trans,
 
 			search_key.objectid = inode_objectid;
 			search_key.type = BTRFS_INODE_EXTREF_KEY;
-			search_key.offset = btrfs_extref_hash(parent_objectid,
+			search_key.offset = (u64) btrfs_crc32c(parent_objectid,
 							      victim_name,
 							      victim_name_len);
 			ret = 0;
@@ -1323,7 +1323,7 @@ static int btrfs_inode_ref_exists(struct inode *inode, struct inode *dir,
 	if (key.type == BTRFS_INODE_REF_KEY)
 		key.offset = parent_id;
 	else
-		key.offset = btrfs_extref_hash(parent_id, name, namelen);
+		key.offset = (u64) btrfs_crc32c(parent_id, name, namelen);
 
 	ret = btrfs_search_slot(NULL, BTRFS_I(inode)->root, &key, path, 0, 0);
 	if (ret < 0)
@@ -1901,7 +1901,7 @@ static bool name_in_log_ref(struct btrfs_root *log_root,
 		return true;
 
 	search_key.type = BTRFS_INODE_EXTREF_KEY;
-	search_key.offset = btrfs_extref_hash(dirid, name, name_len);
+	search_key.offset = (u64) btrfs_crc32c(dirid, name, name_len);
 	if (backref_in_log(log_root, &search_key, dirid, name, name_len))
 		return true;
 
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 04/17] btrfs: use btrfs_crc32c() instead of btrfs_name_hash()
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
                   ` (2 preceding siblings ...)
  2019-05-10 11:15 ` [PATCH 03/17] btrfs: use btrfs_crc32c() instead of btrfs_extref_hash() Johannes Thumshirn
@ 2019-05-10 11:15 ` Johannes Thumshirn
  2019-05-10 12:56   ` Chris Mason
  2019-05-10 11:15 ` [PATCH 05/17] btrfs: don't assume ordered sums to be 4 bytes Johannes Thumshirn
                   ` (13 subsequent siblings)
  17 siblings, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

Like btrfs_crc32c() btrfs_name_hash() is only a shim wrapper over the
crc32c() library function. So we can just use btrfs_crc32c() instead of
btrfs_name_hash().

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/ctree.h        |  5 -----
 fs/btrfs/dir-item.c     | 10 +++++-----
 fs/btrfs/inode.c        |  6 ++++--
 fs/btrfs/props.c        |  5 +++--
 fs/btrfs/send.c         |  3 ++-
 fs/btrfs/tree-checker.c |  3 ++-
 6 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index a3479a4e4f4d..0a2a377f1640 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2654,11 +2654,6 @@ static inline void btrfs_crc32c_final(u32 crc, u8 *result)
 	put_unaligned_le32(~crc, result);
 }
 
-static inline u64 btrfs_name_hash(const char *name, int len)
-{
-       return crc32c((u32)~1, name, len);
-}
-
 static inline bool btrfs_mixed_space_info(struct btrfs_space_info *space_info)
 {
 	return ((space_info->flags & BTRFS_BLOCK_GROUP_METADATA) &&
diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c
index 863367c2c620..c8eccc0a2379 100644
--- a/fs/btrfs/dir-item.c
+++ b/fs/btrfs/dir-item.c
@@ -71,7 +71,7 @@ int btrfs_insert_xattr_item(struct btrfs_trans_handle *trans,
 
 	key.objectid = objectid;
 	key.type = BTRFS_XATTR_ITEM_KEY;
-	key.offset = btrfs_name_hash(name, name_len);
+	key.offset = (u64) btrfs_crc32c((u32)~1, name, name_len);
 
 	data_size = sizeof(*dir_item) + name_len + data_len;
 	dir_item = insert_with_overflow(trans, root, path, &key, data_size,
@@ -122,7 +122,7 @@ int btrfs_insert_dir_item(struct btrfs_trans_handle *trans, const char *name,
 
 	key.objectid = btrfs_ino(dir);
 	key.type = BTRFS_DIR_ITEM_KEY;
-	key.offset = btrfs_name_hash(name, name_len);
+	key.offset = (u64) btrfs_crc32c((u32)~1, name, name_len);
 
 	path = btrfs_alloc_path();
 	if (!path)
@@ -190,7 +190,7 @@ struct btrfs_dir_item *btrfs_lookup_dir_item(struct btrfs_trans_handle *trans,
 	key.objectid = dir;
 	key.type = BTRFS_DIR_ITEM_KEY;
 
-	key.offset = btrfs_name_hash(name, name_len);
+	key.offset = (u64) btrfs_crc32c((u32)~1, name, name_len);
 
 	ret = btrfs_search_slot(trans, root, &key, path, ins_len, cow);
 	if (ret < 0)
@@ -219,7 +219,7 @@ int btrfs_check_dir_item_collision(struct btrfs_root *root, u64 dir,
 
 	key.objectid = dir;
 	key.type = BTRFS_DIR_ITEM_KEY;
-	key.offset = btrfs_name_hash(name, name_len);
+	key.offset = (u64) btrfs_crc32c((u32)~1, name, name_len);
 
 	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
 
@@ -353,7 +353,7 @@ struct btrfs_dir_item *btrfs_lookup_xattr(struct btrfs_trans_handle *trans,
 
 	key.objectid = dir;
 	key.type = BTRFS_XATTR_ITEM_KEY;
-	key.offset = btrfs_name_hash(name, name_len);
+	key.offset = (u64) btrfs_crc32c((u32)~1, name, name_len);
 	ret = btrfs_search_slot(trans, root, &key, path, ins_len, cow);
 	if (ret < 0)
 		return ERR_PTR(ret);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index b6d549c993f6..db19f113d514 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3548,9 +3548,11 @@ static noinline int acls_after_inode_item(struct extent_buffer *leaf,
 	int scanned = 0;
 
 	if (!xattr_access) {
-		xattr_access = btrfs_name_hash(XATTR_NAME_POSIX_ACL_ACCESS,
+		xattr_access = (u64) btrfs_crc32c((u32)~1,
+					XATTR_NAME_POSIX_ACL_ACCESS,
 					strlen(XATTR_NAME_POSIX_ACL_ACCESS));
-		xattr_default = btrfs_name_hash(XATTR_NAME_POSIX_ACL_DEFAULT,
+		xattr_default = (u64) btrfs_crc32c((u32)~1,
+					XATTR_NAME_POSIX_ACL_DEFAULT,
 					strlen(XATTR_NAME_POSIX_ACL_DEFAULT));
 	}
 
diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
index a9e2e66152ee..10687edbb73d 100644
--- a/fs/btrfs/props.c
+++ b/fs/btrfs/props.c
@@ -41,7 +41,7 @@ find_prop_handler(const char *name,
 	struct prop_handler *h;
 
 	if (!handlers) {
-		u64 hash = btrfs_name_hash(name, strlen(name));
+		u64 hash = (u64) btrfs_crc32c((u32)~1, name, strlen(name));
 
 		handlers = find_prop_handlers_by_hash(hash);
 		if (!handlers)
@@ -445,7 +445,8 @@ void __init btrfs_props_init(void)
 
 	for (i = 0; i < ARRAY_SIZE(prop_handlers); i++) {
 		struct prop_handler *p = &prop_handlers[i];
-		u64 h = btrfs_name_hash(p->xattr_name, strlen(p->xattr_name));
+		u64 h = (u64) btrfs_crc32c((u32)~1, p->xattr_name,
+					   strlen(p->xattr_name));
 
 		hash_add(prop_handlers_ht, &p->node, h);
 	}
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index c029ca6d5eba..76eff609d877 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -3434,7 +3434,8 @@ static int wait_for_dest_dir_move(struct send_ctx *sctx,
 
 	key.objectid = parent_ref->dir;
 	key.type = BTRFS_DIR_ITEM_KEY;
-	key.offset = btrfs_name_hash(parent_ref->name, parent_ref->name_len);
+	key.offset = (u64) btrfs_crc32c((u32)~1, parent_ref->name,
+					parent_ref->name_len);
 
 	ret = btrfs_search_slot(NULL, sctx->parent_root, &key, path, 0, 0);
 	if (ret < 0) {
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 748cd1598255..0cb955bc7566 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -343,7 +343,8 @@ static int check_dir_item(struct extent_buffer *leaf,
 
 			read_extent_buffer(leaf, namebuf,
 					(unsigned long)(di + 1), name_len);
-			name_hash = btrfs_name_hash(namebuf, name_len);
+			name_hash = (u64) btrfs_crc32c((u32)~1, namebuf,
+						       name_len);
 			if (key->offset != name_hash) {
 				dir_item_err(leaf, slot,
 		"name hash mismatch with key, have 0x%016x expect 0x%016llx",
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 05/17] btrfs: don't assume ordered sums to be 4 bytes
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
                   ` (3 preceding siblings ...)
  2019-05-10 11:15 ` [PATCH 04/17] btrfs: use btrfs_crc32c() instead of btrfs_name_hash() Johannes Thumshirn
@ 2019-05-10 11:15 ` Johannes Thumshirn
  2019-05-10 13:25   ` Nikolay Borisov
  2019-05-10 11:15 ` [PATCH 06/17] btrfs: dont assume compressed_bio " Johannes Thumshirn
                   ` (12 subsequent siblings)
  17 siblings, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

BTRFS has the implicit assumption that a checksum in btrfs_orderd_sums is 4
bytes. While this is true for CRC32C, it is not for any other checksum.

Change the data type to be a byte array and adjust loop index calculation
accordingly.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/compression.c  |  4 ++--
 fs/btrfs/ctree.h        |  3 ++-
 fs/btrfs/file-item.c    | 28 +++++++++++++++-------------
 fs/btrfs/ordered-data.c | 10 ++++++----
 fs/btrfs/ordered-data.h |  4 ++--
 fs/btrfs/scrub.c        |  2 +-
 6 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 4ec1df369e47..98d8c2ed367f 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -632,7 +632,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 
 			if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
 				ret = btrfs_lookup_bio_sums(inode, comp_bio,
-							    sums);
+							    (u8 *)sums);
 				BUG_ON(ret); /* -ENOMEM */
 			}
 			sums += DIV_ROUND_UP(comp_bio->bi_iter.bi_size,
@@ -658,7 +658,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 	BUG_ON(ret); /* -ENOMEM */
 
 	if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
-		ret = btrfs_lookup_bio_sums(inode, comp_bio, sums);
+		ret = btrfs_lookup_bio_sums(inode, comp_bio, (u8 *) sums);
 		BUG_ON(ret); /* -ENOMEM */
 	}
 
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 0a2a377f1640..e62934fb8748 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3184,7 +3184,8 @@ int btrfs_find_name_in_ext_backref(struct extent_buffer *leaf, int slot,
 struct btrfs_dio_private;
 int btrfs_del_csums(struct btrfs_trans_handle *trans,
 		    struct btrfs_fs_info *fs_info, u64 bytenr, u64 len);
-blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio, u32 *dst);
+blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
+				   u8 *dst);
 blk_status_t btrfs_lookup_bio_sums_dio(struct inode *inode, struct bio *bio,
 			      u64 logical_offset);
 int btrfs_insert_file_extent(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index d431ea8198e4..210ff69917a0 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -22,9 +22,9 @@
 #define MAX_CSUM_ITEMS(r, size) (min_t(u32, __MAX_CSUM_ITEMS(r, size), \
 				       PAGE_SIZE))
 
-#define MAX_ORDERED_SUM_BYTES(fs_info) ((PAGE_SIZE - \
+#define MAX_ORDERED_SUM_BYTES(fs_info, csum_size) ((PAGE_SIZE - \
 				   sizeof(struct btrfs_ordered_sum)) / \
-				   sizeof(u32) * (fs_info)->sectorsize)
+				   (csum_size) * (fs_info)->sectorsize)
 
 int btrfs_insert_file_extent(struct btrfs_trans_handle *trans,
 			     struct btrfs_root *root,
@@ -144,7 +144,7 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
 }
 
 static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
-				   u64 logical_offset, u32 *dst, int dio)
+				   u64 logical_offset, u8 *dst, int dio)
 {
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
 	struct bio_vec bvec;
@@ -211,7 +211,7 @@ static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio
 		if (!dio)
 			offset = page_offset(bvec.bv_page) + bvec.bv_offset;
 		count = btrfs_find_ordered_sum(inode, offset, disk_bytenr,
-					       (u32 *)csum, nblocks);
+					       csum, nblocks);
 		if (count)
 			goto found;
 
@@ -283,7 +283,8 @@ static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio
 	return 0;
 }
 
-blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio, u32 *dst)
+blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
+				   u8 *dst)
 {
 	return __btrfs_lookup_bio_sums(inode, bio, 0, dst, 0);
 }
@@ -374,7 +375,7 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
 				      struct btrfs_csum_item);
 		while (start < csum_end) {
 			size = min_t(size_t, csum_end - start,
-				     MAX_ORDERED_SUM_BYTES(fs_info));
+				     MAX_ORDERED_SUM_BYTES(fs_info, csum_size));
 			sums = kzalloc(btrfs_ordered_sum_size(fs_info, size),
 				       GFP_NOFS);
 			if (!sums) {
@@ -439,6 +440,7 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
 	int i;
 	u64 offset;
 	unsigned nofs_flag;
+	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
 
 	nofs_flag = memalloc_nofs_save();
 	sums = kvzalloc(btrfs_ordered_sum_size(fs_info, bio->bi_iter.bi_size),
@@ -473,6 +475,7 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
 						 - 1);
 
 		for (i = 0; i < nr_sectors; i++) {
+			u32 tmp;
 			if (offset >= ordered->file_offset + ordered->len ||
 				offset < ordered->file_offset) {
 				unsigned long bytes_left;
@@ -498,17 +501,16 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
 				index = 0;
 			}
 
-			sums->sums[index] = ~(u32)0;
+			memset(&sums->sums[index], 0xff, csum_size);
 			data = kmap_atomic(bvec.bv_page);
-			sums->sums[index]
-				= btrfs_csum_data(data + bvec.bv_offset
+			tmp = btrfs_csum_data(data + bvec.bv_offset
 						+ (i * fs_info->sectorsize),
-						sums->sums[index],
+						*(u32 *)&sums->sums[index],
 						fs_info->sectorsize);
 			kunmap_atomic(data);
-			btrfs_csum_final(sums->sums[index],
+			btrfs_csum_final(tmp,
 					(char *)(sums->sums + index));
-			index++;
+			index += csum_size;
 			offset += fs_info->sectorsize;
 			this_sum_bytes += fs_info->sectorsize;
 			total_bytes += fs_info->sectorsize;
@@ -904,9 +906,9 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
 	write_extent_buffer(leaf, sums->sums + index, (unsigned long)item,
 			    ins_size);
 
+	index += ins_size;
 	ins_size /= csum_size;
 	total_bytes += ins_size * fs_info->sectorsize;
-	index += ins_size;
 
 	btrfs_mark_buffer_dirty(path->nodes[0]);
 	if (total_bytes < sums->len) {
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 52889da69113..6f7a18148dcb 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -924,14 +924,16 @@ int btrfs_ordered_update_i_size(struct inode *inode, u64 offset,
  * be reclaimed before their checksum is actually put into the btree
  */
 int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
-			   u32 *sum, int len)
+			   u8 *sum, int len)
 {
+	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
 	struct btrfs_ordered_sum *ordered_sum;
 	struct btrfs_ordered_extent *ordered;
 	struct btrfs_ordered_inode_tree *tree = &BTRFS_I(inode)->ordered_tree;
 	unsigned long num_sectors;
 	unsigned long i;
 	u32 sectorsize = btrfs_inode_sectorsize(inode);
+	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
 	int index = 0;
 
 	ordered = btrfs_lookup_ordered_extent(inode, offset);
@@ -947,10 +949,10 @@ int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
 			num_sectors = ordered_sum->len >>
 				      inode->i_sb->s_blocksize_bits;
 			num_sectors = min_t(int, len - index, num_sectors - i);
-			memcpy(sum + index, ordered_sum->sums + i,
-			       num_sectors);
+			memcpy(sum + index, ordered_sum->sums + i * csum_size,
+			       num_sectors * csum_size);
 
-			index += (int)num_sectors;
+			index += (int)num_sectors * csum_size;
 			if (index == len)
 				goto out;
 			disk_bytenr += num_sectors * sectorsize;
diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
index 4c5991c3de14..9a9884966343 100644
--- a/fs/btrfs/ordered-data.h
+++ b/fs/btrfs/ordered-data.h
@@ -23,7 +23,7 @@ struct btrfs_ordered_sum {
 	int len;
 	struct list_head list;
 	/* last field is a variable length array of csums */
-	u32 sums[];
+	u8 sums[];
 };
 
 /*
@@ -183,7 +183,7 @@ struct btrfs_ordered_extent *btrfs_lookup_ordered_range(
 int btrfs_ordered_update_i_size(struct inode *inode, u64 offset,
 				struct btrfs_ordered_extent *ordered);
 int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
-			   u32 *sum, int len);
+			   u8 *sum, int len);
 u64 btrfs_wait_ordered_extents(struct btrfs_root *root, u64 nr,
 			       const u64 range_start, const u64 range_len);
 u64 btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, u64 nr,
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index f7b29f9db5e2..2cf3cf9e9c9b 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -2448,7 +2448,7 @@ static int scrub_find_csum(struct scrub_ctx *sctx, u64 logical, u8 *csum)
 	ASSERT(index < UINT_MAX);
 
 	num_sectors = sum->len / sctx->fs_info->sectorsize;
-	memcpy(csum, sum->sums + index, sctx->csum_size);
+	memcpy(csum, sum->sums + index * sctx->csum_size, sctx->csum_size);
 	if (index == num_sectors - 1) {
 		list_del(&sum->list);
 		kfree(sum);
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 06/17] btrfs: dont assume compressed_bio sums to be 4 bytes
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
                   ` (4 preceding siblings ...)
  2019-05-10 11:15 ` [PATCH 05/17] btrfs: don't assume ordered sums to be 4 bytes Johannes Thumshirn
@ 2019-05-10 11:15 ` Johannes Thumshirn
  2019-05-10 11:15 ` [PATCH 07/17] btrfs: use btrfs_crc32c{,_final}() in for free space cache Johannes Thumshirn
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

BTRFS has the implicit assumption that a checksum in compressed_bio is 4
bytes. While this is true for CRC32C, it is not for any other checksum.

Change the data type to be a byte array and adjust loop index calculation
accordingly.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/compression.c  | 27 +++++++++++++++++----------
 fs/btrfs/compression.h  |  2 +-
 fs/btrfs/file-item.c    |  2 +-
 fs/btrfs/ordered-data.c |  3 ++-
 4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 98d8c2ed367f..d5642f3b5c44 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -57,12 +57,14 @@ static int check_compressed_csum(struct btrfs_inode *inode,
 				 struct compressed_bio *cb,
 				 u64 disk_start)
 {
+	struct btrfs_fs_info *fs_info = inode->root->fs_info;
+	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
 	int ret;
 	struct page *page;
 	unsigned long i;
 	char *kaddr;
 	u32 csum;
-	u32 *cb_sum = &cb->sums;
+	u8 *cb_sum = cb->sums;
 
 	if (inode->flags & BTRFS_INODE_NODATASUM)
 		return 0;
@@ -76,13 +78,13 @@ static int check_compressed_csum(struct btrfs_inode *inode,
 		btrfs_csum_final(csum, (u8 *)&csum);
 		kunmap_atomic(kaddr);
 
-		if (csum != *cb_sum) {
+		if (memcmp(&csum, cb_sum, csum_size)) {
 			btrfs_print_data_csum_error(inode, disk_start, csum,
-					*cb_sum, cb->mirror_num);
+					*(u32 *)cb_sum, cb->mirror_num);
 			ret = -EIO;
 			goto fail;
 		}
-		cb_sum++;
+		cb_sum += csum_size;
 
 	}
 	ret = 0;
@@ -537,7 +539,8 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 	struct extent_map *em;
 	blk_status_t ret = BLK_STS_RESOURCE;
 	int faili = 0;
-	u32 *sums;
+	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	u8 *sums;
 
 	em_tree = &BTRFS_I(inode)->extent_tree;
 
@@ -559,7 +562,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 	cb->errors = 0;
 	cb->inode = inode;
 	cb->mirror_num = mirror_num;
-	sums = &cb->sums;
+	sums = cb->sums;
 
 	cb->start = em->orig_start;
 	em_len = em->len;
@@ -618,6 +621,8 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 		page->mapping = NULL;
 		if (submit || bio_add_page(comp_bio, page, PAGE_SIZE, 0) <
 		    PAGE_SIZE) {
+			unsigned int nr_sectors;
+
 			ret = btrfs_bio_wq_end_io(fs_info, comp_bio,
 						  BTRFS_WQ_ENDIO_DATA);
 			BUG_ON(ret); /* -ENOMEM */
@@ -632,11 +637,13 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 
 			if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
 				ret = btrfs_lookup_bio_sums(inode, comp_bio,
-							    (u8 *)sums);
+							    sums);
 				BUG_ON(ret); /* -ENOMEM */
 			}
-			sums += DIV_ROUND_UP(comp_bio->bi_iter.bi_size,
-					     fs_info->sectorsize);
+
+			nr_sectors = DIV_ROUND_UP(comp_bio->bi_iter.bi_size,
+						  fs_info->sectorsize);
+			sums += csum_size * nr_sectors;
 
 			ret = btrfs_map_bio(fs_info, comp_bio, mirror_num, 0);
 			if (ret) {
@@ -658,7 +665,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
 	BUG_ON(ret); /* -ENOMEM */
 
 	if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
-		ret = btrfs_lookup_bio_sums(inode, comp_bio, (u8 *) sums);
+		ret = btrfs_lookup_bio_sums(inode, comp_bio, sums);
 		BUG_ON(ret); /* -ENOMEM */
 	}
 
diff --git a/fs/btrfs/compression.h b/fs/btrfs/compression.h
index 9976fe0f7526..191e5f4e3523 100644
--- a/fs/btrfs/compression.h
+++ b/fs/btrfs/compression.h
@@ -61,7 +61,7 @@ struct compressed_bio {
 	 * the start of a variable length array of checksums only
 	 * used by reads
 	 */
-	u32 sums;
+	u8 sums[];
 };
 
 static inline unsigned int btrfs_compress_type(unsigned int type_level)
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 210ff69917a0..c551479afa63 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -182,7 +182,7 @@ static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio
 		}
 		csum = btrfs_bio->csum;
 	} else {
-		csum = (u8 *)dst;
+		csum = dst;
 	}
 
 	if (bio->bi_iter.bi_size > PAGE_SIZE * 8)
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 6f7a18148dcb..a65e5f32160b 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -927,9 +927,10 @@ int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
 			   u8 *sum, int len)
 {
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
+	struct btrfs_inode *btrfs_inode = BTRFS_I(inode);
 	struct btrfs_ordered_sum *ordered_sum;
 	struct btrfs_ordered_extent *ordered;
-	struct btrfs_ordered_inode_tree *tree = &BTRFS_I(inode)->ordered_tree;
+	struct btrfs_ordered_inode_tree *tree = &btrfs_inode->ordered_tree;
 	unsigned long num_sectors;
 	unsigned long i;
 	u32 sectorsize = btrfs_inode_sectorsize(inode);
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 07/17] btrfs: use btrfs_crc32c{,_final}() in for free space cache
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
                   ` (5 preceding siblings ...)
  2019-05-10 11:15 ` [PATCH 06/17] btrfs: dont assume compressed_bio " Johannes Thumshirn
@ 2019-05-10 11:15 ` Johannes Thumshirn
  2019-05-10 13:27   ` Nikolay Borisov
  2019-05-10 11:15 ` [PATCH 08/17] btrfs: format checksums according to type for printing Johannes Thumshirn
                   ` (10 subsequent siblings)
  17 siblings, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

The CRC checksum in the free space cache is not dependant on the super
block's csum_type field but always a CRC32C.

So use btrfs_crc32c() and btrfs_crc32c_final() instead of btrfs_csum_data()
and btrfs_csum_final() for computing these checksums.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/free-space-cache.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index f74dc259307b..26ed8ed60722 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -465,9 +465,8 @@ static void io_ctl_set_crc(struct btrfs_io_ctl *io_ctl, int index)
 	if (index == 0)
 		offset = sizeof(u32) * io_ctl->num_pages;
 
-	crc = btrfs_csum_data(io_ctl->orig + offset, crc,
-			      PAGE_SIZE - offset);
-	btrfs_csum_final(crc, (u8 *)&crc);
+	crc = btrfs_crc32c(crc, io_ctl->orig + offset, PAGE_SIZE - offset);
+	btrfs_crc32c_final(crc, (u8 *)&crc);
 	io_ctl_unmap_page(io_ctl);
 	tmp = page_address(io_ctl->pages[0]);
 	tmp += index;
@@ -493,9 +492,8 @@ static int io_ctl_check_crc(struct btrfs_io_ctl *io_ctl, int index)
 	val = *tmp;
 
 	io_ctl_map_page(io_ctl, 0);
-	crc = btrfs_csum_data(io_ctl->orig + offset, crc,
-			      PAGE_SIZE - offset);
-	btrfs_csum_final(crc, (u8 *)&crc);
+	crc = btrfs_crc32c(crc, io_ctl->orig + offset, PAGE_SIZE - offset);
+	btrfs_crc32c_final(crc, (u8 *)&crc);
 	if (val != crc) {
 		btrfs_err_rl(io_ctl->fs_info,
 			"csum mismatch on free space cache");
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 08/17] btrfs: format checksums according to type for printing
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
                   ` (6 preceding siblings ...)
  2019-05-10 11:15 ` [PATCH 07/17] btrfs: use btrfs_crc32c{,_final}() in for free space cache Johannes Thumshirn
@ 2019-05-10 11:15 ` Johannes Thumshirn
  2019-05-10 13:28   ` Nikolay Borisov
  2019-05-10 11:15 ` [PATCH 09/17] btrfs: add common checksum type validation Johannes Thumshirn
                   ` (9 subsequent siblings)
  17 siblings, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

Add a small helper for btrfs_print_data_csum_error() which formats the
checksum according to it's type for pretty printing.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/btrfs_inode.h | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index d5b438706b77..f0a757eb5744 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -337,22 +337,42 @@ static inline void btrfs_inode_resume_unlocked_dio(struct btrfs_inode *inode)
 	clear_bit(BTRFS_INODE_READDIO_NEED_LOCK, &inode->runtime_flags);
 }
 
+static inline void btrfs_csum_format(struct btrfs_super_block *sb,
+				     u32 csum, u8 *cbuf)
+{
+	size_t size = btrfs_super_csum_size(sb) * 8;
+
+	switch (btrfs_super_csum_type(sb)) {
+	case BTRFS_CSUM_TYPE_CRC32:
+		snprintf(cbuf, size, "0x%08x", csum);
+		break;
+	default: /* can't happen -  csum type is validated at mount time */
+		break;
+	}
+}
+
 static inline void btrfs_print_data_csum_error(struct btrfs_inode *inode,
 		u64 logical_start, u32 csum, u32 csum_expected, int mirror_num)
 {
 	struct btrfs_root *root = inode->root;
+	struct btrfs_super_block *sb = root->fs_info->super_copy;
+	u8 cbuf[BTRFS_CSUM_SIZE];
+	u8 ecbuf[BTRFS_CSUM_SIZE];
+
+	btrfs_csum_format(sb, csum, cbuf);
+	btrfs_csum_format(sb, csum_expected, ecbuf);
 
 	/* Output minus objectid, which is more meaningful */
 	if (root->root_key.objectid >= BTRFS_LAST_FREE_OBJECTID)
 		btrfs_warn_rl(root->fs_info,
-	"csum failed root %lld ino %lld off %llu csum 0x%08x expected csum 0x%08x mirror %d",
+	"csum failed root %lld ino %lld off %llu csum %s expected csum %s mirror %d",
 			root->root_key.objectid, btrfs_ino(inode),
-			logical_start, csum, csum_expected, mirror_num);
+			logical_start, cbuf, ecbuf, mirror_num);
 	else
 		btrfs_warn_rl(root->fs_info,
-	"csum failed root %llu ino %llu off %llu csum 0x%08x expected csum 0x%08x mirror %d",
+	"csum failed root %llu ino %llu off %llu csum %s expected csum %s mirror %d",
 			root->root_key.objectid, btrfs_ino(inode),
-			logical_start, csum, csum_expected, mirror_num);
+			logical_start, cbuf, ecbuf, mirror_num);
 }
 
 #endif
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 09/17] btrfs: add common checksum type validation
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
                   ` (7 preceding siblings ...)
  2019-05-10 11:15 ` [PATCH 08/17] btrfs: format checksums according to type for printing Johannes Thumshirn
@ 2019-05-10 11:15 ` Johannes Thumshirn
  2019-05-10 13:37   ` Nikolay Borisov
  2019-05-10 11:15 ` [PATCH 10/17] btrfs: check for supported superblock checksum type before checksum validation Johannes Thumshirn
                   ` (8 subsequent siblings)
  17 siblings, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

Currently btrfs is only supporting CRC32C as checksumming algorithm. As
this is about to change provide a function to validate the checksum type in
the superblock against all possible algorithms.

This makes adding new algorithms easier as there are fewer places to adjust
when adding new algorithms.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/disk-io.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 663efce22d98..ab13282d91d2 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -356,6 +356,16 @@ static int verify_parent_transid(struct extent_io_tree *io_tree,
 	return ret;
 }
 
+static bool btrfs_supported_super_csum(struct btrfs_super_block *sb)
+{
+	switch (btrfs_super_csum_type(sb)) {
+	case BTRFS_CSUM_TYPE_CRC32:
+		return true;
+	default:
+		return false;
+	}
+}
+
 /*
  * Return 0 if the superblock checksum type matches the checksum value of that
  * algorithm. Pass the raw disk superblock data.
@@ -368,6 +378,12 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
 	u16 csum_type = btrfs_super_csum_type(disk_sb);
 	int ret = 0;
 
+	if (!btrfs_supported_super_csum(disk_sb)) {
+		btrfs_err(fs_info, "unsupported checksum algorithm %u",
+			  csum_type);
+		ret = 1;
+	}
+
 	if (csum_type == BTRFS_CSUM_TYPE_CRC32) {
 		u32 crc = ~(u32)0;
 		char result[sizeof(crc)];
@@ -385,12 +401,6 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
 			ret = 1;
 	}
 
-	if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
-		btrfs_err(fs_info, "unsupported checksum algorithm %u",
-				csum_type);
-		ret = 1;
-	}
-
 	return ret;
 }
 
@@ -2577,7 +2587,7 @@ static int btrfs_validate_write_super(struct btrfs_fs_info *fs_info,
 	ret = validate_super(fs_info, sb, -1);
 	if (ret < 0)
 		goto out;
-	if (btrfs_super_csum_type(sb) != BTRFS_CSUM_TYPE_CRC32) {
+	if (!btrfs_supported_super_csum(sb)) {
 		ret = -EUCLEAN;
 		btrfs_err(fs_info, "invalid csum type, has %u want %u",
 			  btrfs_super_csum_type(sb), BTRFS_CSUM_TYPE_CRC32);
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 10/17] btrfs: check for supported superblock checksum type before checksum validation
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
                   ` (8 preceding siblings ...)
  2019-05-10 11:15 ` [PATCH 09/17] btrfs: add common checksum type validation Johannes Thumshirn
@ 2019-05-10 11:15 ` Johannes Thumshirn
  2019-05-10 13:37   ` Nikolay Borisov
  2019-05-10 11:15 ` [PATCH 11/17] btrfs: Simplify btrfs_check_super_csum() and get rid of size assumptions Johannes Thumshirn
                   ` (7 subsequent siblings)
  17 siblings, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

Check for supported superblock checksum type before doing the actual
checksum validation of the superblock read from disk.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/disk-io.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index ab13282d91d2..74937effaed4 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2822,6 +2822,14 @@ int open_ctree(struct super_block *sb,
 		goto fail_alloc;
 	}
 
+	if (!btrfs_supported_super_csum((struct btrfs_super_block *)
+					bh->b_data)) {
+		btrfs_err(fs_info, "unsupported checksum algorithm");
+		err = -EINVAL;
+		brelse(bh);
+		goto fail_alloc;
+	}
+
 	/*
 	 * We want to check superblock checksum, the type is stored inside.
 	 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 11/17] btrfs: Simplify btrfs_check_super_csum() and get rid of size assumptions
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
                   ` (9 preceding siblings ...)
  2019-05-10 11:15 ` [PATCH 10/17] btrfs: check for supported superblock checksum type before checksum validation Johannes Thumshirn
@ 2019-05-10 11:15 ` Johannes Thumshirn
  2019-05-10 13:41   ` Nikolay Borisov
  2019-05-10 11:15 ` [PATCH 12/17] btrfs: add boilerplate code for directly including the crypto framework Johannes Thumshirn
                   ` (6 subsequent siblings)
  17 siblings, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

Now that we have already checked for a valid checksum type before calling
btrfs_check_super_csum(), it can be simplified even further.

While at it get rid of the implicit size assumption of the resulting
checksum as well.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/disk-io.c | 37 +++++++++++++------------------------
 1 file changed, 13 insertions(+), 24 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 74937effaed4..21ae9daf52b7 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -375,33 +375,22 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
 {
 	struct btrfs_super_block *disk_sb =
 		(struct btrfs_super_block *)raw_disk_sb;
-	u16 csum_type = btrfs_super_csum_type(disk_sb);
-	int ret = 0;
-
-	if (!btrfs_supported_super_csum(disk_sb)) {
-		btrfs_err(fs_info, "unsupported checksum algorithm %u",
-			  csum_type);
-		ret = 1;
-	}
-
-	if (csum_type == BTRFS_CSUM_TYPE_CRC32) {
-		u32 crc = ~(u32)0;
-		char result[sizeof(crc)];
+	u32 crc = ~(u32)0;
+	char result[BTRFS_CSUM_SIZE];
 
-		/*
-		 * The super_block structure does not span the whole
-		 * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
-		 * is filled with zeros and is included in the checksum.
-		 */
-		crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
-				crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
-		btrfs_csum_final(crc, result);
+	/*
+	 * The super_block structure does not span the whole
+	 * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
+	 * is filled with zeros and is included in the checksum.
+	 */
+	crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
+			      crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
+	btrfs_csum_final(crc, result);
 
-		if (memcmp(raw_disk_sb, result, sizeof(result)))
-			ret = 1;
-	}
+	if (memcmp(raw_disk_sb, result, btrfs_super_csum_size(disk_sb)))
+		return 1;
 
-	return ret;
+	return 0;
 }
 
 int btrfs_verify_level_key(struct extent_buffer *eb, int level,
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 12/17] btrfs: add boilerplate code for directly including the crypto framework
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
                   ` (10 preceding siblings ...)
  2019-05-10 11:15 ` [PATCH 11/17] btrfs: Simplify btrfs_check_super_csum() and get rid of size assumptions Johannes Thumshirn
@ 2019-05-10 11:15 ` Johannes Thumshirn
  2019-05-10 16:28   ` Nikolay Borisov
  2019-05-10 11:15 ` [PATCH 13/17] btrfs: pass in an fs_info to btrfs_csum_{data,final}() Johannes Thumshirn
                   ` (5 subsequent siblings)
  17 siblings, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

Add boilerplate code for directly including the crypto framework.

This helps us flipping the switch for new algorithms.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/ctree.h   | 11 +++++++++++
 fs/btrfs/disk-io.c | 49 ++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index e62934fb8748..8733c55ed686 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -73,6 +73,7 @@ struct btrfs_ref;
 
 /* four bytes for CRC32 */
 static const int btrfs_csum_sizes[] = { 4 };
+static char *btrfs_csum_names[] = { "crc32c" };
 
 #define BTRFS_EMPTY_DIR_SIZE 0
 
@@ -1165,6 +1166,8 @@ struct btrfs_fs_info {
 	spinlock_t ref_verify_lock;
 	struct rb_root block_tree;
 #endif
+
+	struct crypto_shash *csum_shash;
 };
 
 static inline struct btrfs_fs_info *btrfs_sb(struct super_block *sb)
@@ -2452,6 +2455,14 @@ static inline int btrfs_super_csum_size(const struct btrfs_super_block *s)
 	return btrfs_csum_sizes[t];
 }
 
+static inline char *btrfs_super_csum_name(const struct btrfs_super_block *s)
+{
+	u16 t = btrfs_super_csum_type(s);
+	/*
+	 * csum type is validated at mount time
+	 */
+	return btrfs_csum_names[t];
+}
 
 /*
  * The leaf data grows from end-to-front in the node.
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 21ae9daf52b7..d57adf3eaa85 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -19,6 +19,7 @@
 #include <linux/crc32c.h>
 #include <linux/sched/mm.h>
 #include <asm/unaligned.h>
+#include <crypto/hash.h>
 #include "ctree.h"
 #include "disk-io.h"
 #include "transaction.h"
@@ -2261,6 +2262,30 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info,
 	return 0;
 }
 
+static int btrfs_init_csum_hash(struct btrfs_fs_info *fs_info,
+				struct btrfs_super_block *sb)
+{
+	struct crypto_shash *csum_shash;
+	const char *csum_name = btrfs_super_csum_name(sb);
+
+	csum_shash = crypto_alloc_shash(csum_name, 0, 0);
+
+	if (IS_ERR(csum_shash)) {
+		btrfs_err(fs_info, "error allocating %s hash for checksum\n",
+			  csum_name);
+		return PTR_ERR(csum_shash);
+	}
+
+	fs_info->csum_shash = csum_shash;
+
+	return 0;
+}
+
+static void btrfs_free_csum_hash(struct btrfs_fs_info *fs_info)
+{
+	crypto_free_shash(fs_info->csum_shash);
+}
+
 static int btrfs_replay_log(struct btrfs_fs_info *fs_info,
 			    struct btrfs_fs_devices *fs_devices)
 {
@@ -2819,6 +2844,14 @@ int open_ctree(struct super_block *sb,
 		goto fail_alloc;
 	}
 
+
+	ret = btrfs_init_csum_hash(fs_info, (struct btrfs_super_block *)
+							   bh->b_data);
+	if (ret) {
+		err = ret;
+		goto fail_alloc;
+	}
+
 	/*
 	 * We want to check superblock checksum, the type is stored inside.
 	 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
@@ -2827,7 +2860,7 @@ int open_ctree(struct super_block *sb,
 		btrfs_err(fs_info, "superblock checksum mismatch");
 		err = -EINVAL;
 		brelse(bh);
-		goto fail_alloc;
+		goto fail_csum;
 	}
 
 	/*
@@ -2864,11 +2897,11 @@ int open_ctree(struct super_block *sb,
 	if (ret) {
 		btrfs_err(fs_info, "superblock contains fatal errors");
 		err = -EINVAL;
-		goto fail_alloc;
+		goto fail_csum;
 	}
 
 	if (!btrfs_super_root(disk_super))
-		goto fail_alloc;
+		goto fail_csum;
 
 	/* check FS state, whether FS is broken. */
 	if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_ERROR)
@@ -2890,7 +2923,7 @@ int open_ctree(struct super_block *sb,
 	ret = btrfs_parse_options(fs_info, options, sb->s_flags);
 	if (ret) {
 		err = ret;
-		goto fail_alloc;
+		goto fail_csum;
 	}
 
 	features = btrfs_super_incompat_flags(disk_super) &
@@ -2900,7 +2933,7 @@ int open_ctree(struct super_block *sb,
 		    "cannot mount because of unsupported optional features (%llx)",
 		    features);
 		err = -EINVAL;
-		goto fail_alloc;
+		goto fail_csum;
 	}
 
 	features = btrfs_super_incompat_flags(disk_super);
@@ -2944,7 +2977,7 @@ int open_ctree(struct super_block *sb,
 		btrfs_err(fs_info,
 "unequal nodesize/sectorsize (%u != %u) are not allowed for mixed block groups",
 			nodesize, sectorsize);
-		goto fail_alloc;
+		goto fail_csum;
 	}
 
 	/*
@@ -2960,7 +2993,7 @@ int open_ctree(struct super_block *sb,
 	"cannot mount read-write because of unsupported optional features (%llx)",
 		       features);
 		err = -EINVAL;
-		goto fail_alloc;
+		goto fail_csum;
 	}
 
 	ret = btrfs_init_workqueues(fs_info, fs_devices);
@@ -3338,6 +3371,8 @@ int open_ctree(struct super_block *sb,
 fail_sb_buffer:
 	btrfs_stop_all_workers(fs_info);
 	btrfs_free_block_groups(fs_info);
+fail_csum:
+	btrfs_free_csum_hash(fs_info);
 fail_alloc:
 fail_iput:
 	btrfs_mapping_tree_free(&fs_info->mapping_tree);
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 13/17] btrfs: pass in an fs_info to btrfs_csum_{data,final}()
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
                   ` (11 preceding siblings ...)
  2019-05-10 11:15 ` [PATCH 12/17] btrfs: add boilerplate code for directly including the crypto framework Johannes Thumshirn
@ 2019-05-10 11:15 ` Johannes Thumshirn
  2019-05-10 11:15 ` [PATCH 14/17] btrfs: directly call into crypto framework for checsumming Johannes Thumshirn
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

Later patches will need the fs_info to get the checksum function in
btrfs_csum_data() and btrfs_csum_final().

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/check-integrity.c |  4 ++--
 fs/btrfs/compression.c     |  4 ++--
 fs/btrfs/disk-io.c         | 18 ++++++++++--------
 fs/btrfs/disk-io.h         |  5 +++--
 fs/btrfs/file-item.c       |  4 ++--
 fs/btrfs/inode.c           |  5 +++--
 fs/btrfs/scrub.c           | 12 ++++++------
 7 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index 85774e2fa3e5..6aad0c3d197f 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -1728,9 +1728,9 @@ static int btrfsic_test_for_metadata(struct btrfsic_state *state,
 		size_t sublen = i ? PAGE_SIZE :
 				    (PAGE_SIZE - BTRFS_CSUM_SIZE);
 
-		crc = btrfs_csum_data(data, crc, sublen);
+		crc = btrfs_csum_data(fs_info, data, crc, sublen);
 	}
-	btrfs_csum_final(crc, csum);
+	btrfs_csum_final(fs_info, crc, csum);
 	if (memcmp(csum, h->csum, state->csum_size))
 		return 1;
 
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index d5642f3b5c44..e63bd29e5a27 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -74,8 +74,8 @@ static int check_compressed_csum(struct btrfs_inode *inode,
 		csum = ~(u32)0;
 
 		kaddr = kmap_atomic(page);
-		csum = btrfs_csum_data(kaddr, csum, PAGE_SIZE);
-		btrfs_csum_final(csum, (u8 *)&csum);
+		csum = btrfs_csum_data(fs_info, kaddr, csum, PAGE_SIZE);
+		btrfs_csum_final(fs_info, csum, (u8 *)&csum);
 		kunmap_atomic(kaddr);
 
 		if (memcmp(&csum, cb_sum, csum_size)) {
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index d57adf3eaa85..fb0b06b7b9f6 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -250,12 +250,13 @@ struct extent_map *btree_get_extent(struct btrfs_inode *inode,
 	return em;
 }
 
-u32 btrfs_csum_data(const char *data, u32 seed, size_t len)
+u32 btrfs_csum_data(struct btrfs_fs_info *fs_info, const char *data,
+		    u32 seed, size_t len)
 {
 	return crc32c(seed, data, len);
 }
 
-void btrfs_csum_final(u32 crc, u8 *result)
+void btrfs_csum_final(struct btrfs_fs_info *fs_info, u32 crc, u8 *result)
 {
 	put_unaligned_le32(~crc, result);
 }
@@ -289,14 +290,14 @@ static int csum_tree_block(struct extent_buffer *buf, u8 *result)
 		if (WARN_ON(err))
 			return err;
 		cur_len = min(len, map_len - (offset - map_start));
-		crc = btrfs_csum_data(kaddr + offset - map_start,
+		crc = btrfs_csum_data(buf->fs_info, kaddr + offset - map_start,
 				      crc, cur_len);
 		len -= cur_len;
 		offset += cur_len;
 	}
 	memset(result, 0, BTRFS_CSUM_SIZE);
 
-	btrfs_csum_final(crc, result);
+	btrfs_csum_final(buf->fs_info, crc, result);
 
 	return 0;
 }
@@ -384,9 +385,9 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
 	 * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
 	 * is filled with zeros and is included in the checksum.
 	 */
-	crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
+	crc = btrfs_csum_data(fs_info, raw_disk_sb + BTRFS_CSUM_SIZE,
 			      crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
-	btrfs_csum_final(crc, result);
+	btrfs_csum_final(fs_info, crc, result);
 
 	if (memcmp(raw_disk_sb, result, btrfs_super_csum_size(disk_sb)))
 		return 1;
@@ -3534,9 +3535,10 @@ static int write_dev_supers(struct btrfs_device *device,
 		btrfs_set_super_bytenr(sb, bytenr);
 
 		crc = ~(u32)0;
-		crc = btrfs_csum_data((const char *)sb + BTRFS_CSUM_SIZE, crc,
+		crc = btrfs_csum_data(device->fs_info,
+				      (const char *)sb + BTRFS_CSUM_SIZE, crc,
 				      BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
-		btrfs_csum_final(crc, sb->csum);
+		btrfs_csum_final(device->fs_info, crc, sb->csum);
 
 		/* One reference for us, and we leave it for the caller */
 		bh = __getblk(device->bdev, bytenr / BTRFS_BDEV_BLOCKSIZE,
diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
index a0161aa1ea0b..434c4d402326 100644
--- a/fs/btrfs/disk-io.h
+++ b/fs/btrfs/disk-io.h
@@ -115,8 +115,9 @@ int btrfs_buffer_uptodate(struct extent_buffer *buf, u64 parent_transid,
 			  int atomic);
 int btrfs_read_buffer(struct extent_buffer *buf, u64 parent_transid, int level,
 		      struct btrfs_key *first_key);
-u32 btrfs_csum_data(const char *data, u32 seed, size_t len);
-void btrfs_csum_final(u32 crc, u8 *result);
+u32 btrfs_csum_data(struct btrfs_fs_info *fs_info, const char *data,
+		    u32 seed, size_t len);
+void btrfs_csum_final(struct btrfs_fs_info *fs_info, u32 crc, u8 *result);
 blk_status_t btrfs_bio_wq_end_io(struct btrfs_fs_info *info, struct bio *bio,
 			enum btrfs_wq_endio_type metadata);
 blk_status_t btrfs_wq_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index c551479afa63..8ff939ebb118 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -503,12 +503,12 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
 
 			memset(&sums->sums[index], 0xff, csum_size);
 			data = kmap_atomic(bvec.bv_page);
-			tmp = btrfs_csum_data(data + bvec.bv_offset
+			tmp = btrfs_csum_data(fs_info, data + bvec.bv_offset
 						+ (i * fs_info->sectorsize),
 						*(u32 *)&sums->sums[index],
 						fs_info->sectorsize);
 			kunmap_atomic(data);
-			btrfs_csum_final(tmp,
+			btrfs_csum_final(fs_info, tmp,
 					(char *)(sums->sums + index));
 			index += csum_size;
 			offset += fs_info->sectorsize;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index db19f113d514..578d1d696889 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3203,6 +3203,7 @@ static int __readpage_endio_check(struct inode *inode,
 				  int icsum, struct page *page,
 				  int pgoff, u64 start, size_t len)
 {
+	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
 	char *kaddr;
 	u32 csum_expected;
 	u32 csum = ~(u32)0;
@@ -3210,8 +3211,8 @@ static int __readpage_endio_check(struct inode *inode,
 	csum_expected = *(((u32 *)io_bio->csum) + icsum);
 
 	kaddr = kmap_atomic(page);
-	csum = btrfs_csum_data(kaddr + pgoff, csum,  len);
-	btrfs_csum_final(csum, (u8 *)&csum);
+	csum = btrfs_csum_data(fs_info, kaddr + pgoff, csum,  len);
+	btrfs_csum_final(fs_info, csum, (u8 *)&csum);
 	if (csum != csum_expected)
 		goto zeroit;
 
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 2cf3cf9e9c9b..b8fd4edeee53 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -1808,7 +1808,7 @@ static int scrub_checksum_data(struct scrub_block *sblock)
 	for (;;) {
 		u64 l = min_t(u64, len, PAGE_SIZE);
 
-		crc = btrfs_csum_data(buffer, crc, l);
+		crc = btrfs_csum_data(sctx->fs_info, buffer, crc, l);
 		kunmap_atomic(buffer);
 		len -= l;
 		if (len == 0)
@@ -1820,7 +1820,7 @@ static int scrub_checksum_data(struct scrub_block *sblock)
 		buffer = kmap_atomic(page);
 	}
 
-	btrfs_csum_final(crc, csum);
+	btrfs_csum_final(sctx->fs_info, crc, csum);
 	if (memcmp(csum, on_disk_csum, sctx->csum_size))
 		sblock->checksum_error = 1;
 
@@ -1875,7 +1875,7 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
 	for (;;) {
 		u64 l = min_t(u64, len, mapped_size);
 
-		crc = btrfs_csum_data(p, crc, l);
+		crc = btrfs_csum_data(fs_info, p, crc, l);
 		kunmap_atomic(mapped_buffer);
 		len -= l;
 		if (len == 0)
@@ -1889,7 +1889,7 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
 		p = mapped_buffer;
 	}
 
-	btrfs_csum_final(crc, calculated_csum);
+	btrfs_csum_final(fs_info, crc, calculated_csum);
 	if (memcmp(calculated_csum, on_disk_csum, sctx->csum_size))
 		sblock->checksum_error = 1;
 
@@ -1934,7 +1934,7 @@ static int scrub_checksum_super(struct scrub_block *sblock)
 	for (;;) {
 		u64 l = min_t(u64, len, mapped_size);
 
-		crc = btrfs_csum_data(p, crc, l);
+		crc = btrfs_csum_data(sctx->fs_info, p, crc, l);
 		kunmap_atomic(mapped_buffer);
 		len -= l;
 		if (len == 0)
@@ -1948,7 +1948,7 @@ static int scrub_checksum_super(struct scrub_block *sblock)
 		p = mapped_buffer;
 	}
 
-	btrfs_csum_final(crc, calculated_csum);
+	btrfs_csum_final(sctx->fs_info, crc, calculated_csum);
 	if (memcmp(calculated_csum, on_disk_csum, sctx->csum_size))
 		++fail_cor;
 
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 14/17] btrfs: directly call into crypto framework for checsumming
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
                   ` (12 preceding siblings ...)
  2019-05-10 11:15 ` [PATCH 13/17] btrfs: pass in an fs_info to btrfs_csum_{data,final}() Johannes Thumshirn
@ 2019-05-10 11:15 ` Johannes Thumshirn
  2019-05-10 13:45   ` Chris Mason
  2019-05-13 13:00   ` David Sterba
  2019-05-10 11:15 ` [PATCH 15/17] btrfs: remove assumption about csum type form btrfs_csum_{data,final}() Johannes Thumshirn
                   ` (3 subsequent siblings)
  17 siblings, 2 replies; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

Currently btrfs_csum_data() relied on the crc32c() wrapper around the crypto
framework for calculating the CRCs.

As we have our own crypto_shash structure in the fs_info now, we can
directly call into the crypto framework without going trough the wrapper.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/disk-io.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index fb0b06b7b9f6..0c9ac7b45ce8 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -253,12 +253,36 @@ struct extent_map *btree_get_extent(struct btrfs_inode *inode,
 u32 btrfs_csum_data(struct btrfs_fs_info *fs_info, const char *data,
 		    u32 seed, size_t len)
 {
-	return crc32c(seed, data, len);
+	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
+	u32 *ctx = (u32 *)shash_desc_ctx(shash);
+	u32 retval;
+	int err;
+
+	shash->tfm = fs_info->csum_shash;
+	shash->flags = 0;
+	*ctx = seed;
+
+	err = crypto_shash_update(shash, data, len);
+	ASSERT(err);
+
+	retval = *ctx;
+	barrier_data(ctx);
+
+	return retval;
 }
 
 void btrfs_csum_final(struct btrfs_fs_info *fs_info, u32 crc, u8 *result)
 {
-	put_unaligned_le32(~crc, result);
+	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
+	u32 *ctx = (u32 *)shash_desc_ctx(shash);
+	int err;
+
+	shash->tfm = fs_info->csum_shash;
+	shash->flags = 0;
+	*ctx = crc;
+
+	err = crypto_shash_final(shash, result);
+	ASSERT(err);
 }
 
 /*
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 15/17] btrfs: remove assumption about csum type form btrfs_csum_{data,final}()
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
                   ` (13 preceding siblings ...)
  2019-05-10 11:15 ` [PATCH 14/17] btrfs: directly call into crypto framework for checsumming Johannes Thumshirn
@ 2019-05-10 11:15 ` Johannes Thumshirn
  2019-05-13 12:56   ` David Sterba
  2019-05-10 11:15 ` [PATCH 16/17] btrfs: remove assumption about csum type form btrfs_print_data_csum_error() Johannes Thumshirn
                   ` (2 subsequent siblings)
  17 siblings, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

btrfs_csum_data() and btrfs_csum_final() still have assumptions on the
checksums' type and size. Remove it so we can plumb in more types.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/check-integrity.c |  6 ++---
 fs/btrfs/compression.c     | 13 ++++++-----
 fs/btrfs/disk-io.c         | 58 +++++++++++++++++++++++++---------------------
 fs/btrfs/disk-io.h         |  6 ++---
 fs/btrfs/file-item.c       | 13 +++++------
 fs/btrfs/inode.c           | 18 +++++++-------
 fs/btrfs/scrub.c           | 20 +++++++++-------
 7 files changed, 72 insertions(+), 62 deletions(-)

diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
index 6aad0c3d197f..54c8a549b505 100644
--- a/fs/btrfs/check-integrity.c
+++ b/fs/btrfs/check-integrity.c
@@ -1712,7 +1712,6 @@ static int btrfsic_test_for_metadata(struct btrfsic_state *state,
 	struct btrfs_fs_info *fs_info = state->fs_info;
 	struct btrfs_header *h;
 	u8 csum[BTRFS_CSUM_SIZE];
-	u32 crc = ~(u32)0;
 	unsigned int i;
 
 	if (num_pages * PAGE_SIZE < state->metablock_size)
@@ -1723,14 +1722,15 @@ static int btrfsic_test_for_metadata(struct btrfsic_state *state,
 	if (memcmp(h->fsid, fs_info->fs_devices->fsid, BTRFS_FSID_SIZE))
 		return 1;
 
+	memset(csum, 0xff, btrfs_super_csum_size(fs_info->super_copy));
 	for (i = 0; i < num_pages; i++) {
 		u8 *data = i ? datav[i] : (datav[i] + BTRFS_CSUM_SIZE);
 		size_t sublen = i ? PAGE_SIZE :
 				    (PAGE_SIZE - BTRFS_CSUM_SIZE);
 
-		crc = btrfs_csum_data(fs_info, data, crc, sublen);
+		btrfs_csum_data(fs_info, data, csum, sublen);
 	}
-	btrfs_csum_final(fs_info, crc, csum);
+	btrfs_csum_final(fs_info, csum, csum);
 	if (memcmp(csum, h->csum, state->csum_size))
 		return 1;
 
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index e63bd29e5a27..bf36cdb641ef 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -63,7 +63,7 @@ static int check_compressed_csum(struct btrfs_inode *inode,
 	struct page *page;
 	unsigned long i;
 	char *kaddr;
-	u32 csum;
+	u8 csum[BTRFS_CSUM_SIZE];
 	u8 *cb_sum = cb->sums;
 
 	if (inode->flags & BTRFS_INODE_NODATASUM)
@@ -71,16 +71,17 @@ static int check_compressed_csum(struct btrfs_inode *inode,
 
 	for (i = 0; i < cb->nr_pages; i++) {
 		page = cb->compressed_pages[i];
-		csum = ~(u32)0;
+		memset(csum, 0xff, csum_size);
 
 		kaddr = kmap_atomic(page);
-		csum = btrfs_csum_data(fs_info, kaddr, csum, PAGE_SIZE);
-		btrfs_csum_final(fs_info, csum, (u8 *)&csum);
+		btrfs_csum_data(fs_info, kaddr, csum, PAGE_SIZE);
+		btrfs_csum_final(fs_info, csum, csum);
 		kunmap_atomic(kaddr);
 
 		if (memcmp(&csum, cb_sum, csum_size)) {
-			btrfs_print_data_csum_error(inode, disk_start, csum,
-					*(u32 *)cb_sum, cb->mirror_num);
+			btrfs_print_data_csum_error(inode, disk_start,
+						    *(u32 *)csum, *(u32 *)cb_sum,
+						    cb->mirror_num);
 			ret = -EIO;
 			goto fail;
 		}
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 0c9ac7b45ce8..2be8f05be1e6 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -250,36 +250,35 @@ struct extent_map *btree_get_extent(struct btrfs_inode *inode,
 	return em;
 }
 
-u32 btrfs_csum_data(struct btrfs_fs_info *fs_info, const char *data,
-		    u32 seed, size_t len)
+void btrfs_csum_data(struct btrfs_fs_info *fs_info, const char *data,
+		    u8 *seed, size_t len)
 {
 	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
-	u32 *ctx = (u32 *)shash_desc_ctx(shash);
-	u32 retval;
+	u8 *ctx = shash_desc_ctx(shash);
+	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
 	int err;
 
 	shash->tfm = fs_info->csum_shash;
 	shash->flags = 0;
-	*ctx = seed;
+	memcpy(ctx, seed, csum_size);
 
 	err = crypto_shash_update(shash, data, len);
 	ASSERT(err);
 
-	retval = *ctx;
 	barrier_data(ctx);
-
-	return retval;
+	memcpy(seed, ctx, csum_size);
 }
 
-void btrfs_csum_final(struct btrfs_fs_info *fs_info, u32 crc, u8 *result)
+void btrfs_csum_final(struct btrfs_fs_info *fs_info, u8 *seed, u8 *result)
 {
 	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
-	u32 *ctx = (u32 *)shash_desc_ctx(shash);
+	u8 *ctx = shash_desc_ctx(shash);
+	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
 	int err;
 
 	shash->tfm = fs_info->csum_shash;
 	shash->flags = 0;
-	*ctx = crc;
+	memcpy(ctx, seed, csum_size);
 
 	err = crypto_shash_final(shash, result);
 	ASSERT(err);
@@ -292,6 +291,7 @@ void btrfs_csum_final(struct btrfs_fs_info *fs_info, u32 crc, u8 *result)
  */
 static int csum_tree_block(struct extent_buffer *buf, u8 *result)
 {
+	struct btrfs_fs_info *fs_info = buf->fs_info;
 	unsigned long len;
 	unsigned long cur_len;
 	unsigned long offset = BTRFS_CSUM_SIZE;
@@ -299,9 +299,12 @@ static int csum_tree_block(struct extent_buffer *buf, u8 *result)
 	unsigned long map_start;
 	unsigned long map_len;
 	int err;
-	u32 crc = ~(u32)0;
+	u8 csum[BTRFS_CSUM_SIZE];
 
 	len = buf->len - offset;
+
+	memset(csum, 0xff, btrfs_super_csum_size(fs_info->super_copy));
+
 	while (len > 0) {
 		/*
 		 * Note: we don't need to check for the err == 1 case here, as
@@ -314,14 +317,14 @@ static int csum_tree_block(struct extent_buffer *buf, u8 *result)
 		if (WARN_ON(err))
 			return err;
 		cur_len = min(len, map_len - (offset - map_start));
-		crc = btrfs_csum_data(buf->fs_info, kaddr + offset - map_start,
-				      crc, cur_len);
+		btrfs_csum_data(fs_info, kaddr + offset - map_start, csum,
+				cur_len);
 		len -= cur_len;
 		offset += cur_len;
 	}
 	memset(result, 0, BTRFS_CSUM_SIZE);
 
-	btrfs_csum_final(buf->fs_info, crc, result);
+	btrfs_csum_final(fs_info, csum, result);
 
 	return 0;
 }
@@ -401,19 +404,22 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
 {
 	struct btrfs_super_block *disk_sb =
 		(struct btrfs_super_block *)raw_disk_sb;
-	u32 crc = ~(u32)0;
+	u8 csum[BTRFS_CSUM_SIZE];
 	char result[BTRFS_CSUM_SIZE];
+	u16 csum_size = btrfs_super_csum_size(disk_sb);
+
+	memset(csum, 0xff, csum_size);
 
 	/*
 	 * The super_block structure does not span the whole
 	 * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
 	 * is filled with zeros and is included in the checksum.
 	 */
-	crc = btrfs_csum_data(fs_info, raw_disk_sb + BTRFS_CSUM_SIZE,
-			      crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
-	btrfs_csum_final(fs_info, crc, result);
+	btrfs_csum_data(fs_info, raw_disk_sb + BTRFS_CSUM_SIZE, csum,
+			BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
+	btrfs_csum_final(fs_info, csum, result);
 
-	if (memcmp(raw_disk_sb, result, btrfs_super_csum_size(disk_sb)))
+	if (memcmp(raw_disk_sb, result, csum_size))
 		return 1;
 
 	return 0;
@@ -3539,11 +3545,12 @@ struct buffer_head *btrfs_read_dev_super(struct block_device *bdev)
 static int write_dev_supers(struct btrfs_device *device,
 			    struct btrfs_super_block *sb, int max_mirrors)
 {
+	struct btrfs_fs_info *fs_info = device->fs_info;
 	struct buffer_head *bh;
 	int i;
 	int ret;
 	int errors = 0;
-	u32 crc;
+	u8 csum[BTRFS_CSUM_SIZE];
 	u64 bytenr;
 	int op_flags;
 
@@ -3558,11 +3565,10 @@ static int write_dev_supers(struct btrfs_device *device,
 
 		btrfs_set_super_bytenr(sb, bytenr);
 
-		crc = ~(u32)0;
-		crc = btrfs_csum_data(device->fs_info,
-				      (const char *)sb + BTRFS_CSUM_SIZE, crc,
-				      BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
-		btrfs_csum_final(device->fs_info, crc, sb->csum);
+		memset(csum, 0xff, btrfs_super_csum_size(fs_info->super_copy));
+		btrfs_csum_data(fs_info, (const char *)sb + BTRFS_CSUM_SIZE,
+				csum, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
+		btrfs_csum_final(fs_info, csum, sb->csum);
 
 		/* One reference for us, and we leave it for the caller */
 		bh = __getblk(device->bdev, bytenr / BTRFS_BDEV_BLOCKSIZE,
diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
index 434c4d402326..6eeb267faa11 100644
--- a/fs/btrfs/disk-io.h
+++ b/fs/btrfs/disk-io.h
@@ -115,9 +115,9 @@ int btrfs_buffer_uptodate(struct extent_buffer *buf, u64 parent_transid,
 			  int atomic);
 int btrfs_read_buffer(struct extent_buffer *buf, u64 parent_transid, int level,
 		      struct btrfs_key *first_key);
-u32 btrfs_csum_data(struct btrfs_fs_info *fs_info, const char *data,
-		    u32 seed, size_t len);
-void btrfs_csum_final(struct btrfs_fs_info *fs_info, u32 crc, u8 *result);
+void btrfs_csum_data(struct btrfs_fs_info *fs_info, const char *data,
+		     u8 *seed, size_t len);
+void btrfs_csum_final(struct btrfs_fs_info *fs_info, u8 *csum, u8 *result);
 blk_status_t btrfs_bio_wq_end_io(struct btrfs_fs_info *info, struct bio *bio,
 			enum btrfs_wq_endio_type metadata);
 blk_status_t btrfs_wq_submit_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 8ff939ebb118..11b7ffa589b3 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -475,7 +475,6 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
 						 - 1);
 
 		for (i = 0; i < nr_sectors; i++) {
-			u32 tmp;
 			if (offset >= ordered->file_offset + ordered->len ||
 				offset < ordered->file_offset) {
 				unsigned long bytes_left;
@@ -503,13 +502,13 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
 
 			memset(&sums->sums[index], 0xff, csum_size);
 			data = kmap_atomic(bvec.bv_page);
-			tmp = btrfs_csum_data(fs_info, data + bvec.bv_offset
-						+ (i * fs_info->sectorsize),
-						*(u32 *)&sums->sums[index],
-						fs_info->sectorsize);
+			btrfs_csum_data(fs_info, data + bvec.bv_offset
+					+ (i * fs_info->sectorsize),
+					&sums->sums[index],
+					fs_info->sectorsize);
 			kunmap_atomic(data);
-			btrfs_csum_final(fs_info, tmp,
-					(char *)(sums->sums + index));
+			btrfs_csum_final(fs_info, &sums->sums[index],
+					 &sums->sums[index]);
 			index += csum_size;
 			offset += fs_info->sectorsize;
 			this_sum_bytes += fs_info->sectorsize;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 578d1d696889..5dda8796ab4c 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3205,22 +3205,24 @@ static int __readpage_endio_check(struct inode *inode,
 {
 	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
 	char *kaddr;
-	u32 csum_expected;
-	u32 csum = ~(u32)0;
+	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
+	u8 *csum_expected;
+	u8 csum[BTRFS_CSUM_SIZE];
 
-	csum_expected = *(((u32 *)io_bio->csum) + icsum);
+	csum_expected = ((u8 *)io_bio->csum) + icsum * csum_size;
 
+	memset(csum, 0xff, csum_size);
 	kaddr = kmap_atomic(page);
-	csum = btrfs_csum_data(fs_info, kaddr + pgoff, csum,  len);
-	btrfs_csum_final(fs_info, csum, (u8 *)&csum);
-	if (csum != csum_expected)
+	btrfs_csum_data(fs_info, kaddr + pgoff, csum,  len);
+	btrfs_csum_final(fs_info, csum, csum);
+	if (memcmp(csum, csum_expected, csum_size))
 		goto zeroit;
 
 	kunmap_atomic(kaddr);
 	return 0;
 zeroit:
-	btrfs_print_data_csum_error(BTRFS_I(inode), start, csum, csum_expected,
-				    io_bio->mirror_num);
+	btrfs_print_data_csum_error(BTRFS_I(inode), start, *(u32 *)csum,
+				    *(u32 *)csum_expected, io_bio->mirror_num);
 	memset(kaddr + pgoff, 1, len);
 	flush_dcache_page(page);
 	kunmap_atomic(kaddr);
diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index b8fd4edeee53..f218682940e7 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -1791,7 +1791,6 @@ static int scrub_checksum_data(struct scrub_block *sblock)
 	u8 *on_disk_csum;
 	struct page *page;
 	void *buffer;
-	u32 crc = ~(u32)0;
 	u64 len;
 	int index;
 
@@ -1803,12 +1802,13 @@ static int scrub_checksum_data(struct scrub_block *sblock)
 	page = sblock->pagev[0]->page;
 	buffer = kmap_atomic(page);
 
+	memset(csum, 0xff, btrfs_super_csum_size(sctx->fs_info->super_copy));
 	len = sctx->fs_info->sectorsize;
 	index = 0;
 	for (;;) {
 		u64 l = min_t(u64, len, PAGE_SIZE);
 
-		crc = btrfs_csum_data(sctx->fs_info, buffer, crc, l);
+		btrfs_csum_data(sctx->fs_info, buffer, csum, l);
 		kunmap_atomic(buffer);
 		len -= l;
 		if (len == 0)
@@ -1820,7 +1820,7 @@ static int scrub_checksum_data(struct scrub_block *sblock)
 		buffer = kmap_atomic(page);
 	}
 
-	btrfs_csum_final(sctx->fs_info, crc, csum);
+	btrfs_csum_final(sctx->fs_info, csum, csum);
 	if (memcmp(csum, on_disk_csum, sctx->csum_size))
 		sblock->checksum_error = 1;
 
@@ -1838,7 +1838,7 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
 	void *mapped_buffer;
 	u64 mapped_size;
 	void *p;
-	u32 crc = ~(u32)0;
+	u8 csum[BTRFS_CSUM_SIZE];
 	u64 len;
 	int index;
 
@@ -1872,10 +1872,11 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
 	mapped_size = PAGE_SIZE - BTRFS_CSUM_SIZE;
 	p = ((u8 *)mapped_buffer) + BTRFS_CSUM_SIZE;
 	index = 0;
+	memset(csum, 0xff, btrfs_super_csum_size(fs_info->super_copy));
 	for (;;) {
 		u64 l = min_t(u64, len, mapped_size);
 
-		crc = btrfs_csum_data(fs_info, p, crc, l);
+		btrfs_csum_data(fs_info, p, csum, l);
 		kunmap_atomic(mapped_buffer);
 		len -= l;
 		if (len == 0)
@@ -1889,7 +1890,7 @@ static int scrub_checksum_tree_block(struct scrub_block *sblock)
 		p = mapped_buffer;
 	}
 
-	btrfs_csum_final(fs_info, crc, calculated_csum);
+	btrfs_csum_final(fs_info, csum, calculated_csum);
 	if (memcmp(calculated_csum, on_disk_csum, sctx->csum_size))
 		sblock->checksum_error = 1;
 
@@ -1906,7 +1907,7 @@ static int scrub_checksum_super(struct scrub_block *sblock)
 	void *mapped_buffer;
 	u64 mapped_size;
 	void *p;
-	u32 crc = ~(u32)0;
+	u8 csum[BTRFS_CSUM_SIZE];
 	int fail_gen = 0;
 	int fail_cor = 0;
 	u64 len;
@@ -1931,10 +1932,11 @@ static int scrub_checksum_super(struct scrub_block *sblock)
 	mapped_size = PAGE_SIZE - BTRFS_CSUM_SIZE;
 	p = ((u8 *)mapped_buffer) + BTRFS_CSUM_SIZE;
 	index = 0;
+	memset(csum, 0xff, btrfs_super_csum_size(s));
 	for (;;) {
 		u64 l = min_t(u64, len, mapped_size);
 
-		crc = btrfs_csum_data(sctx->fs_info, p, crc, l);
+		btrfs_csum_data(sctx->fs_info, p, csum, l);
 		kunmap_atomic(mapped_buffer);
 		len -= l;
 		if (len == 0)
@@ -1948,7 +1950,7 @@ static int scrub_checksum_super(struct scrub_block *sblock)
 		p = mapped_buffer;
 	}
 
-	btrfs_csum_final(sctx->fs_info, crc, calculated_csum);
+	btrfs_csum_final(sctx->fs_info, csum, calculated_csum);
 	if (memcmp(calculated_csum, on_disk_csum, sctx->csum_size))
 		++fail_cor;
 
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 16/17] btrfs: remove assumption about csum type form btrfs_print_data_csum_error()
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
                   ` (14 preceding siblings ...)
  2019-05-10 11:15 ` [PATCH 15/17] btrfs: remove assumption about csum type form btrfs_csum_{data,final}() Johannes Thumshirn
@ 2019-05-10 11:15 ` Johannes Thumshirn
  2019-05-10 11:15 ` [PATCH 17/17] btrfs: add sha256 as another checksum algorithm Johannes Thumshirn
  2019-05-15 17:27 ` [PATCH 00/17] Add support for SHA-256 checksums David Sterba
  17 siblings, 0 replies; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

btrfs_print_data_csum_error() still assumed checksums to be 32 bit in size.

Make it size agnostic.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/btrfs_inode.h | 6 +++---
 fs/btrfs/compression.c | 2 +-
 fs/btrfs/inode.c       | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index f0a757eb5744..e79fd9129075 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -338,13 +338,13 @@ static inline void btrfs_inode_resume_unlocked_dio(struct btrfs_inode *inode)
 }
 
 static inline void btrfs_csum_format(struct btrfs_super_block *sb,
-				     u32 csum, u8 *cbuf)
+				     u8 *csum, u8 *cbuf)
 {
 	size_t size = btrfs_super_csum_size(sb) * 8;
 
 	switch (btrfs_super_csum_type(sb)) {
 	case BTRFS_CSUM_TYPE_CRC32:
-		snprintf(cbuf, size, "0x%08x", csum);
+		snprintf(cbuf, size, "0x%08x", *(u32 *)csum);
 		break;
 	default: /* can't happen -  csum type is validated at mount time */
 		break;
@@ -352,7 +352,7 @@ static inline void btrfs_csum_format(struct btrfs_super_block *sb,
 }
 
 static inline void btrfs_print_data_csum_error(struct btrfs_inode *inode,
-		u64 logical_start, u32 csum, u32 csum_expected, int mirror_num)
+		u64 logical_start, u8 *csum, u8 *csum_expected, int mirror_num)
 {
 	struct btrfs_root *root = inode->root;
 	struct btrfs_super_block *sb = root->fs_info->super_copy;
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index bf36cdb641ef..d7eaaee4031f 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -80,7 +80,7 @@ static int check_compressed_csum(struct btrfs_inode *inode,
 
 		if (memcmp(&csum, cb_sum, csum_size)) {
 			btrfs_print_data_csum_error(inode, disk_start,
-						    *(u32 *)csum, *(u32 *)cb_sum,
+						    csum, cb_sum,
 						    cb->mirror_num);
 			ret = -EIO;
 			goto fail;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 5dda8796ab4c..ca47e5527af4 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3221,8 +3221,8 @@ static int __readpage_endio_check(struct inode *inode,
 	kunmap_atomic(kaddr);
 	return 0;
 zeroit:
-	btrfs_print_data_csum_error(BTRFS_I(inode), start, *(u32 *)csum,
-				    *(u32 *)csum_expected, io_bio->mirror_num);
+	btrfs_print_data_csum_error(BTRFS_I(inode), start, csum, csum_expected,
+				    io_bio->mirror_num);
 	memset(kaddr + pgoff, 1, len);
 	flush_dcache_page(page);
 	kunmap_atomic(kaddr);
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* [PATCH 17/17] btrfs: add sha256 as another checksum algorithm
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
                   ` (15 preceding siblings ...)
  2019-05-10 11:15 ` [PATCH 16/17] btrfs: remove assumption about csum type form btrfs_print_data_csum_error() Johannes Thumshirn
@ 2019-05-10 11:15 ` Johannes Thumshirn
  2019-05-10 12:30   ` Nikolay Borisov
  2019-05-15 17:27 ` [PATCH 00/17] Add support for SHA-256 checksums David Sterba
  17 siblings, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-10 11:15 UTC (permalink / raw)
  To: David Sterba; +Cc: Linux BTRFS Mailinglist, Johannes Thumshirn

Now that we everything in place, we can add SHA-256 as another checksum
algorithm.

SHA-256 was taken as it was the cryptographically strongest algorithm that
can fit into the 32 Bytes we have left.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/btrfs_inode.h          | 3 +++
 fs/btrfs/ctree.h                | 4 ++--
 fs/btrfs/disk-io.c              | 2 ++
 include/uapi/linux/btrfs_tree.h | 1 +
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index e79fd9129075..fccc372ef719 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -346,6 +346,9 @@ static inline void btrfs_csum_format(struct btrfs_super_block *sb,
 	case BTRFS_CSUM_TYPE_CRC32:
 		snprintf(cbuf, size, "0x%08x", *(u32 *)csum);
 		break;
+	case BTRFS_CSUM_TYPE_SHA256:
+		memcpy(cbuf, csum, size);
+		break;
 	default: /* can't happen -  csum type is validated at mount time */
 		break;
 	}
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 8733c55ed686..d60138208dd4 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -72,8 +72,8 @@ struct btrfs_ref;
 #define BTRFS_LINK_MAX 65535U
 
 /* four bytes for CRC32 */
-static const int btrfs_csum_sizes[] = { 4 };
-static char *btrfs_csum_names[] = { "crc32c" };
+static const int btrfs_csum_sizes[] = { 4, 32 };
+static char *btrfs_csum_names[] = { "crc32c", "sha256" };
 
 #define BTRFS_EMPTY_DIR_SIZE 0
 
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 2be8f05be1e6..bdcffa0d6b13 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -390,6 +390,8 @@ static bool btrfs_supported_super_csum(struct btrfs_super_block *sb)
 	switch (btrfs_super_csum_type(sb)) {
 	case BTRFS_CSUM_TYPE_CRC32:
 		return true;
+	case BTRFS_CSUM_TYPE_SHA256:
+		return true;
 	default:
 		return false;
 	}
diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
index 421239b98db2..3667ab4bc215 100644
--- a/include/uapi/linux/btrfs_tree.h
+++ b/include/uapi/linux/btrfs_tree.h
@@ -301,6 +301,7 @@
 
 /* csum types */
 #define BTRFS_CSUM_TYPE_CRC32	0
+#define BTRFS_CSUM_TYPE_SHA256	1
 
 /*
  * flags definitions for directory entry item type
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [PATCH 17/17] btrfs: add sha256 as another checksum algorithm
  2019-05-10 11:15 ` [PATCH 17/17] btrfs: add sha256 as another checksum algorithm Johannes Thumshirn
@ 2019-05-10 12:30   ` Nikolay Borisov
  2019-05-13  7:11     ` Johannes Thumshirn
  2019-05-13 12:55     ` David Sterba
  0 siblings, 2 replies; 60+ messages in thread
From: Nikolay Borisov @ 2019-05-10 12:30 UTC (permalink / raw)
  To: Johannes Thumshirn, David Sterba; +Cc: Linux BTRFS Mailinglist



On 10.05.19 г. 14:15 ч., Johannes Thumshirn wrote:
> Now that we everything in place, we can add SHA-256 as another checksum
> algorithm.
> 
> SHA-256 was taken as it was the cryptographically strongest algorithm that
> can fit into the 32 Bytes we have left.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

LGTM:

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/btrfs_inode.h          | 3 +++
>  fs/btrfs/ctree.h                | 4 ++--
>  fs/btrfs/disk-io.c              | 2 ++
>  include/uapi/linux/btrfs_tree.h | 1 +
>  4 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index e79fd9129075..fccc372ef719 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -346,6 +346,9 @@ static inline void btrfs_csum_format(struct btrfs_super_block *sb,
>  	case BTRFS_CSUM_TYPE_CRC32:
>  		snprintf(cbuf, size, "0x%08x", *(u32 *)csum);
>  		break;
> +	case BTRFS_CSUM_TYPE_SHA256:
> +		memcpy(cbuf, csum, size);
> +		break;
>  	default: /* can't happen -  csum type is validated at mount time */
>  		break;
>  	}
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 8733c55ed686..d60138208dd4 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -72,8 +72,8 @@ struct btrfs_ref;
>  #define BTRFS_LINK_MAX 65535U
>  
>  /* four bytes for CRC32 */
> -static const int btrfs_csum_sizes[] = { 4 };
> -static char *btrfs_csum_names[] = { "crc32c" };
> +static const int btrfs_csum_sizes[] = { 4, 32 };
> +static char *btrfs_csum_names[] = { "crc32c", "sha256" };
>  
>  #define BTRFS_EMPTY_DIR_SIZE 0
>  
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 2be8f05be1e6..bdcffa0d6b13 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -390,6 +390,8 @@ static bool btrfs_supported_super_csum(struct btrfs_super_block *sb)
>  	switch (btrfs_super_csum_type(sb)) {
>  	case BTRFS_CSUM_TYPE_CRC32:
>  		return true;
> +	case BTRFS_CSUM_TYPE_SHA256:
> +		return true;

nit: case BTRFS_CSUM_TYPE_CRC32:
     CASE BTRFS_CSUM_TYPE_SHA256:
           return true;



>  	default:
>  		return false;
>  	}
> diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
> index 421239b98db2..3667ab4bc215 100644
> --- a/include/uapi/linux/btrfs_tree.h
> +++ b/include/uapi/linux/btrfs_tree.h
> @@ -301,6 +301,7 @@
>  
>  /* csum types */
>  #define BTRFS_CSUM_TYPE_CRC32	0
> +#define BTRFS_CSUM_TYPE_SHA256	1

nit: Might be a good idea to turn that into an enum for self-documenting
purposes. Perhaps in a different patch.

>  
>  /*
>   * flags definitions for directory entry item type
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 04/17] btrfs: use btrfs_crc32c() instead of btrfs_name_hash()
  2019-05-10 11:15 ` [PATCH 04/17] btrfs: use btrfs_crc32c() instead of btrfs_name_hash() Johannes Thumshirn
@ 2019-05-10 12:56   ` Chris Mason
  2019-05-13  7:04     ` Johannes Thumshirn
  0 siblings, 1 reply; 60+ messages in thread
From: Chris Mason @ 2019-05-10 12:56 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: David Sterba, Linux BTRFS Mailinglist

On 10 May 2019, at 7:15, Johannes Thumshirn wrote:

> Like btrfs_crc32c() btrfs_name_hash() is only a shim wrapper over the
> crc32c() library function. So we can just use btrfs_crc32c() instead 
> of
> btrfs_name_hash().

Reading through the rest of the series, but I think using 
btrfs_name_hash() is more clear and less error prone:

> -	key.offset = btrfs_name_hash(name, name_len);
> +	key.offset = (u64) btrfs_crc32c((u32)~1, name, name_len);

It groups together everyone using crc32c as a directory name hash (or 
extref hash), so that if we ever want to add different hashes later down 
the line, it's really clear what needs to change for what purpose.  With 
everyone calling into btrfs_crc32c directly, you have to spend more time 
looking through history to figure out how to change it.

Also, sprinkling (u32)~1 makes the code less readable imho.

-chris

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 03/17] btrfs: use btrfs_crc32c() instead of btrfs_extref_hash()
  2019-05-10 11:15 ` [PATCH 03/17] btrfs: use btrfs_crc32c() instead of btrfs_extref_hash() Johannes Thumshirn
@ 2019-05-10 13:03   ` Nikolay Borisov
  0 siblings, 0 replies; 60+ messages in thread
From: Nikolay Borisov @ 2019-05-10 13:03 UTC (permalink / raw)
  To: Johannes Thumshirn, David Sterba; +Cc: Linux BTRFS Mailinglist



On 10.05.19 г. 14:15 ч., Johannes Thumshirn wrote:
> Like btrfs_crc32c() btrfs_extref_hash() is only a shim wrapper over the
> crc32c() library function. So we can just use btrfs_crc32c() instead of
> btrfs_extref_hash().
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

I agree with Chris' feedback on this and next patch, just drop them both.

> ---
>  fs/btrfs/ctree.h      | 9 ---------
>  fs/btrfs/inode-item.c | 6 +++---
>  fs/btrfs/tree-log.c   | 6 +++---
>  3 files changed, 6 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index d85541f13f65..a3479a4e4f4d 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2659,15 +2659,6 @@ static inline u64 btrfs_name_hash(const char *name, int len)
>         return crc32c((u32)~1, name, len);
>  }
>  
> -/*
> - * Figure the key offset of an extended inode ref
> - */
> -static inline u64 btrfs_extref_hash(u64 parent_objectid, const char *name,
> -                                   int len)
> -{
> -       return (u64) crc32c(parent_objectid, name, len);
> -}
> -
>  static inline bool btrfs_mixed_space_info(struct btrfs_space_info *space_info)
>  {
>  	return ((space_info->flags & BTRFS_BLOCK_GROUP_METADATA) &&
> diff --git a/fs/btrfs/inode-item.c b/fs/btrfs/inode-item.c
> index 30d62ef918b9..d2b2a64a1553 100644
> --- a/fs/btrfs/inode-item.c
> +++ b/fs/btrfs/inode-item.c
> @@ -91,7 +91,7 @@ btrfs_lookup_inode_extref(struct btrfs_trans_handle *trans,
>  
>  	key.objectid = inode_objectid;
>  	key.type = BTRFS_INODE_EXTREF_KEY;
> -	key.offset = btrfs_extref_hash(ref_objectid, name, name_len);
> +	key.offset = (u64) btrfs_crc32c(ref_objectid, name, name_len);
>  
>  	ret = btrfs_search_slot(trans, root, &key, path, ins_len, cow);
>  	if (ret < 0)
> @@ -123,7 +123,7 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans,
>  
>  	key.objectid = inode_objectid;
>  	key.type = BTRFS_INODE_EXTREF_KEY;
> -	key.offset = btrfs_extref_hash(ref_objectid, name, name_len);
> +	key.offset = (u64) btrfs_crc32c(ref_objectid, name, name_len);
>  
>  	path = btrfs_alloc_path();
>  	if (!path)
> @@ -272,7 +272,7 @@ static int btrfs_insert_inode_extref(struct btrfs_trans_handle *trans,
>  
>  	key.objectid = inode_objectid;
>  	key.type = BTRFS_INODE_EXTREF_KEY;
> -	key.offset = btrfs_extref_hash(ref_objectid, name, name_len);
> +	key.offset = (u64) btrfs_crc32c(ref_objectid, name, name_len);
>  
>  	path = btrfs_alloc_path();
>  	if (!path)
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 6adcd8a2c5c7..f01c6e1a15ed 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -1111,7 +1111,7 @@ static inline int __add_inode_ref(struct btrfs_trans_handle *trans,
>  
>  			search_key.objectid = inode_objectid;
>  			search_key.type = BTRFS_INODE_EXTREF_KEY;
> -			search_key.offset = btrfs_extref_hash(parent_objectid,
> +			search_key.offset = (u64) btrfs_crc32c(parent_objectid,
>  							      victim_name,
>  							      victim_name_len);
>  			ret = 0;
> @@ -1323,7 +1323,7 @@ static int btrfs_inode_ref_exists(struct inode *inode, struct inode *dir,
>  	if (key.type == BTRFS_INODE_REF_KEY)
>  		key.offset = parent_id;
>  	else
> -		key.offset = btrfs_extref_hash(parent_id, name, namelen);
> +		key.offset = (u64) btrfs_crc32c(parent_id, name, namelen);
>  
>  	ret = btrfs_search_slot(NULL, BTRFS_I(inode)->root, &key, path, 0, 0);
>  	if (ret < 0)
> @@ -1901,7 +1901,7 @@ static bool name_in_log_ref(struct btrfs_root *log_root,
>  		return true;
>  
>  	search_key.type = BTRFS_INODE_EXTREF_KEY;
> -	search_key.offset = btrfs_extref_hash(dirid, name, name_len);
> +	search_key.offset = (u64) btrfs_crc32c(dirid, name, name_len);
>  	if (backref_in_log(log_root, &search_key, dirid, name, name_len))
>  		return true;
>  
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 05/17] btrfs: don't assume ordered sums to be 4 bytes
  2019-05-10 11:15 ` [PATCH 05/17] btrfs: don't assume ordered sums to be 4 bytes Johannes Thumshirn
@ 2019-05-10 13:25   ` Nikolay Borisov
  2019-05-10 13:27     ` Nikolay Borisov
  0 siblings, 1 reply; 60+ messages in thread
From: Nikolay Borisov @ 2019-05-10 13:25 UTC (permalink / raw)
  To: Johannes Thumshirn, David Sterba; +Cc: Linux BTRFS Mailinglist



On 10.05.19 г. 14:15 ч., Johannes Thumshirn wrote:
> BTRFS has the implicit assumption that a checksum in btrfs_orderd_sums is 4
> bytes. While this is true for CRC32C, it is not for any other checksum.
> 
> Change the data type to be a byte array and adjust loop index calculation
> accordingly.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  fs/btrfs/compression.c  |  4 ++--
>  fs/btrfs/ctree.h        |  3 ++-
>  fs/btrfs/file-item.c    | 28 +++++++++++++++-------------
>  fs/btrfs/ordered-data.c | 10 ++++++----
>  fs/btrfs/ordered-data.h |  4 ++--
>  fs/btrfs/scrub.c        |  2 +-
>  6 files changed, 28 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 4ec1df369e47..98d8c2ed367f 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -632,7 +632,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
>  
>  			if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
>  				ret = btrfs_lookup_bio_sums(inode, comp_bio,
> -							    sums);
> +							    (u8 *)sums);

This cast (and other similar) are plain ugly. Imho we first need to get
the code into shape for later modification. This means postponing sha256
patch and instead focusing on first getting the code to use u8 where
relevant. Otherwise such cleanup is going to be postponed for "some time
in the future" will is unlikely to ever materialize.

>  				BUG_ON(ret); /* -ENOMEM */
>  			}
>  			sums += DIV_ROUND_UP(comp_bio->bi_iter.bi_size,
> @@ -658,7 +658,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
>  	BUG_ON(ret); /* -ENOMEM */
>  
>  	if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
> -		ret = btrfs_lookup_bio_sums(inode, comp_bio, sums);
> +		ret = btrfs_lookup_bio_sums(inode, comp_bio, (u8 *) sums);
>  		BUG_ON(ret); /* -ENOMEM */
>  	}
>  
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 0a2a377f1640..e62934fb8748 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -3184,7 +3184,8 @@ int btrfs_find_name_in_ext_backref(struct extent_buffer *leaf, int slot,
>  struct btrfs_dio_private;
>  int btrfs_del_csums(struct btrfs_trans_handle *trans,
>  		    struct btrfs_fs_info *fs_info, u64 bytenr, u64 len);
> -blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio, u32 *dst);
> +blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
> +				   u8 *dst);
>  blk_status_t btrfs_lookup_bio_sums_dio(struct inode *inode, struct bio *bio,
>  			      u64 logical_offset);
>  int btrfs_insert_file_extent(struct btrfs_trans_handle *trans,
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index d431ea8198e4..210ff69917a0 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -22,9 +22,9 @@
>  #define MAX_CSUM_ITEMS(r, size) (min_t(u32, __MAX_CSUM_ITEMS(r, size), \
>  				       PAGE_SIZE))
>  
> -#define MAX_ORDERED_SUM_BYTES(fs_info) ((PAGE_SIZE - \
> +#define MAX_ORDERED_SUM_BYTES(fs_info, csum_size) ((PAGE_SIZE - \
>  				   sizeof(struct btrfs_ordered_sum)) / \
> -				   sizeof(u32) * (fs_info)->sectorsize)
> +				   (csum_size) * (fs_info)->sectorsize)
>  
>  int btrfs_insert_file_extent(struct btrfs_trans_handle *trans,
>  			     struct btrfs_root *root,
> @@ -144,7 +144,7 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
>  }
>  
>  static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
> -				   u64 logical_offset, u32 *dst, int dio)
> +				   u64 logical_offset, u8 *dst, int dio)
>  {
>  	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
>  	struct bio_vec bvec;
> @@ -211,7 +211,7 @@ static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio
>  		if (!dio)
>  			offset = page_offset(bvec.bv_page) + bvec.bv_offset;
>  		count = btrfs_find_ordered_sum(inode, offset, disk_bytenr,
> -					       (u32 *)csum, nblocks);
> +					       csum, nblocks);
>  		if (count)
>  			goto found;
>  
> @@ -283,7 +283,8 @@ static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio
>  	return 0;
>  }
>  
> -blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio, u32 *dst)
> +blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
> +				   u8 *dst)
>  {
>  	return __btrfs_lookup_bio_sums(inode, bio, 0, dst, 0);
>  }
> @@ -374,7 +375,7 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
>  				      struct btrfs_csum_item);
>  		while (start < csum_end) {
>  			size = min_t(size_t, csum_end - start,
> -				     MAX_ORDERED_SUM_BYTES(fs_info));
> +				     MAX_ORDERED_SUM_BYTES(fs_info, csum_size));
>  			sums = kzalloc(btrfs_ordered_sum_size(fs_info, size),
>  				       GFP_NOFS);
>  			if (!sums) {
> @@ -439,6 +440,7 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
>  	int i;
>  	u64 offset;
>  	unsigned nofs_flag;
> +	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
>  
>  	nofs_flag = memalloc_nofs_save();
>  	sums = kvzalloc(btrfs_ordered_sum_size(fs_info, bio->bi_iter.bi_size),
> @@ -473,6 +475,7 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
>  						 - 1);
>  
>  		for (i = 0; i < nr_sectors; i++) {
> +			u32 tmp;
>  			if (offset >= ordered->file_offset + ordered->len ||
>  				offset < ordered->file_offset) {
>  				unsigned long bytes_left;
> @@ -498,17 +501,16 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
>  				index = 0;
>  			}
>  
> -			sums->sums[index] = ~(u32)0;
> +			memset(&sums->sums[index], 0xff, csum_size);
>  			data = kmap_atomic(bvec.bv_page);
> -			sums->sums[index]
> -				= btrfs_csum_data(data + bvec.bv_offset
> +			tmp = btrfs_csum_data(data + bvec.bv_offset
>  						+ (i * fs_info->sectorsize),
> -						sums->sums[index],
> +						*(u32 *)&sums->sums[index],
>  						fs_info->sectorsize);
>  			kunmap_atomic(data);
> -			btrfs_csum_final(sums->sums[index],
> +			btrfs_csum_final(tmp,
>  					(char *)(sums->sums + index));
> -			index++;
> +			index += csum_size;
>  			offset += fs_info->sectorsize;
>  			this_sum_bytes += fs_info->sectorsize;
>  			total_bytes += fs_info->sectorsize;
> @@ -904,9 +906,9 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
>  	write_extent_buffer(leaf, sums->sums + index, (unsigned long)item,
>  			    ins_size);
>  
> +	index += ins_size;
>  	ins_size /= csum_size;
>  	total_bytes += ins_size * fs_info->sectorsize;
> -	index += ins_size;
>  
>  	btrfs_mark_buffer_dirty(path->nodes[0]);
>  	if (total_bytes < sums->len) {
> diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
> index 52889da69113..6f7a18148dcb 100644
> --- a/fs/btrfs/ordered-data.c
> +++ b/fs/btrfs/ordered-data.c
> @@ -924,14 +924,16 @@ int btrfs_ordered_update_i_size(struct inode *inode, u64 offset,
>   * be reclaimed before their checksum is actually put into the btree
>   */
>  int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
> -			   u32 *sum, int len)
> +			   u8 *sum, int len)
>  {
> +	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
>  	struct btrfs_ordered_sum *ordered_sum;
>  	struct btrfs_ordered_extent *ordered;
>  	struct btrfs_ordered_inode_tree *tree = &BTRFS_I(inode)->ordered_tree;
>  	unsigned long num_sectors;
>  	unsigned long i;
>  	u32 sectorsize = btrfs_inode_sectorsize(inode);
> +	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
>  	int index = 0;
>  
>  	ordered = btrfs_lookup_ordered_extent(inode, offset);
> @@ -947,10 +949,10 @@ int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
>  			num_sectors = ordered_sum->len >>
>  				      inode->i_sb->s_blocksize_bits;
>  			num_sectors = min_t(int, len - index, num_sectors - i);
> -			memcpy(sum + index, ordered_sum->sums + i,
> -			       num_sectors);
> +			memcpy(sum + index, ordered_sum->sums + i * csum_size,
> +			       num_sectors * csum_size);
>  
> -			index += (int)num_sectors;
> +			index += (int)num_sectors * csum_size;
>  			if (index == len)
>  				goto out;
>  			disk_bytenr += num_sectors * sectorsize;
> diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
> index 4c5991c3de14..9a9884966343 100644
> --- a/fs/btrfs/ordered-data.h
> +++ b/fs/btrfs/ordered-data.h
> @@ -23,7 +23,7 @@ struct btrfs_ordered_sum {
>  	int len;
>  	struct list_head list;
>  	/* last field is a variable length array of csums */
> -	u32 sums[];
> +	u8 sums[];
>  };
>  
>  /*
> @@ -183,7 +183,7 @@ struct btrfs_ordered_extent *btrfs_lookup_ordered_range(
>  int btrfs_ordered_update_i_size(struct inode *inode, u64 offset,
>  				struct btrfs_ordered_extent *ordered);
>  int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
> -			   u32 *sum, int len);
> +			   u8 *sum, int len);
>  u64 btrfs_wait_ordered_extents(struct btrfs_root *root, u64 nr,
>  			       const u64 range_start, const u64 range_len);
>  u64 btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, u64 nr,
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index f7b29f9db5e2..2cf3cf9e9c9b 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -2448,7 +2448,7 @@ static int scrub_find_csum(struct scrub_ctx *sctx, u64 logical, u8 *csum)
>  	ASSERT(index < UINT_MAX);
>  
>  	num_sectors = sum->len / sctx->fs_info->sectorsize;
> -	memcpy(csum, sum->sums + index, sctx->csum_size);
> +	memcpy(csum, sum->sums + index * sctx->csum_size, sctx->csum_size);
>  	if (index == num_sectors - 1) {
>  		list_del(&sum->list);
>  		kfree(sum);
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 05/17] btrfs: don't assume ordered sums to be 4 bytes
  2019-05-10 13:25   ` Nikolay Borisov
@ 2019-05-10 13:27     ` Nikolay Borisov
  2019-05-13  7:06       ` Johannes Thumshirn
  0 siblings, 1 reply; 60+ messages in thread
From: Nikolay Borisov @ 2019-05-10 13:27 UTC (permalink / raw)
  To: Johannes Thumshirn, David Sterba; +Cc: Linux BTRFS Mailinglist



On 10.05.19 г. 16:25 ч., Nikolay Borisov wrote:
> 
> 
> On 10.05.19 г. 14:15 ч., Johannes Thumshirn wrote:
>> BTRFS has the implicit assumption that a checksum in btrfs_orderd_sums is 4
>> bytes. While this is true for CRC32C, it is not for any other checksum.
>>
>> Change the data type to be a byte array and adjust loop index calculation
>> accordingly.
>>
>> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
>> ---
>>  fs/btrfs/compression.c  |  4 ++--
>>  fs/btrfs/ctree.h        |  3 ++-
>>  fs/btrfs/file-item.c    | 28 +++++++++++++++-------------
>>  fs/btrfs/ordered-data.c | 10 ++++++----
>>  fs/btrfs/ordered-data.h |  4 ++--
>>  fs/btrfs/scrub.c        |  2 +-
>>  6 files changed, 28 insertions(+), 23 deletions(-)
>>
>> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
>> index 4ec1df369e47..98d8c2ed367f 100644
>> --- a/fs/btrfs/compression.c
>> +++ b/fs/btrfs/compression.c
>> @@ -632,7 +632,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
>>  
>>  			if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
>>  				ret = btrfs_lookup_bio_sums(inode, comp_bio,
>> -							    sums);
>> +							    (u8 *)sums);
> 
> This cast (and other similar) are plain ugly. Imho we first need to get
> the code into shape for later modification. This means postponing sha256
> patch and instead focusing on first getting the code to use u8 where
> relevant. Otherwise such cleanup is going to be postponed for "some time
> in the future" will is unlikely to ever materialize.

Oh, so you fix that in the next patch. Okay, disregard this then.
> 
>>  				BUG_ON(ret); /* -ENOMEM */
>>  			}
>>  			sums += DIV_ROUND_UP(comp_bio->bi_iter.bi_size,
>> @@ -658,7 +658,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
>>  	BUG_ON(ret); /* -ENOMEM */
>>  
>>  	if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
>> -		ret = btrfs_lookup_bio_sums(inode, comp_bio, sums);
>> +		ret = btrfs_lookup_bio_sums(inode, comp_bio, (u8 *) sums);
>>  		BUG_ON(ret); /* -ENOMEM */
>>  	}
>>  
>> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
>> index 0a2a377f1640..e62934fb8748 100644
>> --- a/fs/btrfs/ctree.h
>> +++ b/fs/btrfs/ctree.h
>> @@ -3184,7 +3184,8 @@ int btrfs_find_name_in_ext_backref(struct extent_buffer *leaf, int slot,
>>  struct btrfs_dio_private;
>>  int btrfs_del_csums(struct btrfs_trans_handle *trans,
>>  		    struct btrfs_fs_info *fs_info, u64 bytenr, u64 len);
>> -blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio, u32 *dst);
>> +blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
>> +				   u8 *dst);
>>  blk_status_t btrfs_lookup_bio_sums_dio(struct inode *inode, struct bio *bio,
>>  			      u64 logical_offset);
>>  int btrfs_insert_file_extent(struct btrfs_trans_handle *trans,
>> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
>> index d431ea8198e4..210ff69917a0 100644
>> --- a/fs/btrfs/file-item.c
>> +++ b/fs/btrfs/file-item.c
>> @@ -22,9 +22,9 @@
>>  #define MAX_CSUM_ITEMS(r, size) (min_t(u32, __MAX_CSUM_ITEMS(r, size), \
>>  				       PAGE_SIZE))
>>  
>> -#define MAX_ORDERED_SUM_BYTES(fs_info) ((PAGE_SIZE - \
>> +#define MAX_ORDERED_SUM_BYTES(fs_info, csum_size) ((PAGE_SIZE - \
>>  				   sizeof(struct btrfs_ordered_sum)) / \
>> -				   sizeof(u32) * (fs_info)->sectorsize)
>> +				   (csum_size) * (fs_info)->sectorsize)
>>  
>>  int btrfs_insert_file_extent(struct btrfs_trans_handle *trans,
>>  			     struct btrfs_root *root,
>> @@ -144,7 +144,7 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
>>  }
>>  
>>  static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
>> -				   u64 logical_offset, u32 *dst, int dio)
>> +				   u64 logical_offset, u8 *dst, int dio)
>>  {
>>  	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
>>  	struct bio_vec bvec;
>> @@ -211,7 +211,7 @@ static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio
>>  		if (!dio)
>>  			offset = page_offset(bvec.bv_page) + bvec.bv_offset;
>>  		count = btrfs_find_ordered_sum(inode, offset, disk_bytenr,
>> -					       (u32 *)csum, nblocks);
>> +					       csum, nblocks);
>>  		if (count)
>>  			goto found;
>>  
>> @@ -283,7 +283,8 @@ static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio
>>  	return 0;
>>  }
>>  
>> -blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio, u32 *dst)
>> +blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
>> +				   u8 *dst)
>>  {
>>  	return __btrfs_lookup_bio_sums(inode, bio, 0, dst, 0);
>>  }
>> @@ -374,7 +375,7 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
>>  				      struct btrfs_csum_item);
>>  		while (start < csum_end) {
>>  			size = min_t(size_t, csum_end - start,
>> -				     MAX_ORDERED_SUM_BYTES(fs_info));
>> +				     MAX_ORDERED_SUM_BYTES(fs_info, csum_size));
>>  			sums = kzalloc(btrfs_ordered_sum_size(fs_info, size),
>>  				       GFP_NOFS);
>>  			if (!sums) {
>> @@ -439,6 +440,7 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
>>  	int i;
>>  	u64 offset;
>>  	unsigned nofs_flag;
>> +	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
>>  
>>  	nofs_flag = memalloc_nofs_save();
>>  	sums = kvzalloc(btrfs_ordered_sum_size(fs_info, bio->bi_iter.bi_size),
>> @@ -473,6 +475,7 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
>>  						 - 1);
>>  
>>  		for (i = 0; i < nr_sectors; i++) {
>> +			u32 tmp;
>>  			if (offset >= ordered->file_offset + ordered->len ||
>>  				offset < ordered->file_offset) {
>>  				unsigned long bytes_left;
>> @@ -498,17 +501,16 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
>>  				index = 0;
>>  			}
>>  
>> -			sums->sums[index] = ~(u32)0;
>> +			memset(&sums->sums[index], 0xff, csum_size);
>>  			data = kmap_atomic(bvec.bv_page);
>> -			sums->sums[index]
>> -				= btrfs_csum_data(data + bvec.bv_offset
>> +			tmp = btrfs_csum_data(data + bvec.bv_offset
>>  						+ (i * fs_info->sectorsize),
>> -						sums->sums[index],
>> +						*(u32 *)&sums->sums[index],
>>  						fs_info->sectorsize);
>>  			kunmap_atomic(data);
>> -			btrfs_csum_final(sums->sums[index],
>> +			btrfs_csum_final(tmp,
>>  					(char *)(sums->sums + index));
>> -			index++;
>> +			index += csum_size;
>>  			offset += fs_info->sectorsize;
>>  			this_sum_bytes += fs_info->sectorsize;
>>  			total_bytes += fs_info->sectorsize;
>> @@ -904,9 +906,9 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
>>  	write_extent_buffer(leaf, sums->sums + index, (unsigned long)item,
>>  			    ins_size);
>>  
>> +	index += ins_size;
>>  	ins_size /= csum_size;
>>  	total_bytes += ins_size * fs_info->sectorsize;
>> -	index += ins_size;
>>  
>>  	btrfs_mark_buffer_dirty(path->nodes[0]);
>>  	if (total_bytes < sums->len) {
>> diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
>> index 52889da69113..6f7a18148dcb 100644
>> --- a/fs/btrfs/ordered-data.c
>> +++ b/fs/btrfs/ordered-data.c
>> @@ -924,14 +924,16 @@ int btrfs_ordered_update_i_size(struct inode *inode, u64 offset,
>>   * be reclaimed before their checksum is actually put into the btree
>>   */
>>  int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
>> -			   u32 *sum, int len)
>> +			   u8 *sum, int len)
>>  {
>> +	struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
>>  	struct btrfs_ordered_sum *ordered_sum;
>>  	struct btrfs_ordered_extent *ordered;
>>  	struct btrfs_ordered_inode_tree *tree = &BTRFS_I(inode)->ordered_tree;
>>  	unsigned long num_sectors;
>>  	unsigned long i;
>>  	u32 sectorsize = btrfs_inode_sectorsize(inode);
>> +	u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
>>  	int index = 0;
>>  
>>  	ordered = btrfs_lookup_ordered_extent(inode, offset);
>> @@ -947,10 +949,10 @@ int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
>>  			num_sectors = ordered_sum->len >>
>>  				      inode->i_sb->s_blocksize_bits;
>>  			num_sectors = min_t(int, len - index, num_sectors - i);
>> -			memcpy(sum + index, ordered_sum->sums + i,
>> -			       num_sectors);
>> +			memcpy(sum + index, ordered_sum->sums + i * csum_size,
>> +			       num_sectors * csum_size);
>>  
>> -			index += (int)num_sectors;
>> +			index += (int)num_sectors * csum_size;
>>  			if (index == len)
>>  				goto out;
>>  			disk_bytenr += num_sectors * sectorsize;
>> diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
>> index 4c5991c3de14..9a9884966343 100644
>> --- a/fs/btrfs/ordered-data.h
>> +++ b/fs/btrfs/ordered-data.h
>> @@ -23,7 +23,7 @@ struct btrfs_ordered_sum {
>>  	int len;
>>  	struct list_head list;
>>  	/* last field is a variable length array of csums */
>> -	u32 sums[];
>> +	u8 sums[];
>>  };
>>  
>>  /*
>> @@ -183,7 +183,7 @@ struct btrfs_ordered_extent *btrfs_lookup_ordered_range(
>>  int btrfs_ordered_update_i_size(struct inode *inode, u64 offset,
>>  				struct btrfs_ordered_extent *ordered);
>>  int btrfs_find_ordered_sum(struct inode *inode, u64 offset, u64 disk_bytenr,
>> -			   u32 *sum, int len);
>> +			   u8 *sum, int len);
>>  u64 btrfs_wait_ordered_extents(struct btrfs_root *root, u64 nr,
>>  			       const u64 range_start, const u64 range_len);
>>  u64 btrfs_wait_ordered_roots(struct btrfs_fs_info *fs_info, u64 nr,
>> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
>> index f7b29f9db5e2..2cf3cf9e9c9b 100644
>> --- a/fs/btrfs/scrub.c
>> +++ b/fs/btrfs/scrub.c
>> @@ -2448,7 +2448,7 @@ static int scrub_find_csum(struct scrub_ctx *sctx, u64 logical, u8 *csum)
>>  	ASSERT(index < UINT_MAX);
>>  
>>  	num_sectors = sum->len / sctx->fs_info->sectorsize;
>> -	memcpy(csum, sum->sums + index, sctx->csum_size);
>> +	memcpy(csum, sum->sums + index * sctx->csum_size, sctx->csum_size);
>>  	if (index == num_sectors - 1) {
>>  		list_del(&sum->list);
>>  		kfree(sum);
>>
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 07/17] btrfs: use btrfs_crc32c{,_final}() in for free space cache
  2019-05-10 11:15 ` [PATCH 07/17] btrfs: use btrfs_crc32c{,_final}() in for free space cache Johannes Thumshirn
@ 2019-05-10 13:27   ` Nikolay Borisov
  0 siblings, 0 replies; 60+ messages in thread
From: Nikolay Borisov @ 2019-05-10 13:27 UTC (permalink / raw)
  To: Johannes Thumshirn, David Sterba; +Cc: Linux BTRFS Mailinglist



On 10.05.19 г. 14:15 ч., Johannes Thumshirn wrote:
> The CRC checksum in the free space cache is not dependant on the super
> block's csum_type field but always a CRC32C.
> 
> So use btrfs_crc32c() and btrfs_crc32c_final() instead of btrfs_csum_data()
> and btrfs_csum_final() for computing these checksums.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/free-space-cache.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index f74dc259307b..26ed8ed60722 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -465,9 +465,8 @@ static void io_ctl_set_crc(struct btrfs_io_ctl *io_ctl, int index)
>  	if (index == 0)
>  		offset = sizeof(u32) * io_ctl->num_pages;
>  
> -	crc = btrfs_csum_data(io_ctl->orig + offset, crc,
> -			      PAGE_SIZE - offset);
> -	btrfs_csum_final(crc, (u8 *)&crc);
> +	crc = btrfs_crc32c(crc, io_ctl->orig + offset, PAGE_SIZE - offset);
> +	btrfs_crc32c_final(crc, (u8 *)&crc);
>  	io_ctl_unmap_page(io_ctl);
>  	tmp = page_address(io_ctl->pages[0]);
>  	tmp += index;
> @@ -493,9 +492,8 @@ static int io_ctl_check_crc(struct btrfs_io_ctl *io_ctl, int index)
>  	val = *tmp;
>  
>  	io_ctl_map_page(io_ctl, 0);
> -	crc = btrfs_csum_data(io_ctl->orig + offset, crc,
> -			      PAGE_SIZE - offset);
> -	btrfs_csum_final(crc, (u8 *)&crc);
> +	crc = btrfs_crc32c(crc, io_ctl->orig + offset, PAGE_SIZE - offset);
> +	btrfs_crc32c_final(crc, (u8 *)&crc);
>  	if (val != crc) {
>  		btrfs_err_rl(io_ctl->fs_info,
>  			"csum mismatch on free space cache");
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 08/17] btrfs: format checksums according to type for printing
  2019-05-10 11:15 ` [PATCH 08/17] btrfs: format checksums according to type for printing Johannes Thumshirn
@ 2019-05-10 13:28   ` Nikolay Borisov
  0 siblings, 0 replies; 60+ messages in thread
From: Nikolay Borisov @ 2019-05-10 13:28 UTC (permalink / raw)
  To: Johannes Thumshirn, David Sterba; +Cc: Linux BTRFS Mailinglist



On 10.05.19 г. 14:15 ч., Johannes Thumshirn wrote:
> Add a small helper for btrfs_print_data_csum_error() which formats the
> checksum according to it's type for pretty printing.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/btrfs_inode.h | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
> index d5b438706b77..f0a757eb5744 100644
> --- a/fs/btrfs/btrfs_inode.h
> +++ b/fs/btrfs/btrfs_inode.h
> @@ -337,22 +337,42 @@ static inline void btrfs_inode_resume_unlocked_dio(struct btrfs_inode *inode)
>  	clear_bit(BTRFS_INODE_READDIO_NEED_LOCK, &inode->runtime_flags);
>  }
>  
> +static inline void btrfs_csum_format(struct btrfs_super_block *sb,
> +				     u32 csum, u8 *cbuf)
> +{
> +	size_t size = btrfs_super_csum_size(sb) * 8;
> +
> +	switch (btrfs_super_csum_type(sb)) {
> +	case BTRFS_CSUM_TYPE_CRC32:
> +		snprintf(cbuf, size, "0x%08x", csum);
> +		break;
> +	default: /* can't happen -  csum type is validated at mount time */
> +		break;
> +	}
> +}
> +
>  static inline void btrfs_print_data_csum_error(struct btrfs_inode *inode,
>  		u64 logical_start, u32 csum, u32 csum_expected, int mirror_num)
>  {
>  	struct btrfs_root *root = inode->root;
> +	struct btrfs_super_block *sb = root->fs_info->super_copy;
> +	u8 cbuf[BTRFS_CSUM_SIZE];
> +	u8 ecbuf[BTRFS_CSUM_SIZE];
> +
> +	btrfs_csum_format(sb, csum, cbuf);
> +	btrfs_csum_format(sb, csum_expected, ecbuf);
>  
>  	/* Output minus objectid, which is more meaningful */
>  	if (root->root_key.objectid >= BTRFS_LAST_FREE_OBJECTID)
>  		btrfs_warn_rl(root->fs_info,
> -	"csum failed root %lld ino %lld off %llu csum 0x%08x expected csum 0x%08x mirror %d",
> +	"csum failed root %lld ino %lld off %llu csum %s expected csum %s mirror %d",
>  			root->root_key.objectid, btrfs_ino(inode),
> -			logical_start, csum, csum_expected, mirror_num);
> +			logical_start, cbuf, ecbuf, mirror_num);
>  	else
>  		btrfs_warn_rl(root->fs_info,
> -	"csum failed root %llu ino %llu off %llu csum 0x%08x expected csum 0x%08x mirror %d",
> +	"csum failed root %llu ino %llu off %llu csum %s expected csum %s mirror %d",
>  			root->root_key.objectid, btrfs_ino(inode),
> -			logical_start, csum, csum_expected, mirror_num);
> +			logical_start, cbuf, ecbuf, mirror_num);
>  }
>  
>  #endif
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 09/17] btrfs: add common checksum type validation
  2019-05-10 11:15 ` [PATCH 09/17] btrfs: add common checksum type validation Johannes Thumshirn
@ 2019-05-10 13:37   ` Nikolay Borisov
  0 siblings, 0 replies; 60+ messages in thread
From: Nikolay Borisov @ 2019-05-10 13:37 UTC (permalink / raw)
  To: Johannes Thumshirn, David Sterba; +Cc: Linux BTRFS Mailinglist



On 10.05.19 г. 14:15 ч., Johannes Thumshirn wrote:
> Currently btrfs is only supporting CRC32C as checksumming algorithm. As
> this is about to change provide a function to validate the checksum type in
> the superblock against all possible algorithms.
> 
> This makes adding new algorithms easier as there are fewer places to adjust
> when adding new algorithms.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/disk-io.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 663efce22d98..ab13282d91d2 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -356,6 +356,16 @@ static int verify_parent_transid(struct extent_io_tree *io_tree,
>  	return ret;
>  }
>  
> +static bool btrfs_supported_super_csum(struct btrfs_super_block *sb)
> +{
> +	switch (btrfs_super_csum_type(sb)) {
> +	case BTRFS_CSUM_TYPE_CRC32:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>  /*
>   * Return 0 if the superblock checksum type matches the checksum value of that
>   * algorithm. Pass the raw disk superblock data.
> @@ -368,6 +378,12 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
>  	u16 csum_type = btrfs_super_csum_type(disk_sb);
>  	int ret = 0;
>  
> +	if (!btrfs_supported_super_csum(disk_sb)) {
> +		btrfs_err(fs_info, "unsupported checksum algorithm %u",
> +			  csum_type);
> +		ret = 1;
> +	}
> +
>  	if (csum_type == BTRFS_CSUM_TYPE_CRC32) {
>  		u32 crc = ~(u32)0;
>  		char result[sizeof(crc)];
> @@ -385,12 +401,6 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
>  			ret = 1;
>  	}
>  
> -	if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
> -		btrfs_err(fs_info, "unsupported checksum algorithm %u",
> -				csum_type);
> -		ret = 1;
> -	}
> -
>  	return ret;
>  }
>  
> @@ -2577,7 +2587,7 @@ static int btrfs_validate_write_super(struct btrfs_fs_info *fs_info,
>  	ret = validate_super(fs_info, sb, -1);
>  	if (ret < 0)
>  		goto out;
> -	if (btrfs_super_csum_type(sb) != BTRFS_CSUM_TYPE_CRC32) {
> +	if (!btrfs_supported_super_csum(sb)) {
>  		ret = -EUCLEAN;
>  		btrfs_err(fs_info, "invalid csum type, has %u want %u",
>  			  btrfs_super_csum_type(sb), BTRFS_CSUM_TYPE_CRC32);
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 10/17] btrfs: check for supported superblock checksum type before checksum validation
  2019-05-10 11:15 ` [PATCH 10/17] btrfs: check for supported superblock checksum type before checksum validation Johannes Thumshirn
@ 2019-05-10 13:37   ` Nikolay Borisov
  0 siblings, 0 replies; 60+ messages in thread
From: Nikolay Borisov @ 2019-05-10 13:37 UTC (permalink / raw)
  To: Johannes Thumshirn, David Sterba; +Cc: Linux BTRFS Mailinglist



On 10.05.19 г. 14:15 ч., Johannes Thumshirn wrote:
> Check for supported superblock checksum type before doing the actual
> checksum validation of the superblock read from disk.

This is rather terse, how does it improve the code and what was the
current status quo (e.g before this patch).
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  fs/btrfs/disk-io.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index ab13282d91d2..74937effaed4 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2822,6 +2822,14 @@ int open_ctree(struct super_block *sb,
>  		goto fail_alloc;
>  	}
>  
> +	if (!btrfs_supported_super_csum((struct btrfs_super_block *)
> +					bh->b_data)) {
> +		btrfs_err(fs_info, "unsupported checksum algorithm");
> +		err = -EINVAL;
> +		brelse(bh);
> +		goto fail_alloc;
> +	}
> +
>  	/*
>  	 * We want to check superblock checksum, the type is stored inside.
>  	 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 11/17] btrfs: Simplify btrfs_check_super_csum() and get rid of size assumptions
  2019-05-10 11:15 ` [PATCH 11/17] btrfs: Simplify btrfs_check_super_csum() and get rid of size assumptions Johannes Thumshirn
@ 2019-05-10 13:41   ` Nikolay Borisov
  0 siblings, 0 replies; 60+ messages in thread
From: Nikolay Borisov @ 2019-05-10 13:41 UTC (permalink / raw)
  To: Johannes Thumshirn, David Sterba; +Cc: Linux BTRFS Mailinglist



On 10.05.19 г. 14:15 ч., Johannes Thumshirn wrote:
> Now that we have already checked for a valid checksum type before calling
> btrfs_check_super_csum(), it can be simplified even further.
> 
> While at it get rid of the implicit size assumption of the resulting
> checksum as well.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

Reviewed-by: Nikolay Borisov <nborisov@suse.com> , albeit one small nit
below.

> ---
>  fs/btrfs/disk-io.c | 37 +++++++++++++------------------------
>  1 file changed, 13 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 74937effaed4..21ae9daf52b7 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -375,33 +375,22 @@ static int btrfs_check_super_csum(struct btrfs_fs_info *fs_info,
>  {
>  	struct btrfs_super_block *disk_sb =
>  		(struct btrfs_super_block *)raw_disk_sb;
> -	u16 csum_type = btrfs_super_csum_type(disk_sb);
> -	int ret = 0;
> -
> -	if (!btrfs_supported_super_csum(disk_sb)) {
> -		btrfs_err(fs_info, "unsupported checksum algorithm %u",
> -			  csum_type);
> -		ret = 1;
> -	}
> -
> -	if (csum_type == BTRFS_CSUM_TYPE_CRC32) {
> -		u32 crc = ~(u32)0;
> -		char result[sizeof(crc)];
> +	u32 crc = ~(u32)0;
> +	char result[BTRFS_CSUM_SIZE];
>  
> -		/*
> -		 * The super_block structure does not span the whole
> -		 * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
> -		 * is filled with zeros and is included in the checksum.
> -		 */
> -		crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
> -				crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
> -		btrfs_csum_final(crc, result);
> +	/*
> +	 * The super_block structure does not span the whole
> +	 * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
> +	 * is filled with zeros and is included in the checksum.
> +	 */
> +	crc = btrfs_csum_data(raw_disk_sb + BTRFS_CSUM_SIZE,
> +			      crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
> +	btrfs_csum_final(crc, result);
>  
> -		if (memcmp(raw_disk_sb, result, sizeof(result)))
> -			ret = 1;
> -	}
> +	if (memcmp(raw_disk_sb, result, btrfs_super_csum_size(disk_sb)))

nit: I'd rather have the on-disk csum be referred to as : disk_sb->csum
it's more self-documenting.

> +		return 1;
>  
> -	return ret;
> +	return 0;
>  }
>  
>  int btrfs_verify_level_key(struct extent_buffer *eb, int level,
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 14/17] btrfs: directly call into crypto framework for checsumming
  2019-05-10 11:15 ` [PATCH 14/17] btrfs: directly call into crypto framework for checsumming Johannes Thumshirn
@ 2019-05-10 13:45   ` Chris Mason
  2019-05-10 13:54     ` Chris Mason
  2019-05-14 12:46     ` Johannes Thumshirn
  2019-05-13 13:00   ` David Sterba
  1 sibling, 2 replies; 60+ messages in thread
From: Chris Mason @ 2019-05-10 13:45 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: David Sterba, Linux BTRFS Mailinglist

On 10 May 2019, at 7:15, Johannes Thumshirn wrote:

> Currently btrfs_csum_data() relied on the crc32c() wrapper around the 
> crypto
> framework for calculating the CRCs.
>
> As we have our own crypto_shash structure in the fs_info now, we can
> directly call into the crypto framework without going trough the 
> wrapper.

It has been a while since I dug through the cryptoapi internals.  I have 
one vague and somewhat unfounded concern, where we're defining the disk 
format to be whatever is returned from looking up sha256 or crc32c in 
the crypto tables.  I'm not really sure how to resolve this since we 
obviously don't want our own sha256 implementation.

I'm a little concerned about about btrfs_csum_data() and 
btrfs_csum_final() below.  We're using two different 
SHASH_DESC_ON_STACK() and then overwriting internals (shash_desc_ctx()) 
with the assumption that whatever we're doing is going to be the same as 
using the same shash_desc struct for both the update and final calls.  I 
think we should be either using or adding a helper to the crypto api for 
this.  We're digging too deep into cryptoapi private structs with the 
current usage.

Otherwise, thanks for doing this, it looks great overall.

-chris

>
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  fs/btrfs/disk-io.c | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index fb0b06b7b9f6..0c9ac7b45ce8 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -253,12 +253,36 @@ struct extent_map *btree_get_extent(struct 
> btrfs_inode *inode,
>  u32 btrfs_csum_data(struct btrfs_fs_info *fs_info, const char *data,
>  		    u32 seed, size_t len)
>  {
> -	return crc32c(seed, data, len);
> +	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> +	u32 *ctx = (u32 *)shash_desc_ctx(shash);
> +	u32 retval;
> +	int err;
> +
> +	shash->tfm = fs_info->csum_shash;
> +	shash->flags = 0;
> +	*ctx = seed;
> +
> +	err = crypto_shash_update(shash, data, len);
> +	ASSERT(err);
> +
> +	retval = *ctx;
> +	barrier_data(ctx);
> +
> +	return retval;
>  }
>
>  void btrfs_csum_final(struct btrfs_fs_info *fs_info, u32 crc, u8 
> *result)
>  {
> -	put_unaligned_le32(~crc, result);
> +	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> +	u32 *ctx = (u32 *)shash_desc_ctx(shash);
> +	int err;
> +
> +	shash->tfm = fs_info->csum_shash;
> +	shash->flags = 0;
> +	*ctx = crc;
> +
> +	err = crypto_shash_final(shash, result);
> +	ASSERT(err);
>  }
>
>  /*
> -- 
> 2.16.4

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 14/17] btrfs: directly call into crypto framework for checsumming
  2019-05-10 13:45   ` Chris Mason
@ 2019-05-10 13:54     ` Chris Mason
  2019-05-13  7:17       ` Johannes Thumshirn
  2019-05-14 12:46     ` Johannes Thumshirn
  1 sibling, 1 reply; 60+ messages in thread
From: Chris Mason @ 2019-05-10 13:54 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: David Sterba, Linux BTRFS Mailinglist



On 10 May 2019, at 9:45, Chris Mason wrote:

> On 10 May 2019, at 7:15, Johannes Thumshirn wrote:
>
>> Currently btrfs_csum_data() relied on the crc32c() wrapper around the 
>> crypto
>> framework for calculating the CRCs.
>>
>> As we have our own crypto_shash structure in the fs_info now, we can
>> directly call into the crypto framework without going trough the 
>> wrapper.
>
> It has been a while since I dug through the cryptoapi internals.  I 
> have one vague and somewhat unfounded concern, where we're defining 
> the disk format to be whatever is returned from looking up sha256 or 
> crc32c in the crypto tables.  I'm not really sure how to resolve this 
> since we obviously don't want our own sha256 implementation.
>
> I'm a little concerned about about btrfs_csum_data() and 
> btrfs_csum_final() below.  We're using two different 
> SHASH_DESC_ON_STACK() and then overwriting internals 
> (shash_desc_ctx()) with the assumption that whatever we're doing is 
> going to be the same as using the same shash_desc struct for both the 
> update and final calls.  I think we should be either using or adding a 
> helper to the crypto api for this.  We're digging too deep into 
> cryptoapi private structs with the current usage.

Looking at the callers of btrfs_csum_final() and btrfs_csum_data(), lets 
just pass the shash_desc from the callers.  That way you don't have to 
poke at memcpy and shash_desc_ctx().

I'm a little worried about stack usage (at least 360 bytes), but worst 
case we can do some percpu gymnastics.

-chris

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 01/17] btrfs: use btrfs_csum_data() instead of directly calling crc32c
  2019-05-10 11:15 ` [PATCH 01/17] btrfs: use btrfs_csum_data() instead of directly calling crc32c Johannes Thumshirn
@ 2019-05-10 16:16   ` Nikolay Borisov
  0 siblings, 0 replies; 60+ messages in thread
From: Nikolay Borisov @ 2019-05-10 16:16 UTC (permalink / raw)
  To: Johannes Thumshirn, David Sterba; +Cc: Linux BTRFS Mailinglist



On 10.05.19 г. 14:15 ч., Johannes Thumshirn wrote:
> btrfsic_test_for_metadata() directly calls the crc32c() library function
> for calculating the CRC32C checksum, but then uses btrfs_csum_final() to
> invert the result.
> 
> To ease further refactoring and development around checksumming in BTRFS
> convert to calling btrfs_csum_data(), which is a wrapper around crc32c().
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/check-integrity.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
> index b0c8094528d1..85774e2fa3e5 100644
> --- a/fs/btrfs/check-integrity.c
> +++ b/fs/btrfs/check-integrity.c
> @@ -1728,7 +1728,7 @@ static int btrfsic_test_for_metadata(struct btrfsic_state *state,
>  		size_t sublen = i ? PAGE_SIZE :
>  				    (PAGE_SIZE - BTRFS_CSUM_SIZE);
>  
> -		crc = crc32c(crc, data, sublen);
> +		crc = btrfs_csum_data(data, crc, sublen);
>  	}
>  	btrfs_csum_final(crc, csum);
>  	if (memcmp(csum, h->csum, state->csum_size))
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 02/17] btrfs: resurrect btrfs_crc32c()
  2019-05-10 11:15 ` [PATCH 02/17] btrfs: resurrect btrfs_crc32c() Johannes Thumshirn
@ 2019-05-10 16:16   ` Nikolay Borisov
  0 siblings, 0 replies; 60+ messages in thread
From: Nikolay Borisov @ 2019-05-10 16:16 UTC (permalink / raw)
  To: Johannes Thumshirn, David Sterba; +Cc: Linux BTRFS Mailinglist



On 10.05.19 г. 14:15 ч., Johannes Thumshirn wrote:
> Commit 9678c54388b6 ("btrfs: Remove custom crc32c init code") removed the
> btrfs_crc32c() function, because it was a duplicate of the crc32c() library
> function we already have in the kernel.
> 
> Resurrect it as a shim wrapper over crc32c() to make following
> transformations of the checksumming code in btrfs easier.
> 
> Also provide a btrfs_crc32_final() to ease following transformations.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

Mechanical patch so :

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/ctree.h       | 12 ++++++++++++
>  fs/btrfs/extent-tree.c |  6 +++---
>  fs/btrfs/send.c        |  2 +-
>  3 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index b81c331b28fa..d85541f13f65 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -19,6 +19,7 @@
>  #include <linux/kobject.h>
>  #include <trace/events/btrfs.h>
>  #include <asm/kmap_types.h>
> +#include <asm/unaligned.h>
>  #include <linux/pagemap.h>
>  #include <linux/btrfs.h>
>  #include <linux/btrfs_tree.h>
> @@ -2642,6 +2643,17 @@ BTRFS_SETGET_STACK_FUNCS(stack_dev_replace_cursor_right,
>  	((unsigned long)(BTRFS_LEAF_DATA_OFFSET + \
>  	btrfs_item_offset_nr(leaf, slot)))
>  
> +
> +static inline u32 btrfs_crc32c(u32 crc, const void *address, unsigned length)
> +{
> +	return crc32c(crc, address, length);
> +}
> +
> +static inline void btrfs_crc32c_final(u32 crc, u8 *result)
> +{
> +	put_unaligned_le32(~crc, result);
> +}
> +
>  static inline u64 btrfs_name_hash(const char *name, int len)
>  {
>         return crc32c((u32)~1, name, len);
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index f79e477a378e..06a30f2cd2e0 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -1119,11 +1119,11 @@ static u64 hash_extent_data_ref(u64 root_objectid, u64 owner, u64 offset)
>  	__le64 lenum;
>  
>  	lenum = cpu_to_le64(root_objectid);
> -	high_crc = crc32c(high_crc, &lenum, sizeof(lenum));
> +	high_crc = btrfs_crc32c(high_crc, &lenum, sizeof(lenum));
>  	lenum = cpu_to_le64(owner);
> -	low_crc = crc32c(low_crc, &lenum, sizeof(lenum));
> +	low_crc = btrfs_crc32c(low_crc, &lenum, sizeof(lenum));
>  	lenum = cpu_to_le64(offset);
> -	low_crc = crc32c(low_crc, &lenum, sizeof(lenum));
> +	low_crc = btrfs_crc32c(low_crc, &lenum, sizeof(lenum));
>  
>  	return ((u64)high_crc << 31) ^ (u64)low_crc;
>  }
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index dd38dfe174df..c029ca6d5eba 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -686,7 +686,7 @@ static int send_cmd(struct send_ctx *sctx)
>  	hdr->len = cpu_to_le32(sctx->send_size - sizeof(*hdr));
>  	hdr->crc = 0;
>  
> -	crc = crc32c(0, (unsigned char *)sctx->send_buf, sctx->send_size);
> +	crc = btrfs_crc32c(0, (unsigned char *)sctx->send_buf, sctx->send_size);
>  	hdr->crc = cpu_to_le32(crc);
>  
>  	ret = write_buf(sctx->send_filp, sctx->send_buf, sctx->send_size,
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 12/17] btrfs: add boilerplate code for directly including the crypto framework
  2019-05-10 11:15 ` [PATCH 12/17] btrfs: add boilerplate code for directly including the crypto framework Johannes Thumshirn
@ 2019-05-10 16:28   ` Nikolay Borisov
  0 siblings, 0 replies; 60+ messages in thread
From: Nikolay Borisov @ 2019-05-10 16:28 UTC (permalink / raw)
  To: Johannes Thumshirn, David Sterba; +Cc: Linux BTRFS Mailinglist



On 10.05.19 г. 14:15 ч., Johannes Thumshirn wrote:
> Add boilerplate code for directly including the crypto framework.
> 
> This helps us flipping the switch for new algorithms.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/ctree.h   | 11 +++++++++++
>  fs/btrfs/disk-io.c | 49 ++++++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 53 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index e62934fb8748..8733c55ed686 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -73,6 +73,7 @@ struct btrfs_ref;
>  
>  /* four bytes for CRC32 */
>  static const int btrfs_csum_sizes[] = { 4 };
> +static char *btrfs_csum_names[] = { "crc32c" };
>  
>  #define BTRFS_EMPTY_DIR_SIZE 0
>  
> @@ -1165,6 +1166,8 @@ struct btrfs_fs_info {
>  	spinlock_t ref_verify_lock;
>  	struct rb_root block_tree;
>  #endif
> +
> +	struct crypto_shash *csum_shash;
>  };
>  
>  static inline struct btrfs_fs_info *btrfs_sb(struct super_block *sb)
> @@ -2452,6 +2455,14 @@ static inline int btrfs_super_csum_size(const struct btrfs_super_block *s)
>  	return btrfs_csum_sizes[t];
>  }
>  
> +static inline char *btrfs_super_csum_name(const struct btrfs_super_block *s)
> +{
> +	u16 t = btrfs_super_csum_type(s);
> +	/*
> +	 * csum type is validated at mount time
> +	 */
> +	return btrfs_csum_names[t];
> +}
>  
>  /*
>   * The leaf data grows from end-to-front in the node.
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 21ae9daf52b7..d57adf3eaa85 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -19,6 +19,7 @@
>  #include <linux/crc32c.h>
>  #include <linux/sched/mm.h>
>  #include <asm/unaligned.h>
> +#include <crypto/hash.h>
>  #include "ctree.h"
>  #include "disk-io.h"
>  #include "transaction.h"
> @@ -2261,6 +2262,30 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info,
>  	return 0;
>  }
>  
> +static int btrfs_init_csum_hash(struct btrfs_fs_info *fs_info,
> +				struct btrfs_super_block *sb)
> +{
> +	struct crypto_shash *csum_shash;
> +	const char *csum_name = btrfs_super_csum_name(sb);
> +
> +	csum_shash = crypto_alloc_shash(csum_name, 0, 0);
> +
> +	if (IS_ERR(csum_shash)) {
> +		btrfs_err(fs_info, "error allocating %s hash for checksum\n",
> +			  csum_name);
> +		return PTR_ERR(csum_shash);
> +	}
> +
> +	fs_info->csum_shash = csum_shash;
> +
> +	return 0;
> +}
> +
> +static void btrfs_free_csum_hash(struct btrfs_fs_info *fs_info)
> +{
> +	crypto_free_shash(fs_info->csum_shash);
> +}
> +
>  static int btrfs_replay_log(struct btrfs_fs_info *fs_info,
>  			    struct btrfs_fs_devices *fs_devices)
>  {
> @@ -2819,6 +2844,14 @@ int open_ctree(struct super_block *sb,
>  		goto fail_alloc;
>  	}
>  
> +
> +	ret = btrfs_init_csum_hash(fs_info, (struct btrfs_super_block *)
> +							   bh->b_data);
> +	if (ret) {
> +		err = ret;
> +		goto fail_alloc;
> +	}
> +
>  	/*
>  	 * We want to check superblock checksum, the type is stored inside.
>  	 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
> @@ -2827,7 +2860,7 @@ int open_ctree(struct super_block *sb,
>  		btrfs_err(fs_info, "superblock checksum mismatch");
>  		err = -EINVAL;
>  		brelse(bh);
> -		goto fail_alloc;
> +		goto fail_csum;
>  	}
>  
>  	/*
> @@ -2864,11 +2897,11 @@ int open_ctree(struct super_block *sb,
>  	if (ret) {
>  		btrfs_err(fs_info, "superblock contains fatal errors");
>  		err = -EINVAL;
> -		goto fail_alloc;
> +		goto fail_csum;
>  	}
>  
>  	if (!btrfs_super_root(disk_super))
> -		goto fail_alloc;
> +		goto fail_csum;
>  
>  	/* check FS state, whether FS is broken. */
>  	if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_ERROR)
> @@ -2890,7 +2923,7 @@ int open_ctree(struct super_block *sb,
>  	ret = btrfs_parse_options(fs_info, options, sb->s_flags);
>  	if (ret) {
>  		err = ret;
> -		goto fail_alloc;
> +		goto fail_csum;
>  	}
>  
>  	features = btrfs_super_incompat_flags(disk_super) &
> @@ -2900,7 +2933,7 @@ int open_ctree(struct super_block *sb,
>  		    "cannot mount because of unsupported optional features (%llx)",
>  		    features);
>  		err = -EINVAL;
> -		goto fail_alloc;
> +		goto fail_csum;
>  	}
>  
>  	features = btrfs_super_incompat_flags(disk_super);
> @@ -2944,7 +2977,7 @@ int open_ctree(struct super_block *sb,
>  		btrfs_err(fs_info,
>  "unequal nodesize/sectorsize (%u != %u) are not allowed for mixed block groups",
>  			nodesize, sectorsize);
> -		goto fail_alloc;
> +		goto fail_csum;
>  	}
>  
>  	/*
> @@ -2960,7 +2993,7 @@ int open_ctree(struct super_block *sb,
>  	"cannot mount read-write because of unsupported optional features (%llx)",
>  		       features);
>  		err = -EINVAL;
> -		goto fail_alloc;
> +		goto fail_csum;
>  	}
>  
>  	ret = btrfs_init_workqueues(fs_info, fs_devices);
> @@ -3338,6 +3371,8 @@ int open_ctree(struct super_block *sb,
>  fail_sb_buffer:
>  	btrfs_stop_all_workers(fs_info);
>  	btrfs_free_block_groups(fs_info);
> +fail_csum:
> +	btrfs_free_csum_hash(fs_info);
>  fail_alloc:
>  fail_iput:
>  	btrfs_mapping_tree_free(&fs_info->mapping_tree);
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 04/17] btrfs: use btrfs_crc32c() instead of btrfs_name_hash()
  2019-05-10 12:56   ` Chris Mason
@ 2019-05-13  7:04     ` Johannes Thumshirn
  0 siblings, 0 replies; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-13  7:04 UTC (permalink / raw)
  To: Chris Mason; +Cc: David Sterba, Linux BTRFS Mailinglist

On Fri, May 10, 2019 at 12:56:54PM +0000, Chris Mason wrote:
> It groups together everyone using crc32c as a directory name hash (or 
> extref hash), so that if we ever want to add different hashes later down 
> the line, it's really clear what needs to change for what purpose.  With 
> everyone calling into btrfs_crc32c directly, you have to spend more time 
> looking through history to figure out how to change it.
> 
> Also, sprinkling (u32)~1 makes the code less readable imho.

Fair enough, thanks for the review.

	Johannes
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 05/17] btrfs: don't assume ordered sums to be 4 bytes
  2019-05-10 13:27     ` Nikolay Borisov
@ 2019-05-13  7:06       ` Johannes Thumshirn
  0 siblings, 0 replies; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-13  7:06 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: David Sterba, Linux BTRFS Mailinglist

On Fri, May 10, 2019 at 04:27:38PM +0300, Nikolay Borisov wrote:
> >>  			if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
> >>  				ret = btrfs_lookup_bio_sums(inode, comp_bio,
> >> -							    sums);
> >> +							    (u8 *)sums);
> > 
> > This cast (and other similar) are plain ugly. Imho we first need to get
> > the code into shape for later modification. This means postponing sha256
> > patch and instead focusing on first getting the code to use u8 where
> > relevant. Otherwise such cleanup is going to be postponed for "some time
> > in the future" will is unlikely to ever materialize.
> 
> Oh, so you fix that in the next patch. Okay, disregard this then.

Exactly. There are some intermediate temporary hacks in the prep patches of
the series, but they're not persistent.

Byte,
	Johannes
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 17/17] btrfs: add sha256 as another checksum algorithm
  2019-05-10 12:30   ` Nikolay Borisov
@ 2019-05-13  7:11     ` Johannes Thumshirn
  2019-05-13 12:54       ` David Sterba
  2019-05-13 12:55     ` David Sterba
  1 sibling, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-13  7:11 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: David Sterba, Linux BTRFS Mailinglist

On Fri, May 10, 2019 at 03:30:36PM +0300, Nikolay Borisov wrote:
[...]
> 
> nit: Might be a good idea to turn that into an enum for self-documenting
> purposes. Perhaps in a different patch.

Thought about this as well, but I thought I remembered a patch series from
David where he turned everything not being an on-disk format to enums.

This discuraged me from actually doing the switch. I'd be more than happy if I
could just use an enum.

Byte,
	Johannes
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 14/17] btrfs: directly call into crypto framework for checsumming
  2019-05-10 13:54     ` Chris Mason
@ 2019-05-13  7:17       ` Johannes Thumshirn
  2019-05-13 13:55         ` Chris Mason
  0 siblings, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-13  7:17 UTC (permalink / raw)
  To: Chris Mason; +Cc: David Sterba, Linux BTRFS Mailinglist

On Fri, May 10, 2019 at 01:54:30PM +0000, Chris Mason wrote:
> Looking at the callers of btrfs_csum_final() and btrfs_csum_data(), lets 
> just pass the shash_desc from the callers.  That way you don't have to 
> poke at memcpy and shash_desc_ctx().
 
I wanted to avoid spreading knowledge about the crypto api to deep into the
filesystem. I'd actually loved to have something akin to ubifs'
ubifs_info::log_hash but wasn't really sure how to handle concurrency.

> I'm a little worried about stack usage (at least 360 bytes), but worst 
> case we can do some percpu gymnastics.
> 
> -chris

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 17/17] btrfs: add sha256 as another checksum algorithm
  2019-05-13  7:11     ` Johannes Thumshirn
@ 2019-05-13 12:54       ` David Sterba
  2019-05-13 12:55         ` Johannes Thumshirn
  2019-05-15  1:45         ` Jeff Mahoney
  0 siblings, 2 replies; 60+ messages in thread
From: David Sterba @ 2019-05-13 12:54 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: Nikolay Borisov, Linux BTRFS Mailinglist

On Mon, May 13, 2019 at 09:11:14AM +0200, Johannes Thumshirn wrote:
> On Fri, May 10, 2019 at 03:30:36PM +0300, Nikolay Borisov wrote:
> [...]
> > 
> > nit: Might be a good idea to turn that into an enum for self-documenting
> > purposes. Perhaps in a different patch.
> 
> Thought about this as well, but I thought I remembered a patch series from
> David where he turned everything not being an on-disk format to enums.
> 
> This discuraged me from actually doing the switch. I'd be more than happy if I
> could just use an enum.

Enum is fine, but named constants that are part of on-disk format should
be spell the exact value, ie. not relying on the auto-increment of enum
values.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 17/17] btrfs: add sha256 as another checksum algorithm
  2019-05-13 12:54       ` David Sterba
@ 2019-05-13 12:55         ` Johannes Thumshirn
  2019-05-15  1:45         ` Jeff Mahoney
  1 sibling, 0 replies; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-13 12:55 UTC (permalink / raw)
  To: dsterba, Nikolay Borisov, Linux BTRFS Mailinglist

On Mon, May 13, 2019 at 02:54:38PM +0200, David Sterba wrote:
> Enum is fine, but named constants that are part of on-disk format should
> be spell the exact value, ie. not relying on the auto-increment of enum
> values.

Ah ok, got it.

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 17/17] btrfs: add sha256 as another checksum algorithm
  2019-05-10 12:30   ` Nikolay Borisov
  2019-05-13  7:11     ` Johannes Thumshirn
@ 2019-05-13 12:55     ` David Sterba
  2019-05-13 12:58       ` Johannes Thumshirn
  1 sibling, 1 reply; 60+ messages in thread
From: David Sterba @ 2019-05-13 12:55 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: Johannes Thumshirn, Linux BTRFS Mailinglist

On Fri, May 10, 2019 at 03:30:36PM +0300, Nikolay Borisov wrote:
> >  
> >  #define BTRFS_EMPTY_DIR_SIZE 0
> >  
> > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > index 2be8f05be1e6..bdcffa0d6b13 100644
> > --- a/fs/btrfs/disk-io.c
> > +++ b/fs/btrfs/disk-io.c
> > @@ -390,6 +390,8 @@ static bool btrfs_supported_super_csum(struct btrfs_super_block *sb)
> >  	switch (btrfs_super_csum_type(sb)) {
> >  	case BTRFS_CSUM_TYPE_CRC32:
> >  		return true;
> > +	case BTRFS_CSUM_TYPE_SHA256:
> > +		return true;
> 
> nit: case BTRFS_CSUM_TYPE_CRC32:
>      CASE BTRFS_CSUM_TYPE_SHA256:
>            return true;

I'm not sure if the -Wimplicit-fallthrough accepts that without the
annotation or not.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 15/17] btrfs: remove assumption about csum type form btrfs_csum_{data,final}()
  2019-05-10 11:15 ` [PATCH 15/17] btrfs: remove assumption about csum type form btrfs_csum_{data,final}() Johannes Thumshirn
@ 2019-05-13 12:56   ` David Sterba
  0 siblings, 0 replies; 60+ messages in thread
From: David Sterba @ 2019-05-13 12:56 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: Linux BTRFS Mailinglist

On Fri, May 10, 2019 at 01:15:45PM +0200, Johannes Thumshirn wrote:
> btrfs_csum_data() and btrfs_csum_final() still have assumptions on the
> checksums' type and size. Remove it so we can plumb in more types.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  fs/btrfs/check-integrity.c |  6 ++---
>  fs/btrfs/compression.c     | 13 ++++++-----
>  fs/btrfs/disk-io.c         | 58 +++++++++++++++++++++++++---------------------
>  fs/btrfs/disk-io.h         |  6 ++---
>  fs/btrfs/file-item.c       | 13 +++++------
>  fs/btrfs/inode.c           | 18 +++++++-------
>  fs/btrfs/scrub.c           | 20 +++++++++-------
>  7 files changed, 72 insertions(+), 62 deletions(-)
> 
> diff --git a/fs/btrfs/check-integrity.c b/fs/btrfs/check-integrity.c
> index 6aad0c3d197f..54c8a549b505 100644
> --- a/fs/btrfs/check-integrity.c
> +++ b/fs/btrfs/check-integrity.c
> @@ -1712,7 +1712,6 @@ static int btrfsic_test_for_metadata(struct btrfsic_state *state,
>  	struct btrfs_fs_info *fs_info = state->fs_info;
>  	struct btrfs_header *h;
>  	u8 csum[BTRFS_CSUM_SIZE];
> -	u32 crc = ~(u32)0;
>  	unsigned int i;
>  
>  	if (num_pages * PAGE_SIZE < state->metablock_size)
> @@ -1723,14 +1722,15 @@ static int btrfsic_test_for_metadata(struct btrfsic_state *state,
>  	if (memcmp(h->fsid, fs_info->fs_devices->fsid, BTRFS_FSID_SIZE))
>  		return 1;
>  
> +	memset(csum, 0xff, btrfs_super_csum_size(fs_info->super_copy));

This should be added to the csum API as eg. btrfs_csum_init and not
opencoded.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 17/17] btrfs: add sha256 as another checksum algorithm
  2019-05-13 12:55     ` David Sterba
@ 2019-05-13 12:58       ` Johannes Thumshirn
  0 siblings, 0 replies; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-13 12:58 UTC (permalink / raw)
  To: dsterba, Nikolay Borisov, Linux BTRFS Mailinglist

On Mon, May 13, 2019 at 02:55:56PM +0200, David Sterba wrote:
> > nit: case BTRFS_CSUM_TYPE_CRC32:
> >      CASE BTRFS_CSUM_TYPE_SHA256:
> >            return true;
> 
> I'm not sure if the -Wimplicit-fallthrough accepts that without the
> annotation or not.

Looks like it does:
jthumshirn@glass:linux (btrfs-csum-rework)$ git show fs/btrfs/disk-io.c
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 342f513db712..1e0000962b8a 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -389,6 +389,7 @@ static bool btrfs_supported_super_csum(struct btrfs_super_block *sb)
 {
        switch (btrfs_super_csum_type(sb)) {
        case BTRFS_CSUM_TYPE_CRC32:
+       case BTRFS_CSUM_TYPE_SHA256:
                return true;
        default:
                return false;

jthumshirn@glass:linux (btrfs-csum-rework)$ touch fs/btrfs/disk-io.c
jthumshirn@glass:linux (btrfs-csum-rework)$ make CC=gcc-7 -j 144 W=1 M=fs/btrfs
  CC [M]  fs/btrfs/disk-io.o
  LD [M]  fs/btrfs/btrfs.o
  Building modules, stage 2.
  MODPOST 1 modules
  LD [M]  fs/btrfs/btrfs.ko


-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply related	[flat|nested] 60+ messages in thread

* Re: [PATCH 14/17] btrfs: directly call into crypto framework for checsumming
  2019-05-10 11:15 ` [PATCH 14/17] btrfs: directly call into crypto framework for checsumming Johannes Thumshirn
  2019-05-10 13:45   ` Chris Mason
@ 2019-05-13 13:00   ` David Sterba
  2019-05-13 13:01     ` Johannes Thumshirn
  1 sibling, 1 reply; 60+ messages in thread
From: David Sterba @ 2019-05-13 13:00 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: Linux BTRFS Mailinglist

On Fri, May 10, 2019 at 01:15:44PM +0200, Johannes Thumshirn wrote:
> Currently btrfs_csum_data() relied on the crc32c() wrapper around the crypto
> framework for calculating the CRCs.
> 
> As we have our own crypto_shash structure in the fs_info now, we can
> directly call into the crypto framework without going trough the wrapper.
> 
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> ---
>  fs/btrfs/disk-io.c | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index fb0b06b7b9f6..0c9ac7b45ce8 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -253,12 +253,36 @@ struct extent_map *btree_get_extent(struct btrfs_inode *inode,
>  u32 btrfs_csum_data(struct btrfs_fs_info *fs_info, const char *data,
>  		    u32 seed, size_t len)
>  {
> -	return crc32c(seed, data, len);
> +	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> +	u32 *ctx = (u32 *)shash_desc_ctx(shash);
> +	u32 retval;
> +	int err;
> +
> +	shash->tfm = fs_info->csum_shash;
> +	shash->flags = 0;
> +	*ctx = seed;
> +
> +	err = crypto_shash_update(shash, data, len);
> +	ASSERT(err);
> +
> +	retval = *ctx;
> +	barrier_data(ctx);
> +
> +	return retval;

I unfortunatelly had a dive into the crypto API calls and since then I'm
biased against using it (too much indirection and extra memory
references). So my idea of this function is:

switch (fs_info->csum_type) {
case CRC32:
	... calculate crc32c;
	break;
case SHA256:
	... calculate sha56;
	break;
}

with direct calls to the hash primitives rather than thravelling trough
the shash.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 14/17] btrfs: directly call into crypto framework for checsumming
  2019-05-13 13:00   ` David Sterba
@ 2019-05-13 13:01     ` Johannes Thumshirn
  2019-05-13 14:30       ` David Sterba
  0 siblings, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-13 13:01 UTC (permalink / raw)
  To: dsterba, Linux BTRFS Mailinglist

On Mon, May 13, 2019 at 03:00:03PM +0200, David Sterba wrote:
> On Fri, May 10, 2019 at 01:15:44PM +0200, Johannes Thumshirn wrote:
> > Currently btrfs_csum_data() relied on the crc32c() wrapper around the crypto
> > framework for calculating the CRCs.
> > 
> > As we have our own crypto_shash structure in the fs_info now, we can
> > directly call into the crypto framework without going trough the wrapper.
> > 
> > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> > ---
> >  fs/btrfs/disk-io.c | 28 ++++++++++++++++++++++++++--
> >  1 file changed, 26 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > index fb0b06b7b9f6..0c9ac7b45ce8 100644
> > --- a/fs/btrfs/disk-io.c
> > +++ b/fs/btrfs/disk-io.c
> > @@ -253,12 +253,36 @@ struct extent_map *btree_get_extent(struct btrfs_inode *inode,
> >  u32 btrfs_csum_data(struct btrfs_fs_info *fs_info, const char *data,
> >  		    u32 seed, size_t len)
> >  {
> > -	return crc32c(seed, data, len);
> > +	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> > +	u32 *ctx = (u32 *)shash_desc_ctx(shash);
> > +	u32 retval;
> > +	int err;
> > +
> > +	shash->tfm = fs_info->csum_shash;
> > +	shash->flags = 0;
> > +	*ctx = seed;
> > +
> > +	err = crypto_shash_update(shash, data, len);
> > +	ASSERT(err);
> > +
> > +	retval = *ctx;
> > +	barrier_data(ctx);
> > +
> > +	return retval;
> 
> I unfortunatelly had a dive into the crypto API calls and since then I'm
> biased against using it (too much indirection and extra memory
> references). So my idea of this function is:
> 
> switch (fs_info->csum_type) {
> case CRC32:
> 	... calculate crc32c;
> 	break;
> case SHA256:
> 	... calculate sha56;
> 	break;
> }
> 
> with direct calls to the hash primitives rather than thravelling trough
> the shash.

well at least the crc3c2() call we use does nothing else (from
lib/libcrc32c.c):

u32 crc32c(u32 crc, const void *address, unsigned int length)
{
	SHASH_DESC_ON_STACK(shash, tfm);
	u32 ret, *ctx = (u32 *)shash_desc_ctx(shash);
	int err;

	shash->tfm = tfm;
	shash->flags = 0;
	*ctx = crc;

	err = crypto_shash_update(shash, address, length);
	BUG_ON(err);

	ret = *ctx;
	barrier_data(ctx);
	return ret;
}

EXPORT_SYMBOL(crc32c);


-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 14/17] btrfs: directly call into crypto framework for checsumming
  2019-05-13  7:17       ` Johannes Thumshirn
@ 2019-05-13 13:55         ` Chris Mason
  0 siblings, 0 replies; 60+ messages in thread
From: Chris Mason @ 2019-05-13 13:55 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: David Sterba, Linux BTRFS Mailinglist

On 13 May 2019, at 3:17, Johannes Thumshirn wrote:

> On Fri, May 10, 2019 at 01:54:30PM +0000, Chris Mason wrote:
>> Looking at the callers of btrfs_csum_final() and btrfs_csum_data(), 
>> lets
>> just pass the shash_desc from the callers.  That way you don't have 
>> to
>> poke at memcpy and shash_desc_ctx().
>
> I wanted to avoid spreading knowledge about the crypto api to deep 
> into the
> filesystem. I'd actually loved to have something akin to ubifs'
> ubifs_info::log_hash but wasn't really sure how to handle concurrency.

It's much better to be explicit that we're using the crypto API than to 
muck around in crypto api internal data structs.  It's just a few 
callers, and not major surgery to change it around some time later if we 
need to.

-chris

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 14/17] btrfs: directly call into crypto framework for checsumming
  2019-05-13 13:01     ` Johannes Thumshirn
@ 2019-05-13 14:30       ` David Sterba
  0 siblings, 0 replies; 60+ messages in thread
From: David Sterba @ 2019-05-13 14:30 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: dsterba, Linux BTRFS Mailinglist

On Mon, May 13, 2019 at 03:01:11PM +0200, Johannes Thumshirn wrote:
> On Mon, May 13, 2019 at 03:00:03PM +0200, David Sterba wrote:
> > On Fri, May 10, 2019 at 01:15:44PM +0200, Johannes Thumshirn wrote:
> > > Currently btrfs_csum_data() relied on the crc32c() wrapper around the crypto
> > > framework for calculating the CRCs.
> > > 
> > > As we have our own crypto_shash structure in the fs_info now, we can
> > > directly call into the crypto framework without going trough the wrapper.
> > > 
> > > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
> > > ---
> > >  fs/btrfs/disk-io.c | 28 ++++++++++++++++++++++++++--
> > >  1 file changed, 26 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > > index fb0b06b7b9f6..0c9ac7b45ce8 100644
> > > --- a/fs/btrfs/disk-io.c
> > > +++ b/fs/btrfs/disk-io.c
> > > @@ -253,12 +253,36 @@ struct extent_map *btree_get_extent(struct btrfs_inode *inode,
> > >  u32 btrfs_csum_data(struct btrfs_fs_info *fs_info, const char *data,
> > >  		    u32 seed, size_t len)
> > >  {
> > > -	return crc32c(seed, data, len);
> > > +	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> > > +	u32 *ctx = (u32 *)shash_desc_ctx(shash);
> > > +	u32 retval;
> > > +	int err;
> > > +
> > > +	shash->tfm = fs_info->csum_shash;
> > > +	shash->flags = 0;
> > > +	*ctx = seed;
> > > +
> > > +	err = crypto_shash_update(shash, data, len);
> > > +	ASSERT(err);
> > > +
> > > +	retval = *ctx;
> > > +	barrier_data(ctx);
> > > +
> > > +	return retval;
> > 
> > I unfortunatelly had a dive into the crypto API calls and since then I'm
> > biased against using it (too much indirection and extra memory
> > references). So my idea of this function is:
> > 
> > switch (fs_info->csum_type) {
> > case CRC32:
> > 	... calculate crc32c;
> > 	break;
> > case SHA256:
> > 	... calculate sha56;
> > 	break;
> > }
> > 
> > with direct calls to the hash primitives rather than thravelling trough
> > the shash.
> 
> well at least the crc3c2() call we use does nothing else (from
> lib/libcrc32c.c):

Oh I see, and we actually can't avoid the crypto api because we want to
let it select the implementation based on acceleration hw or use the
fallback. Ok then.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 14/17] btrfs: directly call into crypto framework for checsumming
  2019-05-10 13:45   ` Chris Mason
  2019-05-10 13:54     ` Chris Mason
@ 2019-05-14 12:46     ` Johannes Thumshirn
  1 sibling, 0 replies; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-14 12:46 UTC (permalink / raw)
  To: Chris Mason; +Cc: David Sterba, Linux BTRFS Mailinglist

On Fri, May 10, 2019 at 01:45:47PM +0000, Chris Mason wrote:
> I'm a little concerned about about btrfs_csum_data() and 
> btrfs_csum_final() below.  We're using two different 
> SHASH_DESC_ON_STACK() and then overwriting internals (shash_desc_ctx()) 
> with the assumption that whatever we're doing is going to be the same as 
> using the same shash_desc struct for both the update and final calls.  I 
> think we should be either using or adding a helper to the crypto api for 
> this.  We're digging too deep into cryptoapi private structs with the 
> current usage.
 
I think I found the solution. Instead of doing memset() + memcpy() +
crypto_shash_update() + crypto_shash_final(), I could call
crypto_shash_digest() which would wrap all of them for me in both the crc32c
and sha256 case.

Let me have a look and throw it in some testing.

> Otherwise, thanks for doing this, it looks great overall.

Thanks :)
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 17/17] btrfs: add sha256 as another checksum algorithm
  2019-05-13 12:54       ` David Sterba
  2019-05-13 12:55         ` Johannes Thumshirn
@ 2019-05-15  1:45         ` Jeff Mahoney
  1 sibling, 0 replies; 60+ messages in thread
From: Jeff Mahoney @ 2019-05-15  1:45 UTC (permalink / raw)
  To: dsterba, Johannes Thumshirn, Nikolay Borisov, Linux BTRFS Mailinglist

On 5/13/19 8:54 AM, David Sterba wrote:
> On Mon, May 13, 2019 at 09:11:14AM +0200, Johannes Thumshirn wrote:
>> On Fri, May 10, 2019 at 03:30:36PM +0300, Nikolay Borisov wrote:
>> [...]
>>>
>>> nit: Might be a good idea to turn that into an enum for self-documenting
>>> purposes. Perhaps in a different patch.
>>
>> Thought about this as well, but I thought I remembered a patch series from
>> David where he turned everything not being an on-disk format to enums.
>>
>> This discuraged me from actually doing the switch. I'd be more than happy if I
>> could just use an enum.
> 
> Enum is fine, but named constants that are part of on-disk format should
> be spell the exact value, ie. not relying on the auto-increment of enum
> values.
> 

FWIW, enum values make it into the debuginfo, so we can see the values
as names when we load up a dump in a debugger.

-Jeff

-- 
Jeff Mahoney
SUSE Labs

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 00/17] Add support for SHA-256 checksums
  2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
                   ` (16 preceding siblings ...)
  2019-05-10 11:15 ` [PATCH 17/17] btrfs: add sha256 as another checksum algorithm Johannes Thumshirn
@ 2019-05-15 17:27 ` David Sterba
  2019-05-16  6:30   ` Paul Jones
  2019-05-17 18:36   ` Diego Calleja
  17 siblings, 2 replies; 60+ messages in thread
From: David Sterba @ 2019-05-15 17:27 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: David Sterba, Linux BTRFS Mailinglist

On Fri, May 10, 2019 at 01:15:30PM +0200, Johannes Thumshirn wrote:
> This patchset add support for adding new checksum types in BTRFS.
> 
> Currently BTRFS only supports CRC32C as data and metadata checksum, which is
> good if you only want to detect errors due to data corruption in hardware.
> 
> But CRC32C isn't able cover other use-cases like de-duplication or
> cryptographically save data integrity guarantees.
> 
> The following properties made SHA-256 interesting for these use-cases:
> - Still considered cryptographically sound
> - Reasonably well understood by the security industry
> - Result fits into the 32Byte/256Bit we have for the checksum in the on-disk
>   format
> - Small enough collision space to make it feasible for data de-duplication
> - Fast enough to calculate and offloadable to crypto hardware via the kernel's
>   crypto_shash framework.
> 
> The patchset also provides mechanisms for plumbing in different hash
> algorithms relatively easy.

Once the code is ready for more checksum algos, we'll pick candidates
and my idea is to select 1 fast (not necessarily strong, but better
than crc32c) and 1 strong (but slow, and sha256 is the candidate at the
moment).

The discussion from 2014 on that topic brought a lot of useful
information, though some algos have could have evolved since.

https://lore.kernel.org/linux-btrfs/1416806586-18050-1-git-send-email-bo.li.liu@oracle.com/

In about 5 years timeframe we can revisit the algos and potentially add
more, so I hope we'll be able to agree to add just 2 in this round.

The minimum selection criteria for a digest algorithm:

- is provided by linux kernel crypto subsystem
- has a license that will allow to use it in bootloader code (grub at
  lest)
- the implementation is available for btrfs-progs either as some small
  library or can be used directly as a .c file

^ permalink raw reply	[flat|nested] 60+ messages in thread

* RE: [PATCH 00/17] Add support for SHA-256 checksums
  2019-05-15 17:27 ` [PATCH 00/17] Add support for SHA-256 checksums David Sterba
@ 2019-05-16  6:30   ` Paul Jones
  2019-05-16  8:16     ` Nikolay Borisov
  2019-05-17 18:36   ` Diego Calleja
  1 sibling, 1 reply; 60+ messages in thread
From: Paul Jones @ 2019-05-16  6:30 UTC (permalink / raw)
  To: dsterba, Johannes Thumshirn; +Cc: David Sterba, Linux BTRFS Mailinglist


> -----Original Message-----
> From: linux-btrfs-owner@vger.kernel.org <linux-btrfs-
> owner@vger.kernel.org> On Behalf Of David Sterba
> Sent: Thursday, 16 May 2019 3:27 AM
> To: Johannes Thumshirn <jthumshirn@suse.de>
> Cc: David Sterba <dsterba@suse.com>; Linux BTRFS Mailinglist <linux-
> btrfs@vger.kernel.org>
> Subject: Re: [PATCH 00/17] Add support for SHA-256 checksums
> 
> 
> Once the code is ready for more checksum algos, we'll pick candidates and
> my idea is to select 1 fast (not necessarily strong, but better than crc32c) and
> 1 strong (but slow, and sha256 is the candidate at the moment).
> 
> The discussion from 2014 on that topic brought a lot of useful information,
> though some algos have could have evolved since.
> 
> https://lore.kernel.org/linux-btrfs/1416806586-18050-1-git-send-email-
> bo.li.liu@oracle.com/
> 
> In about 5 years timeframe we can revisit the algos and potentially add more,
> so I hope we'll be able to agree to add just 2 in this round.
> 
> The minimum selection criteria for a digest algorithm:
> 
> - is provided by linux kernel crypto subsystem
> - has a license that will allow to use it in bootloader code (grub at
>   lest)
> - the implementation is available for btrfs-progs either as some small
>   library or can be used directly as a .c file


Xxhash would be a good candidate. It's extremely fast and almost crypto secure.  Has been in the kernel for ~2 yeas iirc.


Paul.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 00/17] Add support for SHA-256 checksums
  2019-05-16  6:30   ` Paul Jones
@ 2019-05-16  8:16     ` Nikolay Borisov
  2019-05-16  8:20       ` Johannes Thumshirn
  0 siblings, 1 reply; 60+ messages in thread
From: Nikolay Borisov @ 2019-05-16  8:16 UTC (permalink / raw)
  To: Paul Jones, dsterba, Johannes Thumshirn
  Cc: David Sterba, Linux BTRFS Mailinglist



On 16.05.19 г. 9:30 ч., Paul Jones wrote:
> 
>> -----Original Message-----
>> From: linux-btrfs-owner@vger.kernel.org <linux-btrfs-
>> owner@vger.kernel.org> On Behalf Of David Sterba
>> Sent: Thursday, 16 May 2019 3:27 AM
>> To: Johannes Thumshirn <jthumshirn@suse.de>
>> Cc: David Sterba <dsterba@suse.com>; Linux BTRFS Mailinglist <linux-
>> btrfs@vger.kernel.org>
>> Subject: Re: [PATCH 00/17] Add support for SHA-256 checksums
>>
>>
>> Once the code is ready for more checksum algos, we'll pick candidates and
>> my idea is to select 1 fast (not necessarily strong, but better than crc32c) and
>> 1 strong (but slow, and sha256 is the candidate at the moment).
>>
>> The discussion from 2014 on that topic brought a lot of useful information,
>> though some algos have could have evolved since.
>>
>> https://lore.kernel.org/linux-btrfs/1416806586-18050-1-git-send-email-
>> bo.li.liu@oracle.com/
>>
>> In about 5 years timeframe we can revisit the algos and potentially add more,
>> so I hope we'll be able to agree to add just 2 in this round.
>>
>> The minimum selection criteria for a digest algorithm:
>>
>> - is provided by linux kernel crypto subsystem
>> - has a license that will allow to use it in bootloader code (grub at
>>   lest)
>> - the implementation is available for btrfs-progs either as some small
>>   library or can be used directly as a .c file
> 
> 
> Xxhash would be a good candidate. It's extremely fast and almost crypto secure.  Has been in the kernel for ~2 yeas iirc.

Disclaimer: not a cryptographer. But according to the official site:
xxHash is non-cryptography hash. From the (draft) spec:

It is labelled non-cryptographic, and is not meant to avoid intentional
collisions (same digest for 2 different messages), or to prevent
producing a message with predefined digest.

This doesn't disqualify it, however we need to be aware its limitations.
Perhahps it could be used as a replacement for crc32c but definitely not
as secure crypto hash.

> 
> 
> Paul.
> 

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 00/17] Add support for SHA-256 checksums
  2019-05-16  8:16     ` Nikolay Borisov
@ 2019-05-16  8:20       ` Johannes Thumshirn
  0 siblings, 0 replies; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-16  8:20 UTC (permalink / raw)
  To: Nikolay Borisov
  Cc: Paul Jones, dsterba, David Sterba, Linux BTRFS Mailinglist

On Thu, May 16, 2019 at 11:16:38AM +0300, Nikolay Borisov wrote:
> It is labelled non-cryptographic, and is not meant to avoid intentional
> collisions (same digest for 2 different messages), or to prevent
> producing a message with predefined digest.
> 
> This doesn't disqualify it, however we need to be aware its limitations.
> Perhahps it could be used as a replacement for crc32c but definitely not
> as secure crypto hash.

Agreed, but David's plan was to have 3 hashes and xx seems like a good fit for
the 3rd fast, stronger than crc32c but not cryptographically secure option.

I'll be looking into it for v3.

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 00/17] Add support for SHA-256 checksums
  2019-05-15 17:27 ` [PATCH 00/17] Add support for SHA-256 checksums David Sterba
  2019-05-16  6:30   ` Paul Jones
@ 2019-05-17 18:36   ` Diego Calleja
  2019-05-17 19:07     ` Johannes Thumshirn
                       ` (2 more replies)
  1 sibling, 3 replies; 60+ messages in thread
From: Diego Calleja @ 2019-05-17 18:36 UTC (permalink / raw)
  To: dsterba; +Cc: Johannes Thumshirn, David Sterba, Linux BTRFS Mailinglist

El miércoles, 15 de mayo de 2019 19:27:21 (CEST) David Sterba escribió:
> Once the code is ready for more checksum algos, we'll pick candidates
> and my idea is to select 1 fast (not necessarily strong, but better
> than crc32c) and 1 strong (but slow, and sha256 is the candidate at the
> moment)

Modern CPUs have SHA256 instructions, it is actually that slow? (not sure how 
fast these instructions are)

If btrfs needs an algorithm with good performance/security ratio, I would 
suggest considering BLAKE2 [1]. It is based in the BLAKE algorithm that made 
to the final round in the SHA3 competition, it is considered pretty secure 
(above SHA2 at least), and it was designed to take advantage of modern CPU 
features and be as fast as possible - it even beats SHA1 in that regard. It is 
not currently in the kernel but Wireguard uses it and will add an 
implementation when it's merged (but Wireguard doesn't use the crypto layer 
for some reason...)



^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 00/17] Add support for SHA-256 checksums
  2019-05-17 18:36   ` Diego Calleja
@ 2019-05-17 19:07     ` Johannes Thumshirn
  2019-05-18  0:38       ` Adam Borowski
  2019-05-20 11:42     ` Austin S. Hemmelgarn
  2019-05-30 12:21     ` David Sterba
  2 siblings, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-17 19:07 UTC (permalink / raw)
  To: Diego Calleja; +Cc: dsterba, David Sterba, Linux BTRFS Mailinglist

On Fri, May 17, 2019 at 08:36:23PM +0200, Diego Calleja wrote:
> Modern CPUs have SHA256 instructions, it is actually that slow? (not sure how 
> fast these instructions are)

This still is subject to evaluation.

> If btrfs needs an algorithm with good performance/security ratio, I would 
> suggest considering BLAKE2 [1]. It is based in the BLAKE algorithm that made 
> to the final round in the SHA3 competition, it is considered pretty secure 
> (above SHA2 at least), and it was designed to take advantage of modern CPU 
> features and be as fast as possible - it even beats SHA1 in that regard. It is 
> not currently in the kernel but Wireguard uses it and will add an 
> implementation when it's merged (but Wireguard doesn't use the crypto layer 
> for some reason...)

SHA3 is on my list of other candidates to look at for a performance
evaluation. As for BLAKE2 I haven't done too much research on it and I'm not a
cryptographer so I have to trust FIPS et al.

One other (non chrypto) hash that is often mentioned would be XXHash which is
in the kernel but not yet wired up to the kernel's crypto framework, but this
shouldn't be too hard to do.

Byte,
	Johannes
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 00/17] Add support for SHA-256 checksums
  2019-05-17 19:07     ` Johannes Thumshirn
@ 2019-05-18  0:38       ` Adam Borowski
  2019-05-20  7:47         ` Johannes Thumshirn
  0 siblings, 1 reply; 60+ messages in thread
From: Adam Borowski @ 2019-05-18  0:38 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: Diego Calleja, dsterba, David Sterba, Linux BTRFS Mailinglist

On Fri, May 17, 2019 at 09:07:03PM +0200, Johannes Thumshirn wrote:
> On Fri, May 17, 2019 at 08:36:23PM +0200, Diego Calleja wrote:
> > If btrfs needs an algorithm with good performance/security ratio, I would 
> > suggest considering BLAKE2 [1]. It is based in the BLAKE algorithm that made 
> > to the final round in the SHA3 competition, it is considered pretty secure 
> > (above SHA2 at least), and it was designed to take advantage of modern CPU 
> > features and be as fast as possible - it even beats SHA1 in that regard. It is 
> > not currently in the kernel but Wireguard uses it and will add an 
> > implementation when it's merged (but Wireguard doesn't use the crypto layer 
> > for some reason...)
> 
> SHA3 is on my list of other candidates to look at for a performance
> evaluation. As for BLAKE2 I haven't done too much research on it and I'm not a
> cryptographer so I have to trust FIPS et al.

"Trust FIPS" is the main problem here.  Until recently, FIPS certification
required implementing this nice random generator:
    https://en.wikipedia.org/wiki/Dual_EC_DRBG

Thus, a good part of people are reluctant to use hash functions chosen by
NIST (and published as FIPS).

BLAKE2 is also a good deal faster on most hardware:
    https://bench.cr.yp.to/results-sha3.html
Even with sha_ni, SHA256 wins only on Zen AMDs: sha_ni equipped Intels have
superior SIMD thus BLAKE2 is still faster.  And without sha_ni, the
difference is drastic.


Meow!
-- 
⢀⣴⠾⠻⢶⣦⠀ Latin:   meow 4 characters, 4 columns,  4 bytes
⣾⠁⢠⠒⠀⣿⡁ Greek:   μεου 4 characters, 4 columns,  8 bytes
⢿⡄⠘⠷⠚⠋  Runes:   ᛗᛖᛟᚹ 4 characters, 4 columns, 12 bytes
⠈⠳⣄⠀⠀⠀⠀ Chinese: 喵   1 character,  2 columns,  3 bytes <-- best!

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 00/17] Add support for SHA-256 checksums
  2019-05-18  0:38       ` Adam Borowski
@ 2019-05-20  7:47         ` Johannes Thumshirn
  2019-05-20 11:34           ` Austin S. Hemmelgarn
  0 siblings, 1 reply; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-20  7:47 UTC (permalink / raw)
  To: Adam Borowski
  Cc: Diego Calleja, dsterba, David Sterba, Linux BTRFS Mailinglist

On Sat, May 18, 2019 at 02:38:08AM +0200, Adam Borowski wrote:
> On Fri, May 17, 2019 at 09:07:03PM +0200, Johannes Thumshirn wrote:
> > On Fri, May 17, 2019 at 08:36:23PM +0200, Diego Calleja wrote:
> > > If btrfs needs an algorithm with good performance/security ratio, I would 
> > > suggest considering BLAKE2 [1]. It is based in the BLAKE algorithm that made 
> > > to the final round in the SHA3 competition, it is considered pretty secure 
> > > (above SHA2 at least), and it was designed to take advantage of modern CPU 
> > > features and be as fast as possible - it even beats SHA1 in that regard. It is 
> > > not currently in the kernel but Wireguard uses it and will add an 
> > > implementation when it's merged (but Wireguard doesn't use the crypto layer 
> > > for some reason...)
> > 
> > SHA3 is on my list of other candidates to look at for a performance
> > evaluation. As for BLAKE2 I haven't done too much research on it and I'm not a
> > cryptographer so I have to trust FIPS et al.
> 
> "Trust FIPS" is the main problem here.  Until recently, FIPS certification
> required implementing this nice random generator:
>     https://en.wikipedia.org/wiki/Dual_EC_DRBG
> 
> Thus, a good part of people are reluctant to use hash functions chosen by
> NIST (and published as FIPS).

I know, but please also understand that there are applications which do
require FIPS certified algorithms.

Byte,
	Johannes
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 00/17] Add support for SHA-256 checksums
  2019-05-20  7:47         ` Johannes Thumshirn
@ 2019-05-20 11:34           ` Austin S. Hemmelgarn
  2019-05-20 11:57             ` Johannes Thumshirn
  0 siblings, 1 reply; 60+ messages in thread
From: Austin S. Hemmelgarn @ 2019-05-20 11:34 UTC (permalink / raw)
  To: Johannes Thumshirn, Adam Borowski
  Cc: Diego Calleja, dsterba, David Sterba, Linux BTRFS Mailinglist

On 2019-05-20 03:47, Johannes Thumshirn wrote:
> On Sat, May 18, 2019 at 02:38:08AM +0200, Adam Borowski wrote:
>> On Fri, May 17, 2019 at 09:07:03PM +0200, Johannes Thumshirn wrote:
>>> On Fri, May 17, 2019 at 08:36:23PM +0200, Diego Calleja wrote:
>>>> If btrfs needs an algorithm with good performance/security ratio, I would
>>>> suggest considering BLAKE2 [1]. It is based in the BLAKE algorithm that made
>>>> to the final round in the SHA3 competition, it is considered pretty secure
>>>> (above SHA2 at least), and it was designed to take advantage of modern CPU
>>>> features and be as fast as possible - it even beats SHA1 in that regard. It is
>>>> not currently in the kernel but Wireguard uses it and will add an
>>>> implementation when it's merged (but Wireguard doesn't use the crypto layer
>>>> for some reason...)
>>>
>>> SHA3 is on my list of other candidates to look at for a performance
>>> evaluation. As for BLAKE2 I haven't done too much research on it and I'm not a
>>> cryptographer so I have to trust FIPS et al.
>>
>> "Trust FIPS" is the main problem here.  Until recently, FIPS certification
>> required implementing this nice random generator:
>>      https://en.wikipedia.org/wiki/Dual_EC_DRBG
>>
>> Thus, a good part of people are reluctant to use hash functions chosen by
>> NIST (and published as FIPS).
> 
> I know, but please also understand that there are applications which do
> require FIPS certified algorithms.
Those would also be cryptographic applications, which BTRFS is not.  If 
you're in one of those situations and need to have cryptographic 
verification of files on the system, you need to be using either IMA, 
dm-verity, or dm-integrity.

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 00/17] Add support for SHA-256 checksums
  2019-05-17 18:36   ` Diego Calleja
  2019-05-17 19:07     ` Johannes Thumshirn
@ 2019-05-20 11:42     ` Austin S. Hemmelgarn
  2019-05-30 12:21     ` David Sterba
  2 siblings, 0 replies; 60+ messages in thread
From: Austin S. Hemmelgarn @ 2019-05-20 11:42 UTC (permalink / raw)
  To: Diego Calleja, dsterba
  Cc: Johannes Thumshirn, David Sterba, Linux BTRFS Mailinglist

On 2019-05-17 14:36, Diego Calleja wrote:
> El miércoles, 15 de mayo de 2019 19:27:21 (CEST) David Sterba escribió:
>> Once the code is ready for more checksum algos, we'll pick candidates
>> and my idea is to select 1 fast (not necessarily strong, but better
>> than crc32c) and 1 strong (but slow, and sha256 is the candidate at the
>> moment)
> 
> Modern CPUs have SHA256 instructions, it is actually that slow? (not sure how
> fast these instructions are)
> 
> If btrfs needs an algorithm with good performance/security ratio, I would
> suggest considering BLAKE2 [1]. It is based in the BLAKE algorithm that made
> to the final round in the SHA3 competition, it is considered pretty secure
> (above SHA2 at least), and it was designed to take advantage of modern CPU
> features and be as fast as possible - it even beats SHA1 in that regard. It is
> not currently in the kernel but Wireguard uses it and will add an
> implementation when it's merged (but Wireguard doesn't use the crypto layer
> for some reason...)
If anything, I'd argue for BLAKE2 instead of SHA256 as the 'slow' hash, 
as it's got equivalent or better strength but runs significantly faster.

For the fast hash, we should probably be looking more at stuff like 
xxhash or murmur3, both of which make CRC32c look slow by comparison (at 
least, when you don't have hardware acceleration for the CRC calculations).

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 00/17] Add support for SHA-256 checksums
  2019-05-20 11:34           ` Austin S. Hemmelgarn
@ 2019-05-20 11:57             ` Johannes Thumshirn
  0 siblings, 0 replies; 60+ messages in thread
From: Johannes Thumshirn @ 2019-05-20 11:57 UTC (permalink / raw)
  To: Austin S. Hemmelgarn
  Cc: Adam Borowski, Diego Calleja, dsterba, David Sterba,
	Linux BTRFS Mailinglist

On Mon, May 20, 2019 at 07:34:34AM -0400, Austin S. Hemmelgarn wrote:
> Those would also be cryptographic applications, which BTRFS is not.  If
> you're in one of those situations and need to have cryptographic
> verification of files on the system, you need to be using either IMA,
> dm-verity, or dm-integrity.

This is a system we're aiming at in the followups to this series, but haven't
ultimately validated the design yet.

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 60+ messages in thread

* Re: [PATCH 00/17] Add support for SHA-256 checksums
  2019-05-17 18:36   ` Diego Calleja
  2019-05-17 19:07     ` Johannes Thumshirn
  2019-05-20 11:42     ` Austin S. Hemmelgarn
@ 2019-05-30 12:21     ` David Sterba
  2 siblings, 0 replies; 60+ messages in thread
From: David Sterba @ 2019-05-30 12:21 UTC (permalink / raw)
  To: Diego Calleja
  Cc: dsterba, Johannes Thumshirn, David Sterba, Linux BTRFS Mailinglist

On Fri, May 17, 2019 at 08:36:23PM +0200, Diego Calleja wrote:
> El miércoles, 15 de mayo de 2019 19:27:21 (CEST) David Sterba escribió:
> > Once the code is ready for more checksum algos, we'll pick candidates
> > and my idea is to select 1 fast (not necessarily strong, but better
> > than crc32c) and 1 strong (but slow, and sha256 is the candidate at the
> > moment)
> 
> Modern CPUs have SHA256 instructions, it is actually that slow? (not sure how 
> fast these instructions are)
> 
> If btrfs needs an algorithm with good performance/security ratio, I would 
> suggest considering BLAKE2 [1]. It is based in the BLAKE algorithm that made 
> to the final round in the SHA3 competition, it is considered pretty secure 
> (above SHA2 at least), and it was designed to take advantage of modern CPU 
> features and be as fast as possible - it even beats SHA1 in that regard. It is 
> not currently in the kernel but Wireguard uses it and will add an 
> implementation when it's merged (but Wireguard doesn't use the crypto layer 
> for some reason...)

BLAKE2 looks as a good candidate. I have a glue code to export it as the
crypto module so we'll be able to test it at least. I'm not sure about
SHA3 due to the performance reasons, it comes out slower than SHA256 and
that one is already considered slow.

^ permalink raw reply	[flat|nested] 60+ messages in thread

end of thread, other threads:[~2019-05-30 12:20 UTC | newest]

Thread overview: 60+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-10 11:15 [PATCH 00/17] Add support for SHA-256 checksums Johannes Thumshirn
2019-05-10 11:15 ` [PATCH 01/17] btrfs: use btrfs_csum_data() instead of directly calling crc32c Johannes Thumshirn
2019-05-10 16:16   ` Nikolay Borisov
2019-05-10 11:15 ` [PATCH 02/17] btrfs: resurrect btrfs_crc32c() Johannes Thumshirn
2019-05-10 16:16   ` Nikolay Borisov
2019-05-10 11:15 ` [PATCH 03/17] btrfs: use btrfs_crc32c() instead of btrfs_extref_hash() Johannes Thumshirn
2019-05-10 13:03   ` Nikolay Borisov
2019-05-10 11:15 ` [PATCH 04/17] btrfs: use btrfs_crc32c() instead of btrfs_name_hash() Johannes Thumshirn
2019-05-10 12:56   ` Chris Mason
2019-05-13  7:04     ` Johannes Thumshirn
2019-05-10 11:15 ` [PATCH 05/17] btrfs: don't assume ordered sums to be 4 bytes Johannes Thumshirn
2019-05-10 13:25   ` Nikolay Borisov
2019-05-10 13:27     ` Nikolay Borisov
2019-05-13  7:06       ` Johannes Thumshirn
2019-05-10 11:15 ` [PATCH 06/17] btrfs: dont assume compressed_bio " Johannes Thumshirn
2019-05-10 11:15 ` [PATCH 07/17] btrfs: use btrfs_crc32c{,_final}() in for free space cache Johannes Thumshirn
2019-05-10 13:27   ` Nikolay Borisov
2019-05-10 11:15 ` [PATCH 08/17] btrfs: format checksums according to type for printing Johannes Thumshirn
2019-05-10 13:28   ` Nikolay Borisov
2019-05-10 11:15 ` [PATCH 09/17] btrfs: add common checksum type validation Johannes Thumshirn
2019-05-10 13:37   ` Nikolay Borisov
2019-05-10 11:15 ` [PATCH 10/17] btrfs: check for supported superblock checksum type before checksum validation Johannes Thumshirn
2019-05-10 13:37   ` Nikolay Borisov
2019-05-10 11:15 ` [PATCH 11/17] btrfs: Simplify btrfs_check_super_csum() and get rid of size assumptions Johannes Thumshirn
2019-05-10 13:41   ` Nikolay Borisov
2019-05-10 11:15 ` [PATCH 12/17] btrfs: add boilerplate code for directly including the crypto framework Johannes Thumshirn
2019-05-10 16:28   ` Nikolay Borisov
2019-05-10 11:15 ` [PATCH 13/17] btrfs: pass in an fs_info to btrfs_csum_{data,final}() Johannes Thumshirn
2019-05-10 11:15 ` [PATCH 14/17] btrfs: directly call into crypto framework for checsumming Johannes Thumshirn
2019-05-10 13:45   ` Chris Mason
2019-05-10 13:54     ` Chris Mason
2019-05-13  7:17       ` Johannes Thumshirn
2019-05-13 13:55         ` Chris Mason
2019-05-14 12:46     ` Johannes Thumshirn
2019-05-13 13:00   ` David Sterba
2019-05-13 13:01     ` Johannes Thumshirn
2019-05-13 14:30       ` David Sterba
2019-05-10 11:15 ` [PATCH 15/17] btrfs: remove assumption about csum type form btrfs_csum_{data,final}() Johannes Thumshirn
2019-05-13 12:56   ` David Sterba
2019-05-10 11:15 ` [PATCH 16/17] btrfs: remove assumption about csum type form btrfs_print_data_csum_error() Johannes Thumshirn
2019-05-10 11:15 ` [PATCH 17/17] btrfs: add sha256 as another checksum algorithm Johannes Thumshirn
2019-05-10 12:30   ` Nikolay Borisov
2019-05-13  7:11     ` Johannes Thumshirn
2019-05-13 12:54       ` David Sterba
2019-05-13 12:55         ` Johannes Thumshirn
2019-05-15  1:45         ` Jeff Mahoney
2019-05-13 12:55     ` David Sterba
2019-05-13 12:58       ` Johannes Thumshirn
2019-05-15 17:27 ` [PATCH 00/17] Add support for SHA-256 checksums David Sterba
2019-05-16  6:30   ` Paul Jones
2019-05-16  8:16     ` Nikolay Borisov
2019-05-16  8:20       ` Johannes Thumshirn
2019-05-17 18:36   ` Diego Calleja
2019-05-17 19:07     ` Johannes Thumshirn
2019-05-18  0:38       ` Adam Borowski
2019-05-20  7:47         ` Johannes Thumshirn
2019-05-20 11:34           ` Austin S. Hemmelgarn
2019-05-20 11:57             ` Johannes Thumshirn
2019-05-20 11:42     ` Austin S. Hemmelgarn
2019-05-30 12:21     ` David Sterba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).