<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">Hi Christoph!<br>
<br>
Thanks for your attention.<br>
<br>
On 10/28/2016 02:20 PM, Christoph Hellwig wrote:<br>
</div>
<blockquote cite="mid:20161028062056.GA9994@lst.de" type="cite">
<pre wrap="">Hi Eric,
I've added linux-fsdevel to the cc list as this should get a bit
broader attention.
On Wed, Oct 19, 2016 at 01:19:40PM +0800, Eric Ren wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Mostly, we can avoid recursive locking by writing code carefully. However, as
the deadlock issues have proved out, it's very hard to handle the routines
that are called directly by vfs. For instance:
const struct inode_operations ocfs2_file_iops = {
.permission = ocfs2_permission,
.get_acl = ocfs2_iop_get_acl,
.set_acl = ocfs2_iop_set_acl,
};
ocfs2_permission() and ocfs2_iop_get/set_acl() both call ocfs2_inode_lock().
The problem is that the call chain of ocfs2_permission() includes *_acl().
</pre>
</blockquote>
<pre wrap="">
What do you actually protect in ocfs2_permission? It's a trivial
wrapper around generic_permission which just looks at the VFS inode.</pre>
</blockquote>
Yes, it is.<br>
<br>
<a class="moz-txt-link-freetext" href="https://github.com/torvalds/linux/blob/master/fs/ocfs2/file.c#L1321">https://github.com/torvalds/linux/blob/master/fs/ocfs2/file.c#L1321</a><br>
---<br>
<span class="pl-en">ocfs2_permission</span><span class="pl-en"></span><br>
<span class="pl-c1"> ocfs2_inode_lock()</span><br>
<span class="pl-c1"></span><span class="pl-c1">
generic_permission</span><br>
<span class="pl-c1"></span><span class="pl-c1">
ocfs2_inode_unlock</span>
<blockquote cite="mid:20161028062056.GA9994@lst.de" type="cite">
<pre wrap="">
I think the right fix is to remove ocfs2_permission entirely and use
the default VFS implementation. That both solves your locking problem,
and it will also get you RCU lookup instead of dropping out of
RCU mode all the time.</pre>
</blockquote>
But, from my understanding, the pair of ocfs2_inode_lock/unlock() is
used to prevent any concurrent changes<br>
to the permission of the inode on the other cluster node while we
are checking on it. It's a common case for cluster<br>
filesystem, such as GFS2:
<a class="moz-txt-link-freetext" href="https://github.com/torvalds/linux/blob/master/fs/gfs2/inode.c#L1777">https://github.com/torvalds/linux/blob/master/fs/gfs2/inode.c#L1777</a><br>
<br>
Thanks for your suggestion again!<br>
Eric <br>
<blockquote cite="mid:20161028062056.GA9994@lst.de" type="cite">
<pre wrap="">
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to <a class="moz-txt-link-abbreviated" href="mailto:majordomo@vger.kernel.org">majordomo@vger.kernel.org</a>
More majordomo info at <a class="moz-txt-link-freetext" href="http://vger.kernel.org/majordomo-info.html">http://vger.kernel.org/majordomo-info.html</a>
</pre>
</blockquote>
<p><br>
</p>
</body>
</html>