From: Qu Wenruo <quwenruo.btrfs@gmx.com>
To: Nikolay Borisov <nborisov@suse.com>, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 3/3] btrfs-progs: Remove convert param from btrfs_alloc_data_chunk
Date: Wed, 30 Oct 2019 20:45:16 +0800 [thread overview]
Message-ID: <d90e7757-f646-87f7-a729-8c3652154721@gmx.com> (raw)
In-Reply-To: <20191030122227.28496-4-nborisov@suse.com>
On 2019/10/30 下午8:22, Nikolay Borisov wrote:
> convert is always set to true so there's no point in having it as a
> function parameter or using it as a predicate inside btrfs_alloc_data_chunk.
> Remove it and all relevant code which would have never been executed.
> No semantics changes.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Just one tip for further patches.
The function btrfs_alloc_data_chunk() is purely used for convert to
create a 1:1 mapped data chunk.
It would be be even better to rename the function to indicate it better.
Thanks,
Qu
> ---
> convert/main.c | 3 +--
> volumes.c | 44 ++++++++++++++------------------------------
> volumes.h | 3 +--
> 3 files changed, 16 insertions(+), 34 deletions(-)
>
> diff --git a/convert/main.c b/convert/main.c
> index 9904deafba45..416ab5d264a3 100644
> --- a/convert/main.c
> +++ b/convert/main.c
> @@ -942,8 +942,7 @@ static int make_convert_data_block_groups(struct btrfs_trans_handle *trans,
>
> len = min(max_chunk_size,
> cache->start + cache->size - cur);
> - ret = btrfs_alloc_data_chunk(trans, fs_info,
> - &cur_backup, len, 1);
> + ret = btrfs_alloc_data_chunk(trans, fs_info, &cur_backup, len);
> if (ret < 0)
> break;
> ret = btrfs_make_block_group(trans, fs_info, 0,
> diff --git a/volumes.c b/volumes.c
> index 87315a884b49..39e824a43736 100644
> --- a/volumes.c
> +++ b/volumes.c
> @@ -1238,14 +1238,11 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
> /*
> * Alloc a DATA chunk with SINGLE profile.
> *
> - * If 'convert' is set, it will alloc a chunk with 1:1 mapping
> - * (btrfs logical bytenr == on-disk bytenr)
> - * For that case, caller must make sure the chunk and dev_extent are not
> - * occupied.
> + * It allocates a chunk with 1:1 mapping (btrfs logical bytenr == on-disk bytenr)
> + * Caller must make sure the chunk and dev_extent are not occupied.
> */
> int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
> - struct btrfs_fs_info *info, u64 *start,
> - u64 num_bytes, int convert)
> + struct btrfs_fs_info *info, u64 *start, u64 num_bytes)
> {
> u64 dev_offset;
> struct btrfs_root *extent_root = info->extent_root;
> @@ -1264,25 +1261,18 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
> int stripe_len = BTRFS_STRIPE_LEN;
> struct btrfs_key key;
>
> - key.objectid = BTRFS_FIRST_CHUNK_TREE_OBJECTID;
> - key.type = BTRFS_CHUNK_ITEM_KEY;
> - if (convert) {
> - if (*start != round_down(*start, info->sectorsize)) {
> - error("DATA chunk start not sectorsize aligned: %llu",
> - (unsigned long long)*start);
> - return -EINVAL;
> - }
> - key.offset = *start;
> - dev_offset = *start;
> - } else {
> - u64 tmp;
>
> - ret = find_next_chunk(info, &tmp);
> - key.offset = tmp;
> - if (ret)
> - return ret;
> + if (*start != round_down(*start, info->sectorsize)) {
> + error("DATA chunk start not sectorsize aligned: %llu",
> + (unsigned long long)*start);
> + return -EINVAL;
> }
>
> + key.objectid = BTRFS_FIRST_CHUNK_TREE_OBJECTID;
> + key.type = BTRFS_CHUNK_ITEM_KEY;
> + key.offset = *start;
> + dev_offset = *start;
> +
> chunk = kmalloc(btrfs_chunk_item_size(num_stripes), GFP_NOFS);
> if (!chunk)
> return -ENOMEM;
> @@ -1303,12 +1293,8 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
> while (index < num_stripes) {
> struct btrfs_stripe *stripe;
>
> - if (convert)
> - ret = btrfs_insert_dev_extent(trans, device, key.offset,
> - calc_size, dev_offset);
> - else
> - ret = btrfs_alloc_dev_extent(trans, device, key.offset,
> - calc_size, &dev_offset);
> + ret = btrfs_insert_dev_extent(trans, device, key.offset, calc_size,
> + dev_offset);
> BUG_ON(ret);
>
> device->bytes_used += calc_size;
> @@ -1345,8 +1331,6 @@ int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
> ret = btrfs_insert_item(trans, chunk_root, &key, chunk,
> btrfs_chunk_item_size(num_stripes));
> BUG_ON(ret);
> - if (!convert)
> - *start = key.offset;
>
> map->ce.start = key.offset;
> map->ce.size = num_bytes;
> diff --git a/volumes.h b/volumes.h
> index 83ba827e422b..f6f05054b5c4 100644
> --- a/volumes.h
> +++ b/volumes.h
> @@ -271,8 +271,7 @@ int btrfs_alloc_chunk(struct btrfs_trans_handle *trans,
> struct btrfs_fs_info *fs_info, u64 *start,
> u64 *num_bytes, u64 type);
> int btrfs_alloc_data_chunk(struct btrfs_trans_handle *trans,
> - struct btrfs_fs_info *fs_info, u64 *start,
> - u64 num_bytes, int convert);
> + struct btrfs_fs_info *fs_info, u64 *start, u64 num_bytes);
> int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
> int flags);
> int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
>
next prev parent reply other threads:[~2019-10-30 12:45 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-30 12:22 [PATCH 0/3] Misc btrfs-progs fixes Nikolay Borisov
2019-10-30 12:22 ` [PATCH 1/3] btrfs-progs: Initialize sub_stripes to 1 in btrfs_alloc_data_chunk Nikolay Borisov
2019-10-30 12:42 ` Qu Wenruo
2019-10-30 12:22 ` [PATCH 2/3] btrfs-progs: Remove type argument from btrfs_alloc_data_chunk Nikolay Borisov
2019-10-30 12:42 ` Qu Wenruo
2019-10-30 12:22 ` [PATCH 3/3] btrfs-progs: Remove convert param " Nikolay Borisov
2019-10-30 12:45 ` Qu Wenruo [this message]
2019-11-15 12:22 ` [PATCH 0/3] Misc btrfs-progs fixes 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=d90e7757-f646-87f7-a729-8c3652154721@gmx.com \
--to=quwenruo.btrfs@gmx.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=nborisov@suse.com \
/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).