On 2.03.20 г. 11:45 ч., Qu Wenruo wrote:
> Due to the complex nature of btrfs extent tree, when we want to iterate
> all backrefs of one extent, it involves quite a lot of work, like
> searching the EXTENT_ITEM/METADATA_ITEM, iteration through inline and keyed
> backrefs.
>
> Normally this would result pretty complex code, something like:
> btrfs_search_slot()
> /* Ensure we are at EXTENT_ITEM/METADATA_ITEM */
> while (1) { /* Loop for extent tree items */
> while (ptr < end) { /* Loop for inlined items */
> /* REAL WORK HERE */
> }
> next:
> ret = btrfs_next_item()
> /* Ensure we're still at keyed item for specified bytenr */
> }
>
> The idea of btrfs_backref_iter is to avoid such complex and hard to
> read code structure, but something like the following:
>
> iter = btrfs_backref_iter_alloc();
> ret = btrfs_backref_iter_start(iter, bytenr);
> if (ret < 0)
> goto out;
> for (; ; ret = btrfs_backref_iter_next(iter)) {
> /* REAL WORK HERE */
> }
> out:
> btrfs_backref_iter_free(iter);
>
> This patch is just the skeleton + btrfs_backref_iter_start() code.
>
> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
> ---
> fs/btrfs/backref.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++
> fs/btrfs/backref.h | 60 ++++++++++++++++++++++++++++++++
> 2 files changed, 147 insertions(+)
>
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index 327e4480957b..444cd5d31d87 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -2299,3 +2299,90 @@ void free_ipath(struct inode_fs_paths *ipath)
> kvfree(ipath->fspath);
> kfree(ipath);
> }
> +
> +int btrfs_backref_iter_start(struct btrfs_backref_iter *iter, u64 bytenr)
> +{
> + struct btrfs_fs_info *fs_info = iter->fs_info;
> + struct btrfs_path *path = iter->path;
> + struct btrfs_extent_item *ei;
> + struct btrfs_key key;
> + int ret;
> +
> + key.objectid = bytenr;
> + key.type = BTRFS_METADATA_ITEM_KEY;
> + key.offset = (u64)-1;
> + iter->bytenr = bytenr;
> +
> + ret = btrfs_search_slot(NULL, fs_info->extent_root, &key, path, 0, 0);
> + if (ret < 0)
> + return ret;
> + if (ret == 0) {
> + ret = -EUCLEAN;
> + goto release;
> + }
> + if (path->slots[0] == 0) {
> + WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG));
> + ret = -EUCLEAN;
> + goto release;
> + }
> + path->slots[0]--;
> +
> + btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
> + if (!(key.type == BTRFS_EXTENT_ITEM_KEY ||
> + key.type == BTRFS_METADATA_ITEM_KEY) || key.objectid != bytenr) {
> + ret = -ENOENT;
> + goto release;
> + }
nit: Please keep the logic of the checks identical as it makes
comprehending the code easier. What I mean is here you do
!(type == COND1 || type == COND2) which really translates to
type != COND && type != COND2. However, when checking for keyed items
you use the second style. Yes, they are both identical in terms of
semantics but one might wonder "why is this done in 2 different ways for
no apparent reason" ?
> + memcpy(&iter->cur_key, &key, sizeof(key));
> + iter->item_ptr = btrfs_item_ptr_offset(path->nodes[0],
> + path->slots[0]);
> + iter->end_ptr = iter->item_ptr + btrfs_item_size_nr(path->nodes[0],
> + path->slots[0]);
> + ei = btrfs_item_ptr(path->nodes[0], path->slots[0],
> + struct btrfs_extent_item);
> +
> + /*
> + * Only support iteration on tree backref yet.
> + *
> + * This is extra precaustion for non skinny-metadata, where
> + * EXTENT_ITEM is also used for tree blocks, that we can only use
> + * extent flags to determine if it's a tree block.
> + */
> + if (btrfs_extent_flags(path->nodes[0], ei) & BTRFS_EXTENT_FLAG_DATA) {
> + ret = -ENOTTY;
why not simply -ENOSUPP
> + goto release;
> + }
<snip>
