On Mon, May 29, 2023 at 12:59:50PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> xfs_bmap_longest_free_extent() can return an error when accessing
> the AGF fails. In this case, the behaviour of
> xfs_filestream_pick_ag() is conditional on the error. We may
> continue the loop, or break out of it. The error handling after the
> loop cleans up the perag reference held when the break occurs. If we
> continue, the next loop iteration handles cleaning up the perag
> reference.
>
> EIther way, we don't need to release the active perag reference when
> xfs_bmap_longest_free_extent() fails. Doing so means we do a double
> decrement on the active reference count, and this causes tha active
> reference count to fall to zero. At this point, new active
> references will fail.
>
> This leads to unmount hanging because it tries to grab active
> references to that perag, only for it to fail. This happens inside a
> loop that retries until a inode tree radix tree tag is cleared,
> which cannot happen because we can't get an active reference to the
> perag.
Wouldn't it be nice if C had^W^Wthe kernel coding rules allowed
iterators that managed these active refs for us...
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> The unmount livelocks in this path:
>
> xfs_reclaim_inodes+0x80/0xc0
> xfs_unmount_flush_inodes+0x5b/0x70
> xfs_unmountfs+0x5b/0x1a0
> xfs_fs_put_super+0x49/0x110
> generic_shutdown_super+0x7c/0x1a0
> kill_block_super+0x27/0x50
> deactivate_locked_super+0x30/0x90
> deactivate_super+0x3c/0x50
> cleanup_mnt+0xc2/0x160
> __cleanup_mnt+0x12/0x20
> task_work_run+0x5e/0xa0
> exit_to_user_mode_prepare+0x1bc/0x1c0
> syscall_exit_to_user_mode+0x16/0x40
> do_syscall_64+0x40/0x80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
>
> Reported-by: Pengfei Xu <pengfei.xu@xxxxxxxxx>
> Fixes: eb70aa2d8ed9 ("xfs: use for_each_perag_wrap in xfs_filestream_pick_ag")
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> fs/xfs/xfs_filestream.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_filestream.c b/fs/xfs/xfs_filestream.c
> index 22c13933c8f8..2fc98d313708 100644
> --- a/fs/xfs/xfs_filestream.c
> +++ b/fs/xfs/xfs_filestream.c
> @@ -78,7 +78,6 @@ xfs_filestream_pick_ag(
> *longest = 0;
> err = xfs_bmap_longest_free_extent(pag, NULL, longest);
> if (err) {
> - xfs_perag_rele(pag);
> if (err != -EAGAIN)
> break;
> /* Couldn't lock the AGF, skip this AG. */
> --
> 2.40.1
>
