From: Nikolay Borisov <nborisov@suse.com>
To: Johannes Thumshirn <jthumshirn@suse.de>, David Sterba <dsterba@suse.com>
Cc: Linux BTRFS Mailinglist <linux-btrfs@vger.kernel.org>
Subject: Re: [PATCH 05/17] btrfs: don't assume ordered sums to be 4 bytes
Date: Fri, 10 May 2019 16:27:38 +0300 [thread overview]
Message-ID: <12f8e39d-99e5-3244-a61a-71819ed5586f@suse.com> (raw)
In-Reply-To: <9f6864d5-9659-a121-5de1-3e5993eabef8@suse.com>
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);
>>
>
next prev parent reply other threads:[~2019-05-10 13:27 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=12f8e39d-99e5-3244-a61a-71819ed5586f@suse.com \
--to=nborisov@suse.com \
--cc=dsterba@suse.com \
--cc=jthumshirn@suse.de \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).