On Tue, Aug 14, 2012 at 11:32:43AM +0200, Jan Schmidt wrote:

On Wed, August 08, 2012 at 20:55 (+0200), Mark Fasheh wrote:

quoted

+/*
+ * 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.

Doh, yeah I didn't test collisions on this version of the patch (as you can
obviously see, I messed up the merge of Jeff's change).

The rest of this patch looks good to me.

Awesome, I'll send the above as a fix to this patch series (I have at least
one other fix to send soon too)
	--Mark

--
Mark Fasheh