Re: [PATCH v5 12/18] btrfs: inline owner ref lookup helper

David Sterba
2023-09-07T15:35:30Z

On Thu, Jul 27, 2023 at 03:12:59PM -0700, Boris Burkov wrote:
> Inline ref parsing is a bit tricky and relies on a decent amount of
> implicit information, so I think it is beneficial to have a helper
> function for reading the owner ref, if only to "document" the format,
> along with the write path.
>
> The main subtlety of note which I was missing by open-coding this was
> that it is important to check whether or not inline refs are present
> *at all*. i.e., if we are writing out a new extent under squotas, we
> will always use a big enough item for the inline ref and have it.
> However, it is possible that some random item predating squotas will not
> have any inline refs. In that case, trying to read the "type" field of
> the first inline ref will just be reading garbage in the form of
> whatever is in the next item.
>
> This will be used by the extent free-ing path, which looks up data
> extent owners as well as a relocation path which needs to grab the owner
> before relocating an extent.
>
> Signed-off-by: Boris Burkov <boris@bur.io>
> Reviewed-by: Josef Bacik <josef@toxicpanda.com>
> ---
> fs/btrfs/extent-tree.c | 51 ++++++++++++++++++++++++++++++++++++++++++
> fs/btrfs/extent-tree.h | 3 +++
> 2 files changed, 54 insertions(+)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index c6d537bf5ad4..09fb321fa560 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2805,6 +2805,57 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans)
> return 0;
> }
>
> +/*
> + * Helper to parse an extent item's inline extents looking for a simple
> + * quotas owner ref.

"Parse an extent item's ..."

> + *
> + * @fs_info - the btrfs_fs_info for this mount

The argument description format is " @fs_info:"

> + * @leaf - a leaf in the extent tree containing the extent item
> + * @slot - the slot in the leaf where the extent item is found
> + *
> + * Returns the objectid of the root that originally allocated the extent item
> + * if the inline owner ref is expected and present, otherwise 0.
> + *
> + * If an extent item has an owner ref item, it will be the first
> + * inline ref item. Therefore the logic is to check whether there are
> + * any inline ref items, then check the type of the first one.

Please format comments so they use the 80 columns with acceptable
overflows 3-4 chars if it makes it look nicer.

> + *
> + */
> +u64 btrfs_get_extent_owner_root(struct btrfs_fs_info *fs_info,
> + struct extent_buffer *leaf,
> + int slot)
> +{
> + struct btrfs_extent_item *ei;
> + struct btrfs_extent_inline_ref *iref;
> + struct btrfs_extent_owner_ref *oref;
> + unsigned long ptr;
> + unsigned long end;
> + int type;
> +
> + if (!btrfs_fs_incompat(fs_info, SIMPLE_QUOTA))
> + return 0;
> +
> + ei = btrfs_item_ptr(leaf, slot, struct btrfs_extent_item);
> + ptr = (unsigned long)(ei + 1);
> + end = (unsigned long)ei + btrfs_item_size(leaf, slot);
> +
> + /* No inline ref items of any kind, can't check type */
> + if (ptr == end)
> + return 0;
> +
> + iref = (struct btrfs_extent_inline_ref *)ptr;
> + type = btrfs_get_extent_inline_ref_type(leaf, iref, BTRFS_REF_TYPE_ANY);
> +
> + /* We found an owner ref, get the root out of it */
> + if (type == BTRFS_EXTENT_OWNER_REF_KEY) {
> + oref = (struct btrfs_extent_owner_ref *)(&iref->offset);
> + return btrfs_extent_owner_ref_root_id(leaf, oref);
> + }
> +
> + /* We have inline refs, but not an owner ref */
> + return 0;
> +}
> +
> static int do_free_extent_accounting(struct btrfs_trans_handle *trans,
> u64 bytenr, u64 num_bytes, bool is_data)
> {
> diff --git a/fs/btrfs/extent-tree.h b/fs/btrfs/extent-tree.h
> index b9e148adcd28..7c27652880a2 100644
> --- a/fs/btrfs/extent-tree.h
> +++ b/fs/btrfs/extent-tree.h
> @@ -141,6 +141,9 @@ int btrfs_set_disk_extent_flags(struct btrfs_trans_handle *trans,
> struct extent_buffer *eb, u64 flags);
> int btrfs_free_extent(struct btrfs_trans_handle *trans, struct btrfs_ref *ref);
>
> +u64 btrfs_get_extent_owner_root(struct btrfs_fs_info *fs_info,
> + struct extent_buffer *leaf,
> + int slot);
> int btrfs_free_reserved_extent(struct btrfs_fs_info *fs_info,
> u64 start, u64 len, int delalloc);
> int btrfs_pin_reserved_extent(struct btrfs_trans_handle *trans, u64 start, u64 len);
> --
> 2.41.0