Re: [PATCH v3 3/4] btrfs: remove free space items when creating free space tree

David Sterba
2020-09-24T17:09:10Z

On Thu, Sep 17, 2020 at 11:13:40AM -0700, Boris Burkov wrote:
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -207,6 +207,64 @@ int create_free_space_inode(struct btrfs_trans_handle *trans,
> ino, block_group->start);
> }
>
> +/*
> + * inode is an optional sink: if it is NULL, btrfs_remove_free_space_inode
> + * handles lookup, otherwise it takes ownership and iputs the inode.
> + * Don't reuse an inode pointer after passing it into this function.
> + */
> +int btrfs_remove_free_space_inode(struct btrfs_trans_handle *trans,
> + struct inode *inode,
> + struct btrfs_block_group *block_group)
> +{
> + struct btrfs_path *path;
> + struct btrfs_key key;
> + int ret = 0;
> + path = btrfs_alloc_path();
> + if (!path)
> + return -ENOMEM;
> +
> + if (!inode) {
> + inode = lookup_free_space_inode(block_group, path);
> + }

No { } around single statement

> + if (IS_ERR(inode)) {
> + if (PTR_ERR(inode) != -ENOENT)
> + ret = PTR_ERR(inode);
> + goto out;
> + }
> + ret = btrfs_orphan_add(trans, BTRFS_I(inode));
> + if (ret) {
> + btrfs_add_delayed_iput(inode);
> + goto out;
> + }
> + clear_nlink(inode);
> + /* One for the block groups ref */
> + spin_lock(&block_group->lock);
> + if (block_group->iref) {
> + block_group->iref = 0;
> + block_group->inode = NULL;
> + spin_unlock(&block_group->lock);
> + iput(inode);
> + } else {
> + spin_unlock(&block_group->lock);
> + }
> + /* One for the lookup ref */
> + btrfs_add_delayed_iput(inode);
> +
> + key.objectid = BTRFS_FREE_SPACE_OBJECTID;
> + key.type = 0;
> + key.offset = block_group->start;
> + ret = btrfs_search_slot(trans, trans->fs_info->tree_root, &key, path, -1, 1);

This slightly overflows 80 but it's full one parameter so in this case
I'd rather wrap it (from -1). Otherwise, more than 80 is ok-ish if it's
the ); or end of an identifier that's half visible.

> + if (ret) {
> + if (ret > 0)
> + ret = 0;
> + goto out;
> + }
> + ret = btrfs_del_item(trans, trans->fs_info->tree_root, path);
> +out:
> + btrfs_free_path(path);
> + return ret;
> +}
> +
> int btrfs_check_trunc_cache_free_space(struct btrfs_fs_info *fs_info,
> struct btrfs_block_rsv *rsv)
> {