Re: [PATCH][RFC] make take_dentry_name_snapshot() lockless

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

On Tue, Dec 10, 2024 at 02:45:23AM +0000, Al Viro wrote:
> On Mon, Dec 09, 2024 at 11:12:37PM +0000, Al Viro wrote:
> > 
> > Actually, grepping for DNAME_INLINE_LEN brings some interesting hits:
> > drivers/net/ieee802154/adf7242.c:1165:  char debugfs_dir_name[DNAME_INLINE_LEN + 1];
> > 	cargo-culted, AFAICS; would be better off with a constant of their own.
> > 
> > fs/ext4/fast_commit.c:326:              fc_dentry->fcd_name.len > DNAME_INLINE_LEN)
> > fs/ext4/fast_commit.c:452:      if (dentry->d_name.len > DNAME_INLINE_LEN) {
> > fs/ext4/fast_commit.c:1332:                     fc_dentry->fcd_name.len > DNAME_INLINE_LEN)
> > fs/ext4/fast_commit.h:113:      unsigned char fcd_iname[DNAME_INLINE_LEN];      /* Dirent name string */
> > 	Looks like that might want struct name_snapshot, along with
> > {take,release}_dentry_name_snapshot().
> 
> See viro/vfs.git#work.dcache.  I've thrown ext4/fast_commit conversion
> into the end of that pile.  NOTE: that stuff obviously needs profiling.
> It does survive light testing (boot/ltp/xfstests), but review and more
> testing (including serious profiling) is obviously needed.
> 
> Patches in followups...

More fun with ->d_name, ->d_iname and friends:

87ce955b24c9 "io_uring: add ->show_fdinfo() for the io_uring file descriptor"
is playing silly buggers with ->d_iname for some reason.  This
        seq_printf(m, "UserFiles:\t%u\n", ctx->file_table.data.nr);
        for (i = 0; has_lock && i < ctx->file_table.data.nr; i++) {
                struct file *f = NULL;

                if (ctx->file_table.data.nodes[i])
                        f = io_slot_file(ctx->file_table.data.nodes[i]);
                if (f)
                        seq_printf(m, "%5u: %s\n", i, file_dentry(f)->d_iname);
                else
                        seq_printf(m, "%5u: <none>\n", i);
        }
produces user-visible data.  For each slot in io_uring file table you
show either that it's empty (fine) or, for files with short names, the
last component of the name (no quoting, etc. - just a string as-is) or
the last short name that dentry used to have.

And that's a user-visible ABI.  What the hell?

NOTE: file here is may be anything whatsoever.  It may be a pipe,
an arbitrary file in tmpfs, a socket, etc.

How hard an ABI it is?  If it's really used by random userland code
(admin tools, etc.), we have a problem.  If that thing is cast in
stone, we'll have to emulate the current behaviour of that code,
no matter what.  I really hope it can be replaced with something
saner, though.

Incidentally, call your file "<none>"; is the current behaviour
the right thing to do?

What behaviour _is_ actually wanted?  Jens, Jann?