xfs: don't ever put nlink > 0 inodes on the unlinked list

When XFS creates an O_TMPFILE file, the inode is created with nlink = 1,
put on the unlinked list, and then the VFS sets nlink = 0 in d_tmpfile.
If we crash before anything logs the inode (it's dirty incore but the
vfs doesn't tell us it's dirty so we never log that change), the iunlink
processing part of recovery will then explode with a pile of:

XFS: Assertion failed: VFS_I(ip)->i_nlink == 0, file:
fs/xfs/xfs_log_recover.c, line: 5072

Worse yet, since nlink is nonzero, the inodes also don't get cleaned up
and they just leak until the next xfs_repair run.

Therefore, change xfs_iunlink to require that inodes being put on the
unlinked list have nlink == 0, change the tmpfile callers to instantiate
nodes that way, and set the nlink to 1 just prior to calling d_tmpfile.
Fix the comment for xfs_iunlink while we're at it.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
This commit is contained in:
Darrick J. Wong 2019-02-13 11:15:17 -08:00
parent 15a268d9f2
commit c4a6bf7f6c
2 changed files with 17 additions and 12 deletions

View File

@ -1332,7 +1332,7 @@ xfs_create_tmpfile(
if (error) if (error)
goto out_trans_cancel; goto out_trans_cancel;
error = xfs_dir_ialloc(&tp, dp, mode, 1, 0, prid, &ip); error = xfs_dir_ialloc(&tp, dp, mode, 0, 0, prid, &ip);
if (error) if (error)
goto out_trans_cancel; goto out_trans_cancel;
@ -2231,11 +2231,8 @@ out:
} }
/* /*
* This is called when the inode's link count goes to 0 or we are creating a * This is called when the inode's link count has gone to 0 or we are creating
* tmpfile via O_TMPFILE. In the case of a tmpfile, @ignore_linkcount will be * a tmpfile via O_TMPFILE. The inode @ip must have nlink == 0.
* set to true as the link count is dropped to zero by the VFS after we've
* created the file successfully, so we have to add it to the unlinked list
* while the link count is non-zero.
* *
* We place the on-disk inode on a list in the AGI. It will be pulled from this * We place the on-disk inode on a list in the AGI. It will be pulled from this
* list when the inode is freed. * list when the inode is freed.
@ -2254,6 +2251,7 @@ xfs_iunlink(
short bucket_index = agino % XFS_AGI_UNLINKED_BUCKETS; short bucket_index = agino % XFS_AGI_UNLINKED_BUCKETS;
int error; int error;
ASSERT(VFS_I(ip)->i_nlink == 0);
ASSERT(VFS_I(ip)->i_mode != 0); ASSERT(VFS_I(ip)->i_mode != 0);
trace_xfs_iunlink(ip); trace_xfs_iunlink(ip);
@ -3184,11 +3182,9 @@ xfs_rename_alloc_whiteout(
/* /*
* Prepare the tmpfile inode as if it were created through the VFS. * Prepare the tmpfile inode as if it were created through the VFS.
* Otherwise, the link increment paths will complain about nlink 0->1. * Complete the inode setup and flag it as linkable. nlink is already
* Drop the link count as done by d_tmpfile(), complete the inode setup * zero, so we can skip the drop_nlink.
* and flag it as linkable.
*/ */
drop_nlink(VFS_I(tmpfile));
xfs_setup_iops(tmpfile); xfs_setup_iops(tmpfile);
xfs_finish_inode_setup(tmpfile); xfs_finish_inode_setup(tmpfile);
VFS_I(tmpfile)->i_state |= I_LINKABLE; VFS_I(tmpfile)->i_state |= I_LINKABLE;

View File

@ -191,9 +191,18 @@ xfs_generic_create(
xfs_setup_iops(ip); xfs_setup_iops(ip);
if (tmpfile) 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); d_tmpfile(dentry, inode);
else } else
d_instantiate(dentry, inode); d_instantiate(dentry, inode);
xfs_finish_inode_setup(ip); xfs_finish_inode_setup(ip);