Re: [RFC PATCH v3 0/9] Suppress negative dentry

[Chengguang Xu <cgxu519@mykernel.net>]

在 5/20/2020 10:44 PM, Miklos Szeredi 写道:
> On Tue, May 19, 2020 at 11:24 AM cgxu <cgxu519@mykernel.net> wrote:
>> On 5/19/20 4:21 PM, Miklos Szeredi wrote:
>>> On Tue, May 19, 2020 at 7:02 AM cgxu <cgxu519@mykernel.net> wrote:
>>>
>>>> If we don't consider that only drop negative dentry of our lookup,
>>>> it is possible to do like below, isn't it?
>>> Yes, the code looks good, though I'd consider using d_lock on dentry
>>> instead if i_lock on parent, something like this:
>>>
>>> if (d_is_negative(dentry) && dentry->d_lockref.count == 1) {
>>>       spin_lock(&dentry->d_lock);
>>>       /* Recheck condition under lock */
>>>       if (d_is_negative(dentry) && dentry->d_lockref.count == 1)
>>>           __d_drop(dentry)
>>>       spin_unlock(&dentry->d_lock);
>> And after this we will still treat 'dentry' as negative dentry and dput it
>> regardless of the second check result of d_is_negative(dentry), right?
> I'd restructure it in the same way as lookup_positive_unlocked()...
>
>>> }
>>>
>>> But as Amir noted, we do need to take into account the case where
>>> lower layers are shared by multiple overlays, in which case dropping
>>> the negative dentries could result in a performance regression.
>>> Have you looked at that case, and the effect of this patch on negative
>>> dentry lookup performance?
>> The container which is affected by this feature is just take advantage
>> of previous another container but we could not guarantee that always
>> happening. I think there no way for best of both worlds, consider that
>> some malicious containers continuously make negative dentries by
>> searching non-exist files, so that page cache of clean data, clean
>> inodes/dentries will be freed by memory reclaim. All of those
>> behaviors will impact the performance of other container instances.
>>
>> On the other hand, if this feature significantly affects particular
>> container,
>> doesn't that mean the container is noisy neighbor and should be restricted
>> in some way?
> Not necessarily.   Negative dentries can be useful and in case of
> layers shared between two containers having negative dentries cached
> in the lower layer can in theory positively affect performance.   I
> don't have data to back this up, nor the opposite.  You should run
> some numbers for container startup times with and without this patch.

I did some simple tests  for it but the result seems not very steady, so 
I need to take time to do more detail tests later. Is it possible to 
apply the patch for upper layer first?

Thanks,
cgxu