Re: [RFC] dentry->d_flags locking

[Al Viro <viro@zeniv.linux.org.uk>]

On Mon, Feb 24, 2025 at 03:07:03PM +0100, Miklos Szeredi wrote:
> On Mon, 24 Feb 2025 at 02:06, Al Viro <viro@zeniv.linux.org.uk> wrote:
> 
> > PS: turns out that set_default_d_op() is slightly more interesting
> > than I expected - fuse has separate dentry_operations for its
> > root dentry.  I don't see the point, TBH - the only difference is
> > that root one lacks
> >         * ->d_delete() (never even called for root dentry; it's
> > only called if d_unhashed() is false)
> >         * ->d_revalidate() (never called for root)
> >         * ->d_automount() (not even looked at unless you have
> > S_AUTOMOUNT set on the inode).
> > What's wrong with using fuse_dentry_operations for root dentry?
> > Am I missing something subtle here?  Miklos?
> 
> Looks like a historical accident:
> 
>  - initial version only had .d_revalidate and only set d_op in .loookup.
>  - then it grew .d_init/.d_release
>  - then, as a bugfix, the separate fuse_root_dentry_operations was added

Speaking of historical accidents - "seqlock: Add a new locking reader type"
(introduction of read_seqlock_excl()) happened a week after your
"vfs: check unlinked ancestors before mount" (Sep 12 and Sep 5 2013, resp.)

I'm switching d_set_mounted() from write_seqlock(&rename_lock); to
read_seqlock_excl(&rename_lock) - we need exclusion with movers,
but there's no need for anyone to retry ancestry chains or hash lookups
just because we have marked something as a mountpoint.

IOW, it's a reader, not writer; we can't replace it with need_seqretry
loop without more audit of d_mountpoint() callers I want to mess with
at the moment[*], but exclusive reader is fine.


[*] the ones under ->d_lock are not a problem and so are all that
are followed by "check if something's mounted in our namespace,
ignore if there isn't" as well as the ones that have an exclusion with
the caller of d_set_mounted(), but for now let's not go there.