linux-unionfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: cgxu <cgxu519@mykernel.net>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: Amir Goldstein <amir73il@gmail.com>, Ian Kent <raven@themaw.net>,
	Al Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	overlayfs <linux-unionfs@vger.kernel.org>
Subject: Re: [RFC PATCH v3 0/9] Suppress negative dentry
Date: Tue, 19 May 2020 17:23:43 +0800	[thread overview]
Message-ID: <7fcb778f-ba80-8095-4d48-20682f5242a9@mykernel.net> (raw)
In-Reply-To: <CAJfpegtyZw=6zqWQWm-fN0KpGEp9stcfvnbA7eh6E-7XHxaG=Q@mail.gmail.com>

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?


> }
>
> 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?

Thanks,
cgxu


  reply	other threads:[~2020-05-19  9:24 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-15  7:20 [RFC PATCH v3 0/9] Suppress negative dentry Chengguang Xu
2020-05-15  7:20 ` [RFC PATCH v3 1/9] fs/dcache: Introduce a new lookup flag LOOKUP_DONTCACHE_NEGATIVE Chengguang Xu
2020-05-15  7:20 ` [RFC PATCH v3 2/9] ovl: Suppress negative dentry in lookup Chengguang Xu
2020-05-15  7:20 ` [RFC PATCH v3 3/9] cifs: Adjust argument for lookup_positive_unlocked() Chengguang Xu
2020-05-15  7:20 ` [RFC PATCH v3 4/9] debugfs: " Chengguang Xu
2020-05-15  7:20 ` [RFC PATCH v3 5/9] ecryptfs: Adjust argument for lookup_one_len_unlocked() Chengguang Xu
2020-05-15  7:20 ` [RFC PATCH v3 6/9] exportfs: " Chengguang Xu
2020-05-15  7:20 ` [RFC PATCH v3 7/9] kernfs: Adjust argument for lookup_positive_unlocked() Chengguang Xu
2020-05-15  7:20 ` [RFC PATCH v3 8/9] nfsd: " Chengguang Xu
2020-05-15  7:20 ` [RFC PATCH v3 9/9] quota: " Chengguang Xu
2020-05-15  7:30 ` [RFC PATCH v3 0/9] Suppress negative dentry Amir Goldstein
2020-05-15  8:25   ` Chengguang Xu
2020-05-15  8:42     ` Miklos Szeredi
2020-05-18  0:53 ` Ian Kent
2020-05-18  5:27   ` Amir Goldstein
2020-05-18  7:52     ` Miklos Szeredi
2020-05-18  8:51       ` Amir Goldstein
2020-05-18  9:17         ` Miklos Szeredi
2020-05-19  5:01       ` cgxu
2020-05-19  8:21         ` Miklos Szeredi
2020-05-19  9:23           ` cgxu [this message]
2020-05-20 14:44             ` Miklos Szeredi
2020-05-25 13:37               ` Chengguang Xu
2020-05-25 13:50                 ` Miklos Szeredi
2020-05-18 10:26     ` Ian Kent
2020-05-18 10:39       ` Ian Kent

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=7fcb778f-ba80-8095-4d48-20682f5242a9@mykernel.net \
    --to=cgxu519@mykernel.net \
    --cc=amir73il@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=raven@themaw.net \
    --cc=viro@zeniv.linux.org.uk \
    /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).