On Wed, May 11, 2016 at 10:52:00AM -0700, Bart Van Assche wrote:
> Hi Dave,
>
> While retesting the SRP initiator with xfstests on top of an XFS filesystem I
> hit the below call trace once. I do not expect that this is related to the SRP
> initiator changes I made. Please let me know if you need more information.
YALFP.(*)
It's a "problem" with the filesystem freeze protection when
interacting with emergency ENOSPC paths in the filesystem.
sb_start_intwrite() can block when a freeze is being progressed, and
hence on the surface this looks like a trivial ABBA deadlock
condition.
However, this *cannot deadlock* because the trigger for
sb_start_intwrite() to block is a change of state to
SB_FREEZE_COMPLETE, and that happens under a
down_write(sb->s_umount) context. i.e.:
int freeze_super(struct super_block *sb)
{
int ret;
atomic_inc(&sb->s_active);
down_write(&sb->s_umount)
.....
The XFS code that takes s_sync_lock here:
> nc_lock){+.+...}:
> [<ffffffff810a4e70>] lock_acquire+0x60/0x80
> [<ffffffff81582d2f>] mutex_lock_nested+0x5f/0x360
> [<ffffffff811bba2d>] sync_inodes_sb+0xbd/0x1d0
> [<ffffffffa0616963>] xfs_flush_inodes+0x23/0x30 [xfs]
Does:
if (down_read_trylock(&sb->s_umount)) {
sync_inodes_sb(sb);
up_read(&sb->s_umount);
}
So any attempt to freeze the filesystem is prevented /before/ XFS
tries to flush data in this ENOSPC path. And if a freeze is
alreadybeing processed, XFS will skip this path, thereby avoiding
the possibility of a ABBA deadlock between freeze and s_sync_lock.
> [<ffffffffa060ea9f>] xfs_create+0x46f/0x5f0 [xfs]
> [<ffffffffa0609f39>] xfs_generic_create+0x1b9/0x290 [xfs]
> [<ffffffffa060a03f>] xfs_vn_mknod+0xf/0x20 [xfs]
> [<ffffffffa060a07e>] xfs_vn_create+0xe/0x10 [xfs]
> [<ffffffff8119af96>] vfs_create+0x76/0xd0
> [<ffffffff8119f13e>] path_openat+0xc1e/0x10d0
> [<ffffffff811a04d9>] do_filp_open+0x79/0xd0
> [<ffffffff8118f636>] do_sys_open+0x116/0x1f0
> [<ffffffff8118f769>] SyS_creat+0x19/0x20
> [<ffffffff81585fe5>] entry_SYSCALL_64_fastpath+0x18/0xa8
>
> nal#2){++++.+}:
> [<ffffffff810a461f>] __lock_acquire+0x1b0f/0x1b20
> [<ffffffff810a4e70>] lock_acquire+0x60/0x80
> [<ffffffff8109edd5>] percpu_down_read+0x45/0x90
> [<ffffffff81193172>] __sb_start_write+0xb2/0xf0
> [<ffffffffa061838f>] xfs_trans_alloc+0x1f/0x40 [xfs]
> [<ffffffffa060f4d0>] xfs_inactive_truncate+0x20/0x130 [xfs]
> [<ffffffffa060fc9e>] xfs_inactive+0x1ae/0x1e0 [xfs]
> [<ffffffffa0614e88>] xfs_fs_evict_inode+0xb8/0xc0 [xfs]
> [<ffffffff811acc83>] evict+0xb3/0x180
> [<ffffffff811acefc>] iput+0x14c/0x1e0
> [<ffffffff811bbab5>] sync_inodes_sb+0x145/0x1d0
> [<ffffffff811c23e0>] sync_inodes_one_sb+0x10/0x20
> [<ffffffff81193bda>] iterate_supers+0xaa/0x100
And iterate_supers() does the same thing:
down_read(&sb->s_umount);
if (sb->s_root && (sb->s_flags & MS_BORN))
f(sb, arg);
up_read(&sb->s_umount);
IOWs, freeze_super() cannot start/progress freeze state to the point
that it would cause a deadlock in this code path because we hold the
lock that freeze_super() requires to change the freeze state.
Cheers,
Dave.
(*) YALFP: Yet Another Lockdep False Positive
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
