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

[Wangkai (Kevin,C)]

> -----Original Message-----
> From: Waiman Long [mailto:longman@xxxxxxxxxx]
> Sent: Wednesday, August 16, 2017 9:29 PM
> To: Wangkai (Kevin,C); Alexander Viro; Jonathan Corbet
> Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-doc@xxxxxxxxxxxxxxx;
> linux-fsdevel@xxxxxxxxxxxxxxx; Paul E. McKenney; Andrew Morton; Ingo Molnar;
> Miklos Szeredi; Matthew Wilcox; Larry Woodman; James Bottomley
> Subject: Re: [PATCH v3 0/5] fs/dcache: Limit # of negative dentries
> 
> On 08/16/2017 06:33 AM, Wangkai (Kevin,C) wrote:
> >> -----Original Message-----
> >> From: linux-fsdevel-owner@xxxxxxxxxxxxxxx
> >> [mailto:linux-fsdevel-owner@xxxxxxxxxxxxxxx] On Behalf Of Waiman Long
> >> Sent: Wednesday, August 16, 2017 1:15 AM
> >> To: Alexander Viro; Jonathan Corbet
> >> Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-doc@xxxxxxxxxxxxxxx;
> >> linux-fsdevel@xxxxxxxxxxxxxxx; Paul E. McKenney; Andrew Morton; Ingo
> >> Molnar; Miklos Szeredi; Matthew Wilcox; Larry Woodman; James
> >> Bottomley
> >> Subject: Re: [PATCH v3 0/5] fs/dcache: Limit # of negative dentries
> >>
> >> On 07/28/2017 02:34 PM, Waiman Long wrote:
> >>>  v2->v3:
> >>>   - Add a faster pruning rate when the free pool is closed to depletion.
> >>>   - As suggested by James Bottomley, add an artificial delay waiting
> >>>     loop before killing a negative dentry and properly clear the
> >>>     DCACHE_KILL_NEGATIVE flag if killing doesn't happen.
> >>>   - Add a new patch to track number of negative dentries that are
> >>>     forcifully killed.
> >>>
> >>>  v1->v2:
> >>>   - Move the new nr_negative field to the end of dentry_stat_t structure
> >>>     as suggested by Matthew Wilcox.
> >>>   - With the help of Miklos Szeredi, fix incorrect locking order in
> >>>     dentry_kill() by using lock_parent() instead of locking the parent's
> >>>     d_lock directly.
> >>>   - Correctly account for positive to negative dentry transitions.
> >>>   - Automatic pruning of negative dentries will now ignore the reference
> >>>     bit in negative dentries but not the regular shrinking.
> >>>
> >>> A rogue application can potentially create a large number of
> >>> negative dentries in the system consuming most of the memory
> >>> available. This can impact performance of other applications running on the
> system.
> >>>
> >>> This patchset introduces changes to the dcache subsystem to limit
> >>> the number of negative dentries allowed to be created thus limiting
> >>> the amount of memory that can be consumed by negative dentries.
> >>>
> >>> Patch 1 tracks the number of negative dentries used and disallow the
> >>> creation of more when the limit is reached.
> >>>
> >>> Patch 2 enables /proc/sys/fs/dentry-state to report the number of
> >>> negative dentries in the system.
> >>>
> >>> Patch 3 enables automatic pruning of negative dentries when it is
> >>> close to the limit so that we won't end up killing recently used
> >>> negative dentries.
> >>>
> >>> Patch 4 prevents racing between negative dentry pruning and umount
> >>> operation.
> >>>
> >>> Patch 5 shows the number of forced negative dentry killings in
> >>> /proc/sys/fs/dentry-state. End users can then tune the
> >>> neg_dentry_pc= kernel boot parameter if they want to reduce forced
> >>> negative dentry killings.
> >>>
> >>> Waiman Long (5):
> >>>   fs/dcache: Limit numbers of negative dentries
> >>>   fs/dcache: Report negative dentry number in dentry-state
> >>>   fs/dcache: Enable automatic pruning of negative dentries
> >>>   fs/dcache: Protect negative dentry pruning from racing with umount
> >>>   fs/dcache: Track count of negative dentries forcibly killed
> >>>
> >>>  Documentation/admin-guide/kernel-parameters.txt |   7 +
> >>>  fs/dcache.c                                     | 451
> >> ++++++++++++++++++++++--
> >>>  include/linux/dcache.h                          |   8 +-
> >>>  include/linux/list_lru.h                        |   1 +
> >>>  mm/list_lru.c                                   |   4 +-
> >>>  5 files changed, 435 insertions(+), 36 deletions(-)
> >>>
> >> I haven't received any comment on this v3 patch for over 2 weeks. Is
> >> there anything I can do to make it more ready to be merged?
> >>
> >> Thanks,
> >> Longman
> > 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.

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.

Regards,
Kevin