On Thu, Jul 20, 2017 at 4:21 PM, Waiman Long <longman@xxxxxxxxxx> wrote:
> On 07/20/2017 03:20 AM, Miklos Szeredi wrote:
>> On Wed, Jul 19, 2017 at 10:42 PM, Waiman Long <longman@xxxxxxxxxx> wrote:
>>>
>>>>> @@ -603,7 +698,13 @@ static struct dentry *dentry_kill(struct dentry *dentry)
>>>>>
>>>>> if (!IS_ROOT(dentry)) {
>>>>> parent = dentry->d_parent;
>>>>> - if (unlikely(!spin_trylock(&parent->d_lock))) {
>>>>> + /*
>>>>> + * Force the killing of this negative dentry when
>>>>> + * DCACHE_KILL_NEGATIVE flag is set.
>>>>> + */
>>>>> + if (unlikely(dentry->d_flags & DCACHE_KILL_NEGATIVE)) {
>>>>> + spin_lock(&parent->d_lock);
>>>> This looks like d_lock ordering problem (should be parent first, child
>>>> second). Why is this needed, anyway?
>>>>
>>> Yes, that is a bug. I should have used lock_parent() instead.
>> lock_parent() can release dentry->d_lock, which means it's perfectly
>> useless for this.
>
> As the reference count is kept at 1 in dentry_kill(), the dentry won't
> go away even if the dentry lock is temporarily released.
It won't go away, but anything else might happen to it (ref grabbed by
somebody else, instantiated, etc). Don't see how it's going to be
better than the existing trylock.
Thanks,
Miklos
