From: Alex Elder <elder@dreamhost.com>
To: Sage Weil <sage@newdream.net>
Cc: ceph-devel@vger.kernel.org
Subject: Re: [PATCH 6/6] ceph: make ceph_setxattr() and ceph_removexattr() more alike
Date: Fri, 02 Mar 2012 20:17:56 -0800 [thread overview]
Message-ID: <4F519B74.9020708@dreamhost.com> (raw)
In-Reply-To: <Pine.LNX.4.64.1203021134230.6413@cobra.newdream.net>
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<elder@dreamhost.com>
>> ---
. . .
>> @@ -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<sage@newdream.net>
>
>> @@ -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 majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
next prev parent reply other threads:[~2012-03-03 4:17 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-29 3:17 [PATCH 0/6] ceph: virtual extended attribute cleanup Alex Elder
2012-02-29 3:21 ` [PATCH 1/6] ceph: use a symbolic name for "ceph." extended attribute namespace Alex Elder
2012-02-29 3:21 ` [PATCH 2/6] ceph: use macros to normalize vxattr table definitions Alex Elder
2012-02-29 3:21 ` [PATCH 3/6] ceph: drop "_cb" from name of struct ceph_vxattr_cb Alex Elder
2012-02-29 3:21 ` [PATCH 4/6] ceph: encode type in vxattr callback routines Alex Elder
2012-02-29 3:21 ` [PATCH 5/6] ceph: avoid repeatedly computing the size of constant vxattr names Alex Elder
2012-02-29 4:47 ` Yehuda Sadeh Weinraub
2012-02-29 5:19 ` Alex Elder
2012-02-29 3:21 ` [PATCH 6/6] ceph: make ceph_setxattr() and ceph_removexattr() more alike Alex Elder
2012-03-02 19:35 ` Sage Weil
2012-03-03 4:17 ` Alex Elder [this message]
2012-02-29 4:20 ` [PATCH 0/6] ceph: virtual extended attribute cleanup Christoph Hellwig
2012-02-29 5:15 ` Alex Elder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4F519B74.9020708@dreamhost.com \
--to=elder@dreamhost.com \
--cc=ceph-devel@vger.kernel.org \
--cc=sage@newdream.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.