From: Al Viro <viro@zeniv.linux.org.uk>
To: NeilBrown <neil@brown.name>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Christian Brauner <brauner@kernel.org>,
linux-fsdevel@vger.kernel.org, Jan Kara <jack@suse.cz>
Subject: Re: [RFC] a possible way of reducing the PITA of ->d_name audits
Date: Mon, 8 Sep 2025 06:19:51 +0100 [thread overview]
Message-ID: <20250908051951.GI31600@ZenIV> (raw)
In-Reply-To: <175730701033.2850467.1822935583045267017@noble.neil.brown.name>
On Mon, Sep 08, 2025 at 02:50:10PM +1000, NeilBrown wrote:
> On Mon, 08 Sep 2025, Al Viro wrote:
> > That way xfs hits will be down to that claim_stability() and the obscenity in
> > trace.h - until the users of the latter get wrapped into something that would
> > take snapshots and pass those instead of messing with ->d_name. Considering
> > the fun quoted above, not having to repeat that digging is something I'd
> > count as a win...
> >
>
> What would you think of providing an accessor function and insisting
> everyone use it - and have some sort of lockdep_assert_held() to that
> function so that developers who test their code will see these problem?
>
> Then a simple grep can find any unapproved uses.
Really? Consider ->link(). Both arguments *are* stable, but the reasons
are not just different - they don't even intersect.
Old: known to be a regular file, held locked. The former guarantees that
it can't be moved by d_splice_alias(), the latter prevents that being done
by vfs_rename(), which also locks the objects being moved.
New: has been looked up after its parent had been locked. Note that _after_
is important here - you can't just blindly fetch ->d_parent and check if
its inode is locked (not to mention anything else, you'd need to check it
being non-NULL, do it under rcu_read_lock(), et sodding cetera - ->d_parent
stability rules are not that different from ->d_name ones).
And this "everyone use it" is not going to fly - you still have places where
it's done simply under ->d_lock. Or ->d_lock on known parent - either would
suffice.
Besides, there's "which primitive do I use here?" - with the annotation approach
it's as simple as "if I have it as stable_dentry, just use stable_dentry_name(),
otherwise think hard - it may be tricky".
I don't believe that lockdep is an answer here - annotations (and these *are*
annotations - no code generation changes at all) give better coverage, and
bitrot tends to happen in rarely taken failure exits.
next prev parent reply other threads:[~2025-09-08 5:19 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-07 20:32 [RFC] a possible way of reducing the PITA of ->d_name audits Al Viro
2025-09-07 21:51 ` Linus Torvalds
2025-09-08 0:06 ` Al Viro
2025-09-08 0:47 ` Linus Torvalds
2025-09-08 2:51 ` Al Viro
2025-09-08 3:57 ` Al Viro
2025-09-08 4:50 ` NeilBrown
2025-09-08 5:19 ` Al Viro [this message]
2025-09-08 6:25 ` NeilBrown
2025-09-08 9:05 ` Al Viro
2025-09-10 2:45 ` NeilBrown
2025-09-10 7:24 ` Al Viro
2025-09-10 22:52 ` NeilBrown
2025-09-12 5:49 ` ->atomic_open() fun (was Re: [RFC] a possible way of reducing the PITA of ->d_name audits) Al Viro
2025-09-12 8:23 ` Miklos Szeredi
2025-09-12 18:29 ` Al Viro
2025-09-12 19:22 ` Miklos Szeredi
2025-09-12 20:36 ` Al Viro
2025-09-12 20:50 ` Al Viro
2025-09-13 3:36 ` NeilBrown
2025-09-13 5:07 ` Al Viro
2025-09-13 5:50 ` NeilBrown
2025-09-14 19:01 ` Miklos Szeredi
2025-09-14 19:50 ` Al Viro
2025-09-14 20:05 ` Miklos Szeredi
2025-09-15 8:54 ` Bernd Schubert
2025-09-12 18:55 ` Al Viro
2025-09-12 18:59 ` [PATCH 1/9] allow finish_no_open(file, ERR_PTR(-E...)) Al Viro
2025-09-12 18:59 ` [PATCH 2/9] 9p: simplify v9fs_vfs_atomic_open() Al Viro
2025-09-12 18:59 ` [PATCH 3/9] 9p: simplify v9fs_vfs_atomic_open_dotl() Al Viro
2025-09-12 18:59 ` [PATCH 4/9] simplify cifs_atomic_open() Al Viro
2025-09-12 18:59 ` [PATCH 5/9] simplify vboxsf_dir_atomic_open() Al Viro
2025-09-12 18:59 ` [PATCH 6/9] simplify nfs_atomic_open_v23() Al Viro
2025-09-12 18:59 ` [PATCH 7/9] simplify fuse_atomic_open() Al Viro
2025-09-12 18:59 ` [PATCH 8/9] simplify gfs2_atomic_open() Al Viro
2025-09-12 18:59 ` [PATCH 9/9] slightly simplify nfs_atomic_open() Al Viro
2025-09-12 22:23 ` [PATCH 1/9] allow finish_no_open(file, ERR_PTR(-E...)) Linus Torvalds
2025-09-13 3:34 ` NeilBrown
2025-09-13 21:28 ` [RFC] a possible way of reducing the PITA of ->d_name audits Al Viro
2025-09-14 1:05 ` NeilBrown
2025-09-14 1:37 ` Al Viro
2025-09-14 5:56 ` Al Viro
2025-09-14 23:07 ` NeilBrown
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=20250908051951.GI31600@ZenIV \
--to=viro@zeniv.linux.org.uk \
--cc=brauner@kernel.org \
--cc=jack@suse.cz \
--cc=linux-fsdevel@vger.kernel.org \
--cc=neil@brown.name \
--cc=torvalds@linux-foundation.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.