Re: [PATCH 1/3] btrfs: extended inode refs

[Jan Schmidt <list.btrfs@jan-o-sch.net>]

On Wed, August 08, 2012 at 20:55 (+0200), Mark Fasheh wrote:
> +/*
> + * btrfs_insert_inode_extref() - Inserts an extended inode ref into a tree.
> + *
> + * The caller must have checked against BTRFS_LINK_MAX already.
> + */
> +static int btrfs_insert_inode_extref(struct btrfs_trans_handle *trans,
> +				     struct btrfs_root *root,
> +				     const char *name, int name_len,
> +				     u64 inode_objectid, u64 ref_objectid, u64 index)
> +{
> +	struct btrfs_inode_extref *extref;
> +	int ret;
> +	int ins_len = name_len + sizeof(*extref);
> +	unsigned long ptr;
> +	struct btrfs_path *path;
> +	struct btrfs_key key;
> +	struct extent_buffer *leaf;
> +	struct btrfs_item *item;
> +
> +	key.objectid = inode_objectid;
> +	key.type = BTRFS_INODE_EXTREF_KEY;
> +	key.offset = btrfs_extref_hash(ref_objectid, name, name_len);
> +
> +	path = btrfs_alloc_path();
> +	if (!path)
> +		return -ENOMEM;
> +
> +	path->leave_spinning = 1;
> +	ret = btrfs_insert_empty_item(trans, root, path, &key,
> +				      ins_len);
> +	if (ret == -EEXIST) {
> +		if (btrfs_find_name_in_ext_backref(path, name, name_len, NULL))
> +			goto out;
> +
> +		btrfs_extend_item(trans, root, path, ins_len);
> +	}
> +	if (ret < 0)
> +		goto out;

This doesn't look right. Did you actually test it? I haven't, but I claim that
with this version of the patch, you won't be able to add a hash collision link.

Jeff changed btrfs_extend_item from int to void in March, so we're no longer
setting ret there. I suggest adding "ret = 0" within the EEXIST-block.

The rest of this patch looks good to me.

-Jan