Re: [RFC][overlayfs] do we still need d_instantiate_anon() and export of d_alloc_anon()?

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

On Sun, Nov 19, 2023 at 08:57:25AM +0200, Amir Goldstein wrote:
> On Sat, Nov 18, 2023 at 10:02 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > On Sun, Nov 12, 2023 at 09:26:28AM +0200, Amir Goldstein wrote:
> >
> > > Tested the patch below.
> > > If you want to apply it as part of dcache cleanup, it's fine by me.
> > > Otherwise, I will queue it for the next overlayfs update.
> >
> > OK...  Let's do it that way - overlayfs part goes into never-rebased branch
> > (no matter which tree), pulled into dcache series and into your overlayfs
> > update, with removal of unused stuff done in a separate patch in dcache
> > series.
> >
> 
> Sounds good.
> 
> > That way we won't step on each other's toes when reordering, etc.
> > Does that work for you?  I can put the overlayfs part into #no-rebase-overlayfs
> > in vfs.git, or you could do it in a v6.7-rc1-based branch in your tree -
> > whatever's more convenient for you.
> 
> I've reset overlayfs-next to no-rebase-overlayfs, as it  had my version
> with removal so far.
> 
> For the final update, I doubt I will need to include it at all, because
> the chances of ovl_obtain_alias() colliding with anything for the next
> cycle are pretty slim, but it's good that I have the option and I will
> anyway make sure to always test the next update with this change.

OK...  Several overlayfs locking questions:
ovl_indexdir_cleanup()
{
	...
	inode_lock_nested(dir, I_MUTEX_PARENT);
	...
		index = ovl_lookup_upper(ofs, p->name, indexdir, p->len);
		...
                        err = ovl_cleanup_and_whiteout(ofs, dir, index);

with ovl_cleanup_and_whiteout() moving stuff between workdir and parent of index.
Where do you do lock_rename()?  It's a cross-directory rename, so it *must*
lock both (and take ->s_vfs_rename_mutex as well).  How can that possibly
work?

Similar in ovl_cleanup_index() - you lock indexdir, then call
ovl_cleanup_and_whiteout(), with the same locking issues.

Another fun question: ovl_copy_up_one() has
        if (parent) {
                ovl_path_upper(parent, &parentpath);
                ctx.destdir = parentpath.dentry;
                ctx.destname = dentry->d_name;

                err = vfs_getattr(&parentpath, &ctx.pstat,
                                  STATX_ATIME | STATX_MTIME,
                                  AT_STATX_SYNC_AS_STAT);
                if (err)
                        return err;
        }
What stabilizes dentry->d_name here?  I might be missing something about the
locking environment here, so it might be OK, but...