Re: [PATCH 1/4] fs/dcache: Limit numbers of negative dentries

From	Miklos Szeredi <mszeredi@redhat.com>
Newsgroups	linux.kernel
Subject	Re: [PATCH 1/4] fs/dcache: Limit numbers of negative dentries
Date	2017-07-20 17:10 +0200
Message-ID	<u5lfr-JN-11@gated-at.bofh.it> (permalink)
References	(1 earlier) <u4ezo-5Kj-23@gated-at.bofh.it> <u53Lz-5fP-9@gated-at.bofh.it> <u544W-5nE-21@gated-at.bofh.it> <u5e4h-41p-7@gated-at.bofh.it> <u5kCJ-ew-1@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Thu, Jul 20, 2017 at 4:21 PM, Waiman Long <longman@redhat.com> wrote:
> On 07/20/2017 03:20 AM, Miklos Szeredi wrote:
>> On Wed, Jul 19, 2017 at 10:42 PM, Waiman Long <longman@redhat.com> 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

Thread

Re: [PATCH 1/4] fs/dcache: Limit numbers of negative dentries Miklos Szeredi <mszeredi@redhat.com> - 2017-07-19 22:30 +0200
  Re: [PATCH 1/4] fs/dcache: Limit numbers of negative dentries Waiman Long <longman@redhat.com> - 2017-07-19 22:50 +0200
    Re: [PATCH 1/4] fs/dcache: Limit numbers of negative dentries Miklos Szeredi <mszeredi@redhat.com> - 2017-07-20 09:30 +0200
      Re: [PATCH 1/4] fs/dcache: Limit numbers of negative dentries Waiman Long <longman@redhat.com> - 2017-07-20 16:30 +0200
        Re: [PATCH 1/4] fs/dcache: Limit numbers of negative dentries Miklos Szeredi <mszeredi@redhat.com> - 2017-07-20 17:10 +0200
          Re: [PATCH 1/4] fs/dcache: Limit numbers of negative dentries Waiman Long <longman@redhat.com> - 2017-07-20 17:50 +0200

csiph-web