On 03/02/2012 11:35 AM, Sage Weil wrote:
On Tue, 28 Feb 2012, Alex Elder wrote:

This patch just rearranges a few bits of code to make more
portions of ceph_setxattr() and ceph_removexattr() identical.

Signed-off-by: Alex Elder<[email protected]>
---

. . .

@@ -803,6 +803,7 @@ int ceph_setxattr(struct dentry *dentry, const char *name,
        spin_lock(&ci->i_ceph_lock);
  retry:

This:

        issued = __ceph_caps_issued(ci, NULL);
+       dout("setxattr %p issued %s\n", inode, ceph_cap_string(issued));
        if (!(issued&  CEPH_CAP_XATTR_EXCL))
                goto do_sync;

needs to go after this:

        __build_xattrs(inode);

All of the __build_xattrs() calls should go before the cap check, because
they drop i_ceph_lock.

Can probably do that in a new patch, since it was broken for some of these
before.

I've created this to (try to) document your observation:
    http://tracker.newdream.net/issues/2129

I am committing this patch as-is for now.

                                        -Alex

Otherwise, this series looks good!

Reviewed-by: Sage Weil<[email protected]>

@@ -811,7 +812,7 @@ retry:

        if (!ci->i_xattrs.prealloc_blob ||
            required_blob_size>  ci->i_xattrs.prealloc_blob->alloc_len) {
-               struct ceph_buffer *blob = NULL;
+               struct ceph_buffer *blob;

                spin_unlock(&ci->i_ceph_lock);
                dout(" preaallocating new blob size=%d\n",
required_blob_size);
@@ -825,12 +826,13 @@ retry:
                goto retry;
        }

-       dout("setxattr %p issued %s\n", inode, ceph_cap_string(issued));
        err = __set_xattr(ci, newname, name_len, newval,
                          val_len, 1, 1, 1,&xattr);
+
        dirty = __ceph_mark_dirty_caps(ci, CEPH_CAP_XATTR_EXCL);
        ci->i_xattrs.dirty = true;
        inode->i_ctime = CURRENT_TIME;
+
        spin_unlock(&ci->i_ceph_lock);
        if (dirty)
                __mark_inode_dirty(inode, dirty);
@@ -892,18 +894,17 @@ int ceph_removexattr(struct dentry *dentry, const char
*name)
                return -EOPNOTSUPP;

        spin_lock(&ci->i_ceph_lock);
-       __build_xattrs(inode);
        issued = __ceph_caps_issued(ci, NULL);
        dout("removexattr %p issued %s\n", inode, ceph_cap_string(issued));
-
        if (!(issued&  CEPH_CAP_XATTR_EXCL))
                goto do_sync;
+       __build_xattrs(inode);

        err = __remove_xattr_by_name(ceph_inode(inode), name);
+
        dirty = __ceph_mark_dirty_caps(ci, CEPH_CAP_XATTR_EXCL);
        ci->i_xattrs.dirty = true;
        inode->i_ctime = CURRENT_TIME;
-
        spin_unlock(&ci->i_ceph_lock);
        if (dirty)
                __mark_inode_dirty(inode, dirty);
--
1.7.5.4


--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to