linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Waiman Long <longman@redhat.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
	Jonathan Corbet <corbet@lwn.net>,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Ingo Molnar <mingo@kernel.org>,
	Miklos Szeredi <mszeredi@redhat.com>
Subject: Re: [PATCH 1/4] fs/dcache: Limit numbers of negative dentries
Date: Mon, 17 Jul 2017 14:31:03 -0400	[thread overview]
Message-ID: <7eb6c2d8-9ac6-bda0-b515-a177bc73f791@redhat.com> (raw)
In-Reply-To: <20170717174939.GB14983@bombadil.infradead.org>

On 07/17/2017 01:49 PM, Matthew Wilcox wrote:
> On Mon, Jul 17, 2017 at 09:39:30AM -0400, Waiman Long wrote:
>> The number of positive dentries is limited by the number of files
>> in the filesystems. The number of negative dentries, however,
>> has no limit other than the total amount of memory available in
>> the system. So a rogue application that generates a lot of negative
>> dentries can potentially exhaust most of the memory available in the
>> system impacting performance on other running applications.
>>
>> To prevent this from happening, the dcache code is now updated to limit
>> the amount of the negative dentries in the LRU lists that can be kept
>> as a percentage of total available system memory. The default is 5%
>> and can be changed by specifying the "neg_dentry_pc=" kernel command
>> line option.
> I see the problem, but rather than restricting the number of negative
> dentries to be a fraction of the total amount of memory in the machine,
> wouldn't it make more sense to limit the number of negative dentries to be
> some multiple of the number of positive dentries currently in the system?

The number of positive dentries will be a rapidly changing number. So we
can't use __read_mostly variable for the limits. That may have a certain
performance impact. I chose to use a fixed number because of simplicity
and performance. I can compromise on simplicity, but not on performance.
I am open to maybe adjust the free pool count in some ways as long as
the performance impact is negligible.
 
> Or make negative dentries more easily prunable.  For example, we could
> allocate them from a separate slab and use the existing reclaim mechanism
> to just throw them away.  Since they can't be pinned by an inode, they're
> much easier to get rid of than positive dentries.  Might make changing
> a dentry from positive to negative or vice versa a bit more expensive ...

I don't quite understand what you mean by having two separate slabs. The
current reclaim mechanism is through scanning the LRU lists.

I had been thinking about having a separate LRU list for negative
dentries. Giving the complexity of the current per-node/per-memcg LRU
list, maintaining 2 separate LRU lists in each super_block may be
error-prone.

It is true that positive dentries will also be pruned in the process. By
the time automatic pruning happens, there should have a lot of negative
dentries in the LRU lists already. We can skip over positive dentries in
the scanning, but we have to either allow scanning more entries in each
pass prolonging the interruption or do no pruning at all if the LRU
lists are front-loaded with a bunch of positive dentries.

BTW, you remind me that I should have accounted for the
positive-to-negative dentry transitions which is missing in the current
patch.

Cheers,
Longman

  reply	other threads:[~2017-07-17 18:31 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-17 13:39 [PATCH 0/4] fs/dcache: Limit # of negative dentries Waiman Long
2017-07-17 13:39 ` [PATCH 1/4] fs/dcache: Limit numbers " Waiman Long
2017-07-17 17:49   ` Matthew Wilcox
2017-07-17 18:31     ` Waiman Long [this message]
2017-07-19 14:39   ` Miklos Szeredi
2017-07-19 15:02     ` Waiman Long
2017-07-19 20:24   ` Miklos Szeredi
2017-07-19 20:42     ` Waiman Long
2017-07-20  7:20       ` Miklos Szeredi
2017-07-20 14:21         ` Waiman Long
2017-07-20 15:08           ` Miklos Szeredi
2017-07-20 15:46             ` Waiman Long
2017-07-17 13:39 ` [PATCH 2/4] fs/dcache: Report negative dentry number in dentry-state Waiman Long
2017-07-17 14:09   ` Matthew Wilcox
2017-07-17 14:39     ` Waiman Long
2017-07-17 13:39 ` [PATCH 3/4] fs/dcache: Enable automatic pruning of negative dentries Waiman Long
2017-07-17 13:39 ` [PATCH 4/4] fs/dcache: Protect negative dentry pruning from racing with umount Waiman Long

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7eb6c2d8-9ac6-bda0-b515-a177bc73f791@redhat.com \
    --to=longman@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=corbet@lwn.net \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mszeredi@redhat.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).