Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1712362 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v3 0/5] fs/dcache: Limit # of negative dentries Waiman Long <longman@redhat.com> - 2017-08-15 19:20 +0200
    RE: [PATCH v3 0/5] fs/dcache: Limit # of negative dentries "Wangkai (Kevin,C)" <wangkai86@huawei.com> - 2017-08-16 12:40 +0200
      Re: [PATCH v3 0/5] fs/dcache: Limit # of negative dentries Waiman Long <longman@redhat.com> - 2017-08-16 15:30 +0200
        RE: [PATCH v3 0/5] fs/dcache: Limit # of negative dentries "Wangkai (Kevin,C)" <wangkai86@huawei.com> - 2017-08-17 06:10 +0200
          Re: [PATCH v3 0/5] fs/dcache: Limit # of negative dentries Waiman Long <longman@redhat.com> - 2017-08-17 15:10 +0200
            RE: [PATCH v3 0/5] fs/dcache: Limit # of negative dentries "Wangkai (Kevin,C)" <wangkai86@huawei.com> - 2017-08-18 12:10 +0200
              Re: [PATCH v3 0/5] fs/dcache: Limit # of negative dentries Waiman Long <longman@redhat.com> - 2017-08-18 16:20 +0200
                RE: [PATCH v3 0/5] fs/dcache: Limit # of negative dentries "Wangkai (Kevin,C)" <wangkai86@huawei.com> - 2017-08-21 05:30 +0200
                  Re: [PATCH v3 0/5] fs/dcache: Limit # of negative dentries Waiman Long <longman@redhat.com> - 2017-08-21 15:40 +0200
                    RE: [PATCH v3 0/5] fs/dcache: Limit # of negative dentries "Wangkai (Kevin,C)" <wangkai86@huawei.com> - 2017-08-22 05:10 +0200

#1712362 — 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

[toc] | [next] | [standalone]

#1712830

> -----Original Message-----
> From: linux-fsdevel-owner@vger.kernel.org
> [mailto:linux-fsdevel-owner@vger.kernel.org] On Behalf Of Waiman Long
> Sent: Wednesday, August 16, 2017 1:15 AM
> To: Alexander Viro; Jonathan Corbet
> Cc: linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org;
> linux-fsdevel@vger.kernel.org; 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

[toc] | [prev] | [next] | [standalone]

#1712940

On 08/16/2017 06:33 AM, Wangkai (Kevin,C) wrote:
>> -----Original Message-----
>> From: linux-fsdevel-owner@vger.kernel.org
>> [mailto:linux-fsdevel-owner@vger.kernel.org] On Behalf Of Waiman Long
>> Sent: Wednesday, August 16, 2017 1:15 AM
>> To: Alexander Viro; Jonathan Corbet
>> Cc: linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org;
>> linux-fsdevel@vger.kernel.org; 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

[toc] | [prev] | [next] | [standalone]

#1713524

> -----Original Message-----
> From: Waiman Long [mailto:longman@redhat.com]
> Sent: Wednesday, August 16, 2017 9:29 PM
> To: Wangkai (Kevin,C); Alexander Viro; Jonathan Corbet
> Cc: linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org;
> linux-fsdevel@vger.kernel.org; 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@vger.kernel.org
> >> [mailto:linux-fsdevel-owner@vger.kernel.org] On Behalf Of Waiman Long
> >> Sent: Wednesday, August 16, 2017 1:15 AM
> >> To: Alexander Viro; Jonathan Corbet
> >> Cc: linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org;
> >> linux-fsdevel@vger.kernel.org; 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

[toc] | [prev] | [next] | [standalone]

#1714008

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

[toc] | [prev] | [next] | [standalone]

#1714777

> -----Original Message-----
> From: Waiman Long [mailto:longman@redhat.com]
> Sent: Thursday, August 17, 2017 9:04 PM
> To: Wangkai (Kevin,C); Alexander Viro; Jonathan Corbet
> Cc: linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org;
> linux-fsdevel@vger.kernel.org; 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/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.

Yes, I am not check the lock parent code, it is necessary.

> > 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.

After the file was removed, the dentry flag was not MISS, the flag was:
DCACHE_REFERENCED | DCACHE_RCUACCESS | DCACHE_LRU_LIST | DCACHE_REGULAR_TYPE
So, the dentry never be freed, until the kernel reclaim the slab memory.

Regards,
Kevin

[toc] | [prev] | [next] | [standalone]

#1715132

On 08/18/2017 05:59 AM, Wangkai (Kevin,C) wrote:
>
>>> 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.
> After the file was removed, the dentry flag was not MISS, the flag was:
> DCACHE_REFERENCED | DCACHE_RCUACCESS | DCACHE_LRU_LIST | DCACHE_REGULAR_TYPE
> So, the dentry never be freed, until the kernel reclaim the slab memory.

The dentry_unlink_inode() function will clear DCACHE_REGULAR_TYPE.

Cheers,
Longman

[toc] | [prev] | [next] | [standalone]

#1716081

> -----Original Message-----
> From: Waiman Long [mailto:longman@redhat.com]
> Sent: Friday, August 18, 2017 10:10 PM
> To: Wangkai (Kevin,C); Alexander Viro; Jonathan Corbet
> Cc: linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org;
> linux-fsdevel@vger.kernel.org; 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/18/2017 05:59 AM, Wangkai (Kevin,C) wrote:
> >
> >>> 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.
> > After the file was removed, the dentry flag was not MISS, the flag was:
> > DCACHE_REFERENCED | DCACHE_RCUACCESS | DCACHE_LRU_LIST |
> > DCACHE_REGULAR_TYPE So, the dentry never be freed, until the kernel
> reclaim the slab memory.
> 
> The dentry_unlink_inode() function will clear DCACHE_REGULAR_TYPE.
> 

Yes, I have add some trace info for the dentry state changed, with dentry flag and reference count:

File create:
[   42.636675] dentry [xxxx_1234] 0xffff880230be8180 flag 0x0 ref 1 ev dentry alloc
File close:
[   42.637421] dentry [xxxx_1234] 0xffff880230be8180 flag 0x4800c0 ref 0 ev dput called

Unlink lookup:
[  244.658086] dentry [xxxx_1234] 0xffff880230be8180 flag 0x4800c0 ref 1 ev d_lookup
Unlink d_delete:
[  244.658254] dentry [xxxx_1234] 0xffff880230be8180 flag 0x800c0 ref 1 ev d_lockref ref 1
Unlink dput:
[  244.658438] dentry [xxxx_1234] 0xffff880230be8180 flag 0x800c0 ref 0 ev dput called

The end, dentry's flag stay at 0x800c0, but this dentry was not freed, keeped by the dcache as unused,
After tens of thousands of the dentries slow down the dentry lookup performance, kernel memory usage
Keep high.

Regards,
Kevin

[toc] | [prev] | [next] | [standalone]

#1716481

On 08/20/2017 11:23 PM, Wangkai (Kevin,C) wrote:
>
> Yes, I have add some trace info for the dentry state changed, with dentry flag and reference count:
>
> File create:
> [   42.636675] dentry [xxxx_1234] 0xffff880230be8180 flag 0x0 ref 1 ev dentry alloc
> File close:
> [   42.637421] dentry [xxxx_1234] 0xffff880230be8180 flag 0x4800c0 ref 0 ev dput called
>
> Unlink lookup:
> [  244.658086] dentry [xxxx_1234] 0xffff880230be8180 flag 0x4800c0 ref 1 ev d_lookup
> Unlink d_delete:
> [  244.658254] dentry [xxxx_1234] 0xffff880230be8180 flag 0x800c0 ref 1 ev d_lockref ref 1
> Unlink dput:
> [  244.658438] dentry [xxxx_1234] 0xffff880230be8180 flag 0x800c0 ref 0 ev dput called
>
> The end, dentry's flag stay at 0x800c0, but this dentry was not freed, keeped by the dcache as unused,
> After tens of thousands of the dentries slow down the dentry lookup performance, kernel memory usage
> Keep high.
>
> Regards,
> Kevin

That is expected. The kernel does not get rid of negative dentries until
the shrinker is called because of memory pressure. Negative dentries do
help to improve file lookup performance. However, too much of negative
dentries suppress the amount of free memory available for other use.
That is why I send out my patch to limit the number of negative dentries
outstanding.

Cheers,
Longman

[toc] | [prev] | [next] | [standalone]

#1716993

> -----Original Message-----
> From: Waiman Long [mailto:longman@redhat.com]
> Sent: Monday, August 21, 2017 9:35 PM
> To: Wangkai (Kevin,C); Alexander Viro; Jonathan Corbet
> Cc: linux-kernel@vger.kernel.org; linux-doc@vger.kernel.org;
> linux-fsdevel@vger.kernel.org; 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/20/2017 11:23 PM, Wangkai (Kevin,C) wrote:
> >
> > Yes, I have add some trace info for the dentry state changed, with dentry flag
> and reference count:
> >
> > File create:
> > [   42.636675] dentry [xxxx_1234] 0xffff880230be8180 flag 0x0 ref 1 ev
> dentry alloc
> > File close:
> > [   42.637421] dentry [xxxx_1234] 0xffff880230be8180 flag 0x4800c0 ref 0
> ev dput called
> >
> > Unlink lookup:
> > [  244.658086] dentry [xxxx_1234] 0xffff880230be8180 flag 0x4800c0 ref
> > 1 ev d_lookup Unlink d_delete:
> > [  244.658254] dentry [xxxx_1234] 0xffff880230be8180 flag 0x800c0 ref
> > 1 ev d_lockref ref 1 Unlink dput:
> > [  244.658438] dentry [xxxx_1234] 0xffff880230be8180 flag 0x800c0 ref
> > 0 ev dput called
> >
> > The end, dentry's flag stay at 0x800c0, but this dentry was not freed,
> > keeped by the dcache as unused, After tens of thousands of the
> > dentries slow down the dentry lookup performance, kernel memory usage
> Keep high.
> >
> > Regards,
> > Kevin
> 
> That is expected. The kernel does not get rid of negative dentries until the
> shrinker is called because of memory pressure. Negative dentries do help to
> improve file lookup performance. However, too much of negative dentries
> suppress the amount of free memory available for other use.
> That is why I send out my patch to limit the number of negative dentries
> outstanding.
> 
I think there are two issue:
1. when a file was removed, the dentry should be deleted, this is as a bug, if the
Dentry memory cannot be reclaimed, there is a memory leak.

2. limit the dentries number can improve when there were lots of files operations,
And all the files were valid.

Regards,
Kevin 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web