Re: [PATCH 2/3] xfs: don't ever put nlink > 0 inodes on the unlinked list

Darrick J. Wong
2019-02-14T16:03:51Z

On Thu, Feb 14, 2019 at 12:15:56AM -0800, Christoph Hellwig wrote:
> On Wed, Feb 13, 2019 at 12:50:53PM -0800, Darrick J. Wong wrote:
> > + if (tmpfile) {
> > + /*
> > + * The VFS requires that any inode fed to d_tmpfile must have
> > + * nlink == 1 so that it can decrement the nlink in d_tmpfile.
> > + * However, we created the temp file with nlink == 0 because
> > + * we're not allowed to put an inode with nlink > 0 on the
> > + * unlinked list. Therefore we have to set nlink to 1 so that
> > + * d_tmpfile can immediately set it back to zero.
> > + */
> > + set_nlink(inode, 1);
> > d_tmpfile(dentry, inode);
> > + } else
>
> At least btrtfs has to work around these d_tmpfile assumptions as well.
> Instead of piling hacks over hacks I'd rather move the call to
> inode_dec_link_count from d_tmpfile, which should lead to a saner
> interface.

I'm working on a bigger change to fix the d_tmpfile behavior, but that's
a complex multi-fs change that may or may not make it for 5.1. :(

In the meantime this prevents leaking inodes during unlink recovery by
ensuring that we never put linked inodes on the unlink list so I would
still like to get this one reviewed. :)

--D