lkml.org 
[lkml]   [2018]   [Mar]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: dcache: remove trylock loops (was Re: [BUG] lock_parent() breakage when used from shrink_dentry_list())
Date
On 2018-03-12, Al Viro <viro@ZenIV.linux.org.uk> wrote:
>> If someone else has grabbed a reference, it shouldn't be added to the
>> lru list. Only decremented.
>>
>> if (entry->d_lockref.count == 1)
>
> Nah, better handle that in retain_dentry() itself. See updated
> #work.dcache.
>
> + if (unlikely(dentry->d_lockref.count != 1)) {
> + dentry->d_lockref.count--;
> + } else if (likely(!retain_dentry(dentry))) {
> + __dentry_kill(dentry);
> + return parent;
> + }

Although the updated version is correct (and saves on lines of code), I
find putting the deref and lru_add code in the "true" case of
retain_dentry() to be pretty tricky. IMHO the code is easier to
understand if it looks like this:

if (unlikely(dentry->d_lockref.count != 1)) {
dentry->d_lockref.count--;
} else if (likely(!retain_dentry(dentry))) {
__dentry_kill(dentry);
return parent;
} else {
dentry->d_lockref.count--;
dentry_lru_add(dentry);
}

This is what your version is doing, but that final else is hiding in the
retain_dentry() "true" case.

My suggestion is to revert 7479f57fecd2a4837b5c79ce1cf0dcf284db54be (and
then fixup dput() to deref before calling dentry_lru_add()).

> FWIW, there's another trylock loop on dentries - one in
> autofs get_next_positive_dentry(). Any plans re dealing
> with that one?

I will need to dig into it a bit deeper (I am unfamiliar with autofs),
but it looks like it is trying to do basically the same thing as the
ascend loop in d_walk().

> I'd spent the last couple of weeks (when not being too sick
> for any work) going through dcache.c and related code; hopefully
> this time I will get the documentation into postable shape ;-/

Thank you for all your help in getting these changes cleaned and
correctly implemented so quickly.

I've reviewed your latest trylock loop removal patches and found only 1
minor issue. I'll post that separately.

John Ogness

\
 
 \ /
  Last update: 2018-03-13 21:47    [W:0.113 / U:0.544 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site