Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8179c036 authored by Dave Chinner's avatar Dave Chinner Committed by Dave Chinner
Browse files

xfs: remove xfs_fs_evict_inode()



Joe Lawrence reported a list_add corruption with 4.6-rc1 when
testing some custom md administration code that made it's own
block device nodes for the md array. The simple test loop of:

for i in {0..100}; do
	mknod --mode=0600 $tmp/tmp_node b $MAJOR $MINOR
	mdadm --detail --export $tmp/tmp_node > /dev/null
	rm -f $tmp/tmp_node
done


Would produce this warning in bd_acquire() when mdadm opened the
device node:

list_add double add: new=ffff88043831c7b8, prev=ffff8804380287d8, next=ffff88043831c7b8.

And then produce this from bd_forget from kdevtmpfs evicting a block
dev inode:

list_del corruption. prev->next should be ffff8800bb83eb10, but was ffff88043831c7b8

This is a regression caused by commit c19b3b05 ("xfs: mode di_mode
to vfs inode"). The issue is that xfs_inactive() frees the
unlinked inode, and the above commit meant that this freeing zeroed
the mode in the struct inode. The problem is that after evict() has
called ->evict_inode, it expects the i_mode to be intact so that it
can call bd_forget() or cd_forget() to drop the reference to the
block device inode attached to the XFS inode.

In reality, the only thing we do in xfs_fs_evict_inode() that is not
generic is call xfs_inactive(). We can move the xfs_inactive() call
to xfs_fs_destroy_inode() without any problems at all, and this
will leave the VFS inode intact until it is completely done with it.

So, remove xfs_fs_evict_inode(), and do the work it used to do in
->destroy_inode instead.

cc: <stable@vger.kernel.org> # 4.6
Reported-by: default avatarJoe Lawrence <joe.lawrence@stratus.com>
Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
Signed-off-by: default avatarDave Chinner <david@fromorbit.com>
parent f55532a0
Loading
Loading
Loading
Loading
+7 −21
Original line number Diff line number Diff line
@@ -928,7 +928,7 @@ xfs_fs_alloc_inode(

/*
 * Now that the generic code is guaranteed not to be accessing
 * the linux inode, we can reclaim the inode.
 * the linux inode, we can inactivate and reclaim the inode.
 */
STATIC void
xfs_fs_destroy_inode(
@@ -938,9 +938,14 @@ xfs_fs_destroy_inode(

	trace_xfs_destroy_inode(ip);

	XFS_STATS_INC(ip->i_mount, vn_reclaim);
	ASSERT(!rwsem_is_locked(&ip->i_iolock.mr_lock));
	XFS_STATS_INC(ip->i_mount, vn_rele);
	XFS_STATS_INC(ip->i_mount, vn_remove);

	xfs_inactive(ip);

	ASSERT(XFS_FORCED_SHUTDOWN(ip->i_mount) || ip->i_delayed_blks == 0);
	XFS_STATS_INC(ip->i_mount, vn_reclaim);

	/*
	 * We should never get here with one of the reclaim flags already set.
@@ -987,24 +992,6 @@ xfs_fs_inode_init_once(
		     "xfsino", ip->i_ino);
}

STATIC void
xfs_fs_evict_inode(
	struct inode		*inode)
{
	xfs_inode_t		*ip = XFS_I(inode);

	ASSERT(!rwsem_is_locked(&ip->i_iolock.mr_lock));

	trace_xfs_evict_inode(ip);

	truncate_inode_pages_final(&inode->i_data);
	clear_inode(inode);
	XFS_STATS_INC(ip->i_mount, vn_rele);
	XFS_STATS_INC(ip->i_mount, vn_remove);

	xfs_inactive(ip);
}

/*
 * We do an unlocked check for XFS_IDONTCACHE here because we are already
 * serialised against cache hits here via the inode->i_lock and igrab() in
@@ -1663,7 +1650,6 @@ xfs_fs_free_cached_objects(
static const struct super_operations xfs_super_operations = {
	.alloc_inode		= xfs_fs_alloc_inode,
	.destroy_inode		= xfs_fs_destroy_inode,
	.evict_inode		= xfs_fs_evict_inode,
	.drop_inode		= xfs_fs_drop_inode,
	.put_super		= xfs_fs_put_super,
	.sync_fs		= xfs_fs_sync_fs,