<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type content="text/html; charset=utf-8"><meta name=Generator content="Microsoft Word 12 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
        {font-family:宋体;
        panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
        {font-family:"Cambria Math";
        panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
        {font-family:Tahoma;
        panose-1:2 11 6 4 3 5 4 4 2 4;}
@font-face
        {font-family:"\@宋体";
        panose-1:2 1 6 0 3 1 1 1 1 1;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0cm;
        margin-bottom:.0001pt;
        font-size:12.0pt;
        font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:blue;
        text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
        {mso-style-priority:99;
        color:purple;
        text-decoration:underline;}
p.MsoAcetate, li.MsoAcetate, div.MsoAcetate
        {mso-style-priority:99;
        mso-style-link:"Balloon Text Char";
        margin:0cm;
        margin-bottom:.0001pt;
        font-size:8.0pt;
        font-family:"Tahoma","sans-serif";}
span.hoenzb
        {mso-style-name:hoenzb;}
span.EmailStyle18
        {mso-style-type:personal-reply;
        font-family:"Calibri","sans-serif";
        color:#1F497D;}
span.gwt-inlinelabel
        {mso-style-name:gwt-inlinelabel;}
span.BalloonTextChar
        {mso-style-name:"Balloon Text Char";
        mso-style-priority:99;
        mso-style-link:"Balloon Text";
        font-family:"Tahoma","sans-serif";}
.MsoChpDefault
        {mso-style-type:export-only;
        font-size:10.0pt;}
@page WordSection1
        {size:612.0pt 792.0pt;
        margin:72.0pt 90.0pt 72.0pt 90.0pt;}
div.WordSection1
        {page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--></head><body lang=EN-US link=blue vlink=purple><div class=WordSection1><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Hi Siyao,<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Yes, ll_lookup_lock should be able to be removed. And looks like it will be handled by Yang Sheng in ‘LU-506 kernel: FC15 - Dcache RCU-walk changes’ as commented by Fan Yong (<a href="http://review.whamcloud.com/#patch,sidebyside,1865,2,lustre/include/linux/lustre_patchless_compat.h">http://review.whamcloud.com/#patch,sidebyside,1865,2,lustre/include/linux/lustre_patchless_compat.h</a>). I will leave it as is to let Sheng handle it.<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Thanks,<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'>Tao<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D'><o:p> </o:p></span></p><div style='border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt'><div><div style='border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm'><p class=MsoNormal><b><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'>From:</span></b><span style='font-size:10.0pt;font-family:"Tahoma","sans-serif"'> Lai Siyao <a href="mailto:[mailto:laisiyao@whamcloud.com]">[mailto:laisiyao@whamcloud.com]</a> <br><b>Sent:</b> Friday, January 20, 2012 10:56 AM<br><b>To:</b> Oleg Drokin<br><b>Cc:</b> Andreas Dilger; Peng, Tao; faibish, sorin; Tang, Haiying; Liu, Xuezhao; Fan Yong; Eric Barton<br><b>Subject:</b> Re: Lustre dcache clean up and distributed locking<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><p class=MsoNormal>Hi Tao,<o:p></o:p></p><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>The reason why ll_lookup_lock is introduced is because in some kernels (eg. 2.6.32 for RHEL6), d_rehash_cond() doesn't exist and only d_rehash can be used, which needs unlock dcache_lock before entry, therefore there is a race here, and a similar global lock like dcache_lock is needed. The improvement can be done here is when d_rehash_cond() is exported, ll_lookup_lock lock/unlock can be a null operation, and in recent kernels, dcache_lock is removed, ll_lookup_lock should be able to be removed too.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>I like most of the cleanups, but I don't see much benefits introducing a DENTRY bit lock, because a dentry without LOOKUP lock can hardly be used, and inodebit lock coverage upgrade/downgrade is more useful here.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Bye,<o:p></o:p></p></div><div><p class=MsoNormal style='margin-bottom:12.0pt'>- Lai<o:p></o:p></p><div><p class=MsoNormal>On Fri, Jan 20, 2012 at 1:11 AM, Oleg Drokin <<a href="mailto:green@whamcloud.com">green@whamcloud.com</a>> wrote:<o:p></o:p></p><p class=MsoNormal>Hello!<o:p></o:p></p><div><p class=MsoNormal style='margin-bottom:12.0pt'><br>On Jan 17, 2012, at 3:16 AM, Andreas Dilger wrote:<br><br>> On 2012-01-16, at 9:25 PM, <<a href="mailto:tao.peng@emc.com">tao.peng@emc.com</a>> wrote:<br>>> I finally started to work on Lustre dcache cleanup and locking. After reading Lustre, ocfs2 and VFS dcache related code, I came to a design for cleaning up Lustre dcache code and doing distributed locking under dcache. For distributed locking, the main idea is to add a new inode bitlock DENTRY lock to just protect valid dentry, instead of letting LOOKUP lock handle multiple purpose, which makes client unable to know whether file is deleted or not when server cancels LOOKUP lock. Instead, client holds PR mode DENTRY lock on all valid denties and when server cancels it, client knows the file is deleted.<br>>><br>>> For details, please see the attachments (I attached both word and pdf versions as I am not sure if which is more convenient for you:). And please help to review and comment. Thank you!<o:p></o:p></p></div><p class=MsoNormal style='margin-bottom:12.0pt'>Thanks for looking into this.<br>It is indeed possible to add another bit for just the dentry, but the problem is it would need to be granted in conjunction with<br>the LOOKUP bit anyway (along with permission information), if we don't do this, then we would need to send an additional RPC to get<br>this information very shortly after lookup in a lot of times and that would be pretty slow. Of course there is no way to get different<br>lock mode for every bit, so in this scheme the lock mode swould really be the same.<br>And the (currently) big problem with multiple bit locks is we don't have a way to cancel just subset of a lock (or degrade a lock coverage<br>I should say). This was planned for quite a while ago, but somehow never ended up at the top of the priority list so was never implemented after all.<br>There are minor comments on the logic flow after that too. E.g. the locks in lustre are unlocked right away on the client side, but just placed in the LRU and remain on the client in this sort of unreferenced state. When there is a conflict, the server sends the AST RPC and then the lock<br>is actually released (cancelled) by client. Currently the AST is not delivered to a lock that has non-zero usage counters.<br>Additionally due to the way we implement locks, it's not possible to have a separate lock for every name of the file.<br><br>For your cleanup list, I think that is good. Note that item 'c' was already implemented some time ago, you can see it here:<br><a href="https://bugzilla.lustre.org/show_bug.cgi?id=16417" target="_blank">https://bugzilla.lustre.org/show_bug.cgi?id=16417</a> the patch is this: <a href="https://bugzilla.lustre.org/attachment.cgi?id=24200" target="_blank">https://bugzilla.lustre.org/attachment.cgi?id=24200</a><br>It's probably somewhat stale now. It did not work correctly back at the time due to some other logic, but should work now.<br>You can read the comments in that bug for some more info.<br><br>Bye,<br>   Oleg<br><span class=hoenzb><span style='color:#888888'>--</span></span><span style='color:#888888'><br><span class=hoenzb>Oleg Drokin</span><br><span class=hoenzb>Senior Software Engineer</span><br><span class=hoenzb>Whamcloud, Inc.</span></span><o:p></o:p></p></div><p class=MsoNormal><o:p> </o:p></p></div></div></div></body></html>