Re: [PATCH v3 0/5] fs/dcache: Limit # of negative dentries

[Waiman Long <longman@redhat.com>]

On 08/17/2017 12:00 AM, Wangkai (Kevin,C) wrote:
>
>>>
>>> Hi Longman,
>>> I am a fresher of fsdevel, about 2 weeks before, I have joined this
>>> mail list, recently I have met the same problem of negative dentries,
>>> in my opinion, the dentries should be remove together with the files or
>> directories, I don't know you have submit this patch, I have another patch
>> about this:
>>> http://marc.info/?l=linux-fsdevel&m=150209902215266&w=2
>>>
>>> maybe this is a foo idea...
>>>
>>> regards
>>> Kevin
>> If you look at the code, the front dentries of the LRU list are removed when
>> there are too many negative dentries. That includes positive dentries as well as
>> it is not practical to just remove the negative dentries.
>>
>> I have looked at your patch. The dentry of a removed file becomes a negative
>> dentry. The kernel can keep track of those negative entries and there is no need
>> to add an additional flag for that.
>>
>> Cheers,
>> Longman
> One comment about your patch:
> In the patch 1/5 function dentry_kill first get dentry->d_flags, after lock parent and
> Compare d_flags again, is this needed? The d_flags was changed under lock.

Yes, it is necessary. We are talking about an SMP system with multiple
threads running concurrently. If you look at the lock parent code, it
may release the current dentry lock before taking the parent's and then
the dentry lock again. As soon as the lock is released, anything can
happen to the dentry including changes in d_flags.

> In my patch the DCACHE_FILE_REMOVED flag was to distinguish the removed file and
> The closed file, I found there was no difference of a dentry between the removed file and the closed
> File, they all on the lru list.

There is a difference between removed file and closed file. The type
field of d_flags will be empty for a removed file which indicate a
negative dentry. Anything else is a positive dentry. Look at the inline
function d_is_negative() [d_is_miss()] and you will see how it is done.

Cheers,
Longman