Fix the nested PR lock calling issue

Hi,

This is an attempt to fix the bug
https://bugzilla.novell.com/show_bug.cgi?id=614332
(also http://oss.oracle.com/bugzilla/show_bug.cgi?id=1278)

The full log is available at comment #54 of bug 614332 at Novell bugzilla. Simply speaking, it should be nested PR lock calling issue. The calling sequence should be:

node1 node2

            gr PR
              |
              V
PR(EX) -> BAST: OCFS2_LOCK_BLOCKED
              |
              V
            rq PR
              |
              V
            wait = 1

Since we have already had the granted PR lock from the beginning, we may not need to request the PR lock again.

Patch attached below. Review and comments are highly appreciated;)

Thanks,
Jiaju

Signed-off-by Jiaju Zhang <[email protected]>

Cc Joel Becker <[email protected]>

Cc Mark Fasheh <[email protected]>

Cc Sunil Mushran <[email protected]>

Cc Tiger Yang <[email protected]>

---
 fs/ocfs2/acl.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

fs/ocfs2/acl.c +1 -3

index da70229..0354af1 100644
@@ -152,7 +152,7 @@ static struct posix_acl *ocfs2_get_acl(struct inode *inode, int type)
 	if (!(osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL))
 		return NULL;
 
-	ret = ocfs2_inode_lock(inode, &di_bh, 0);
+	ret = ocfs2_read_inode_block(inode, &di_bh);
 	if (ret < 0) {
 		mlog_errno(ret);
 		acl = ERR_PTR(ret);

@@ -161,8 +161,6 @@ static struct posix_acl *ocfs2_get_acl(struct inode *inode, int type)
 
 	acl = ocfs2_get_acl_nolock(inode, type, di_bh);
 
-	ocfs2_inode_unlock(inode, 0);
-
 	brelse(di_bh);
 
 	return acl;

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel

Hi Jiaju,

Quoted 23 lines · Jiaju Zhang

Hi,

This is an attempt to fix the bug
https://bugzilla.novell.com/show_bug.cgi?id=614332
(also http://oss.oracle.com/bugzilla/show_bug.cgi?id=1278)

The full log is available at comment #54 of bug 614332 at Novell bugzilla. Simply speaking, it should be nested PR lock calling issue. The calling sequence should be:

node1 ? ? ? node2

? ? ? ? ? ? gr PR
? ? ? ? ? ? ? |
? ? ? ? ? ? ? V
PR(EX) -> BAST: OCFS2_LOCK_BLOCKED
? ? ? ? ? ? ? |
? ? ? ? ? ? ? V
? ? ? ? ? ? rq PR
? ? ? ? ? ? ? |
? ? ? ? ? ? ? V
? ? ? ? ? ? wait = 1

Quoted 2 lines

Since we have already had the granted PR lock from the beginning, we may not need to request the PR lock again.

What's the real calling path?
I guess you meant
ocfs2_permission()(lock gothere)
?->ocfs2_check_acl()
?->ocfs2_get_acl()(require the lock again)

But there is other ocfs2_get_acl() calling paths in which the lock is not taken before calling ocfs2_get_acl(). such as ocfs2_setattr()(the lock is taken and released before calling ocfs2_acl_chmod()) ?->ocfs2_acl_chmod()
?->ocfs2_get_acl().

So I think the patch is not safe.