Mark Fasheh wrote:
The thing is, the locking as you have it here isn't protecting against
racing a data write, which is reading l_count on the extent list (or id_count on
inline data) and an xattr write which might want to shrink those. You'll
need at least ip_alloc_sem around those, since ocfs2_page_mkwrite() doesn't
take i_mutex because it doesn't want to deadlock against the mmap
semaphore.
Thanks, You point out a potential bug in my patch. I didn't protect
reading/writing xattr against file data.

My patch check l_count/id_count in ocfs2_xattr_has_space_inline() and
may change it in ocfs2_xattr_ibody_set().
is patch attached fix this problem?


Or are you trying to protect xattr against itself? If that's the case, you
could push this lock up to the top and take it around entire operations.
Actually I am trying to protect xattr read/write by this semaphore,
since we found a bug about it.
http://oss.oracle.com/bugzilla/show_bug.cgi?id=990

So I need change comment about xattr semaphore.
/* protects extended attribute change on this inode */

Best regards,
tiger

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 0b34db1..93e46ee 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -1596,12 +1596,16 @@ static int ocfs2_xattr_ibody_find(struct inode *inode,
 	struct ocfs2_inode_info *oi = OCFS2_I(inode);
 	struct ocfs2_dinode *di = (struct ocfs2_dinode *)xs->inode_bh->b_data;
 	int ret;
+	int has_space = 0;
 
 	if (inode->i_sb->s_blocksize == OCFS2_MIN_BLOCKSIZE)
 		return 0;
 
 	if (!(oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL)) {
-		if (!ocfs2_xattr_has_space_inline(inode, di))
+		down_read(&oi->ip_alloc_sem);
+		has_space = ocfs2_xattr_has_space_inline(inode, di);
+		up_read(&oi->ip_alloc_sem);
+		if (!has_space)
 			return 0;
 	}
 
@@ -1644,13 +1648,18 @@ static int ocfs2_xattr_ibody_set(struct inode *inode,
 	if (inode->i_sb->s_blocksize == OCFS2_MIN_BLOCKSIZE)
 		return -ENOSPC;
 
+	down_write(&oi->ip_alloc_sem);
 	if (!(oi->ip_dyn_features & OCFS2_INLINE_XATTR_FL)) {
-		if (!ocfs2_xattr_has_space_inline(inode, di))
-			return -ENOSPC;
+		if (!ocfs2_xattr_has_space_inline(inode, di)) {
+			ret = -ENOSPC;
+			goto out;
+		}
 	}
 
 	ret = ocfs2_xattr_set_entry(inode, xi, xs,
 				(OCFS2_INLINE_XATTR_FL | OCFS2_HAS_XATTR_FL));
+out:
+	up_write(&oi->ip_alloc_sem);
 
 	return ret;
 }
_______________________________________________
Ocfs2-devel mailing list
[email protected]
http://oss.oracle.com/mailman/listinfo/ocfs2-devel

Reply via email to