All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shakeel Butt <shakeel.butt@linux.dev>
To: Christian Brauner <brauner@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	 Tejun Heo <tj@kernel.org>,
	Christian Brauner <christian@brauner.io>,
	 Meta kernel team <kernel-team@meta.com>,
	linux-kselftest@vger.kernel.org, driver-core@lists.linux.dev,
	 linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] selftests: cover kernfs dentry revalidation
Date: Fri, 4 Sep 2026 07:33:45 -0700	[thread overview]
Message-ID: <aprVEyqOrdmuD-HN@linux.dev> (raw)
In-Reply-To: <20260904-kandidat-bergbahn-ersonnen-3a6dd1f1cf0e@brauner>

On Fri, Sep 04, 2026 at 10:13:11AM +0200, Christian Brauner wrote:
> On Tue, Sep 01, 2026 at 06:40:50PM -0700, Shakeel Butt wrote:
> > kernfs_test only checked two xattr calls. It did not touch the part of
> > kernfs that userspace hits most: the dentry cache.
> > 
> > The kernel adds and removes kernfs nodes on its own, with no VFS call
> > behind it, and removal cannot always unhash the dentries that are
> > already cached. So ->d_revalidate() is what keeps the cache honest,
> > let's add tests for it.
> > 
> > Add tests that make the kernel change kernfs directly, instead of going
> > through create and unlink:
> > 
> >  - writing cgroup.subtree_control makes the kernel add and remove files
> >    in every child cgroup. That checks two things: a cached negative
> >    dentry is dropped once the file appears, and a cached positive one
> >    is dropped once the file goes away.
> >  - renaming a network interface renames the sysfs node under a dentry
> >    that is already cached.
> >  - sysfs mounted in a new network namespace must list that namespace's
> >    interfaces and not the parent's, which covers the KERNFS_NS tagging
> >    code.
> >  - many lookups running against concurrent mkdir and rmdir. A lookup
> >    may only succeed or fail with an errno that means "it is gone".
> >  - open across removal, readdir returning each entry once, and a
> >    telldir()/seekdir() round trip, which uses the name hash cookie in
> >    kernfs_dir_pos().
> > 
> > Test the other direction too: walking dentries that are already cached
> > must not drop them. That is not just a slowdown. d_invalidate() calls
> > detach_mounts(), so a dentry dropped by mistake also unmounts whatever
> > is mounted under that directory.
> > 
> > Both filesystems are needed. cgroup2 has no ->rename and is not
> > namespace tagged. sysfs cannot be mounted with a set of children we
> > control. The config fragment stops a kernel built with kselftest-merge
> > from passing while it runs almost nothing.
> > 
> > To confirm the tests really catch a broken backstop, kernfs_dir_changed()
> > was stubbed to return false. 8 of the 11 tests then failed.
> > 
> > Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
> > ---
> 
> I have to say I would appreciate trimmed-down commit messages with less
> relative clauses. This reads very LLMy still. Especially for tests my
> appetite for wading through multiple paragraphs of info is very low.
> Same goes for the very wordy changelog below...

My appologies, I will be more precise and concise in future.

I assume the kernfs bug fixes [1] will also go to your vfs branch. I will send a
v2 of that series with appropriate tags and will try to improve the commit
messages as well. I have a followup improvements for kernfs_rwsem which I will
base on vfs branch. Just wanted to give a headsup.

Thanks a lot, Christian.

[1] https://lore.kernel.org/all/20260903040253.670020-1-shakeel.butt@linux.dev/ 

  reply	other threads:[~2026-09-04 14:34 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-02  1:40 [PATCH v2] selftests: cover kernfs dentry revalidation Shakeel Butt
2026-09-04  8:07 ` Christian Brauner
2026-09-04  8:13 ` Christian Brauner
2026-09-04 14:33   ` Shakeel Butt [this message]
2026-09-04 14:44     ` Greg Kroah-Hartman
2026-09-04 15:34       ` Shakeel Butt
2026-09-09  8:23       ` Christian Brauner
2026-09-09  8:45         ` Greg Kroah-Hartman

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=aprVEyqOrdmuD-HN@linux.dev \
    --to=shakeel.butt@linux.dev \
    --cc=brauner@kernel.org \
    --cc=christian@brauner.io \
    --cc=driver-core@lists.linux.dev \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel-team@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=tj@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.