From: Sasha Levin <sashal@kernel.org>
To: Suraj Jitindar Singh <surajjs@amazon.com>
Cc: stable@vger.kernel.org, sjitindarsingh@gmail.com,
linux-xfs@vger.kernel.org, kaixuxia <xiakaixu1987@gmail.com>,
kaixuxia <kaixuxia@tencent.com>,
"Darrick J . Wong" <darrick.wong@oracle.com>
Subject: Re: [PATCH STABLE v4.14.y 2/2] xfs: Fix deadlock between AGI and AGF with RENAME_WHITEOUT
Date: Fri, 24 Apr 2020 21:49:39 -0400 [thread overview]
Message-ID: <20200425014939.GE13035@sasha-vm> (raw)
In-Reply-To: <20200424230532.2852-3-surajjs@amazon.com>
On Fri, Apr 24, 2020 at 04:05:32PM -0700, Suraj Jitindar Singh wrote:
>From: kaixuxia <xiakaixu1987@gmail.com>
>
>commit bc56ad8c74b8588685c2875de0df8ab6974828ef upstream.
>
>When performing rename operation with RENAME_WHITEOUT flag, we will
>hold AGF lock to allocate or free extents in manipulating the dirents
>firstly, and then doing the xfs_iunlink_remove() call last to hold
>AGI lock to modify the tmpfile info, so we the lock order AGI->AGF.
>
>The big problem here is that we have an ordering constraint on AGF
>and AGI locking - inode allocation locks the AGI, then can allocate
>a new extent for new inodes, locking the AGF after the AGI. Hence
>the ordering that is imposed by other parts of the code is AGI before
>AGF. So we get an ABBA deadlock between the AGI and AGF here.
>
>Process A:
>Call trace:
> ? __schedule+0x2bd/0x620
> schedule+0x33/0x90
> schedule_timeout+0x17d/0x290
> __down_common+0xef/0x125
> ? xfs_buf_find+0x215/0x6c0 [xfs]
> down+0x3b/0x50
> xfs_buf_lock+0x34/0xf0 [xfs]
> xfs_buf_find+0x215/0x6c0 [xfs]
> xfs_buf_get_map+0x37/0x230 [xfs]
> xfs_buf_read_map+0x29/0x190 [xfs]
> xfs_trans_read_buf_map+0x13d/0x520 [xfs]
> xfs_read_agf+0xa6/0x180 [xfs]
> ? schedule_timeout+0x17d/0x290
> xfs_alloc_read_agf+0x52/0x1f0 [xfs]
> xfs_alloc_fix_freelist+0x432/0x590 [xfs]
> ? down+0x3b/0x50
> ? xfs_buf_lock+0x34/0xf0 [xfs]
> ? xfs_buf_find+0x215/0x6c0 [xfs]
> xfs_alloc_vextent+0x301/0x6c0 [xfs]
> xfs_ialloc_ag_alloc+0x182/0x700 [xfs]
> ? _xfs_trans_bjoin+0x72/0xf0 [xfs]
> xfs_dialloc+0x116/0x290 [xfs]
> xfs_ialloc+0x6d/0x5e0 [xfs]
> ? xfs_log_reserve+0x165/0x280 [xfs]
> xfs_dir_ialloc+0x8c/0x240 [xfs]
> xfs_create+0x35a/0x610 [xfs]
> xfs_generic_create+0x1f1/0x2f0 [xfs]
> ...
>
>Process B:
>Call trace:
> ? __schedule+0x2bd/0x620
> ? xfs_bmapi_allocate+0x245/0x380 [xfs]
> schedule+0x33/0x90
> schedule_timeout+0x17d/0x290
> ? xfs_buf_find+0x1fd/0x6c0 [xfs]
> __down_common+0xef/0x125
> ? xfs_buf_get_map+0x37/0x230 [xfs]
> ? xfs_buf_find+0x215/0x6c0 [xfs]
> down+0x3b/0x50
> xfs_buf_lock+0x34/0xf0 [xfs]
> xfs_buf_find+0x215/0x6c0 [xfs]
> xfs_buf_get_map+0x37/0x230 [xfs]
> xfs_buf_read_map+0x29/0x190 [xfs]
> xfs_trans_read_buf_map+0x13d/0x520 [xfs]
> xfs_read_agi+0xa8/0x160 [xfs]
> xfs_iunlink_remove+0x6f/0x2a0 [xfs]
> ? current_time+0x46/0x80
> ? xfs_trans_ichgtime+0x39/0xb0 [xfs]
> xfs_rename+0x57a/0xae0 [xfs]
> xfs_vn_rename+0xe4/0x150 [xfs]
> ...
>
>In this patch we move the xfs_iunlink_remove() call to
>before acquiring the AGF lock to preserve correct AGI/AGF locking
>order.
>
>Signed-off-by: kaixuxia <kaixuxia@tencent.com>
>Reviewed-by: Brian Foster <bfoster@redhat.com>
>Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
>Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>
>Minor massage required to backport to apply due to removal of
>out_bmap_cancel: error path label upstream as a result of code
>rework. Only change was to the last code block removed by the
>patch. Functionally equivalent to upstream.
This patch also needs to be backported to 4.19. I can look into that
tomorrow unless you can send a patch sooner :)
--
Thanks,
Sasha
next prev parent reply other threads:[~2020-04-25 1:49 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-24 23:05 [PATCH STABLE v4.14.y 0/2] xfs: Backport two fixes Suraj Jitindar Singh
2020-04-24 23:05 ` [PATCH STABLE v4.14.y 1/2] xfs: validate sb_logsunit is a multiple of the fs blocksize Suraj Jitindar Singh
2020-04-24 23:05 ` [PATCH STABLE v4.14.y 2/2] xfs: Fix deadlock between AGI and AGF with RENAME_WHITEOUT Suraj Jitindar Singh
2020-04-25 1:49 ` Sasha Levin [this message]
2020-04-27 23:24 ` [PATCH STABLE 4.19.y] " Suraj Jitindar Singh
2020-04-28 9:02 ` [PATCH STABLE v4.14.y 0/2] xfs: Backport two fixes Greg KH
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200425014939.GE13035@sasha-vm \
--to=sashal@kernel.org \
--cc=darrick.wong@oracle.com \
--cc=kaixuxia@tencent.com \
--cc=linux-xfs@vger.kernel.org \
--cc=sjitindarsingh@gmail.com \
--cc=stable@vger.kernel.org \
--cc=surajjs@amazon.com \
--cc=xiakaixu1987@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).