On Thu, Oct 17, 2024 at 6:52 AM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, Oct 07, 2024 at 04:19:20PM +0200, Amir Goldstein wrote:
> > Hi all,
> >
> > This is v3 of the code to avoid temporary backing file opens in
> > overlayfs, taking into account Al's and Miklos' comments on v2 [1].
> >
> > If no further comments, this is going for overlayfs-next.
>
> BTW, looking through the vicinity of that stuff:
>
> int ovl_cleanup(struct ovl_fs *ofs, struct inode *wdir, struct dentry *wdentry)
> {
> int err;
>
> dget(wdentry);
> if (d_is_dir(wdentry))
> err = ovl_do_rmdir(ofs, wdir, wdentry);
> else
> err = ovl_do_unlink(ofs, wdir, wdentry);
> dput(wdentry);
>
> if (err) {
> pr_err("cleanup of '%pd2' failed (%i)\n",
> wdentry, err);
> }
>
> return err;
> }
>
> What the hell are those dget()/dput() doing there? Not to mention an
> access after dput(), both vfs_rmdir() and vfs_unlink() expect the
> reference to dentry argument to be held by the caller and leave it
> for the caller to dispose of. What am I missing here?
It has been like that since the first upstream version.
My guess is that it is an attempt to avoid turning wdentry
into a negative dentry, which is not expected to be useful in
ovl_clear_empty() situations, but this is just a guess.
Miklos?
Thanks,
Amir.
