<html><head><meta http-equiv="Content-Type" content="text/html charset=gb2312"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi Eric,<div class=""><br class=""><div><blockquote type="cite" class=""><div class="">在 2016年10月19日，下午1:19，Eric Ren <<a href="mailto:zren@suse.com" class="">zren@suse.com</a>> 写道：</div><br class="Apple-interchange-newline"><div class="">Hi all!<br class=""><br class="">Commit 743b5f1434f5 ("ocfs2: take inode lock in ocfs2_iop_set/get_acl()")<br class="">results in another deadlock as we have discussed in the recent thread:<br class=""> <a href="https://oss.oracle.com/pipermail/ocfs2-devel/2016-October/012454.html" class="">https://oss.oracle.com/pipermail/ocfs2-devel/2016-October/012454.html</a><br class=""><br class="">Before this one, a similiar deadlock has been fixed by Junxiao:<br class=""> commit c25a1e0671fb ("ocfs2: fix posix_acl_create deadlock")<br class=""> commit 5ee0fbd50fdf ("ocfs2: revert using ocfs2_acl_chmod to avoid inode cluster lock hang")<br class=""><br class="">We are in the situation that we have to avoid recursive cluster locking, but<br class="">there is no way to check if a cluster lock has been taken by a precess already.<br class=""><br class="">Mostly, we can avoid recursive locking by writing code carefully. However, as<br class="">the deadlock issues have proved out, it's very hard to handle the routines<br class="">that are called directly by vfs. For instance:<br class=""><br class=""> const struct inode_operations ocfs2_file_iops = {<br class=""> .permission = ocfs2_permission,<br class=""> .get_acl = ocfs2_iop_get_acl,<br class=""> .set_acl = ocfs2_iop_set_acl,<br class=""> };<br class=""><br class=""><br class="">ocfs2_permission() and ocfs2_iop_get/set_acl() both call ocfs2_inode_lock().<br class="">The problem is that the call chain of ocfs2_permission() includes *_acl().<br class=""><br class="">Possibly, there are three solutions I can think of. The first one is to<br class="">implement the inode permission routine for ocfs2 itself, replacing the<br class="">existing generic_permission(); this will bring lots of changes and<br class="">involve too many trivial vfs functions into ocfs2 code. Frown on this.<br class=""><br class="">The second one is, what I am trying now, to keep track of the processes who<br class="">lock/unlock a cluster lock by the following draft patches. But, I quickly<br class="">find out that a cluster locking which has been taken by processA can be unlocked<br class="">by processB. For example, systemfiles like journal:0000 is locked during mout, and<br class="">unlocked during umount. <br class=""></div></blockquote>I had ever implemented generic recursive locking support, please check the patch at <a href="https://oss.oracle.com/pipermail/ocfs2-devel/2015-December/011408.html" class="">https://oss.oracle.com/pipermail/ocfs2-devel/2015-December/011408.html</a> , the issue that locking and unlocking in different processes was considered. But it was rejected by Mark as recursive locking is not allowed in ocfs2/kernel . <br class=""><blockquote type="cite" class=""><div class=""><br class="">The thrid one is to revert that problematic commit! It looks like get/set_acl()<br class="">are always been called by other vfs callback like ocfs2_permission(). I think<br class="">we can do this if it's true, right? Anyway, I'll try to work out if it's true;-)<br class=""></div></blockquote>Not sure whether get/set_acl() will be called directly by vfs. Even not now, we can’t make sure that in the future. So revert it may be a little risky. But if refactor is complicated, then this maybe the only way we can do.</div><div><br class=""></div><div>Thanks,</div><div>Junxiao.<br class=""><blockquote type="cite" class=""><div class=""><br class="">Hope for your input to solve this problem;-)<br class=""><br class="">Thanks,<br class="">Eric<br class=""><br class=""></div></blockquote></div><br class=""></div></body></html>