diff mbox series

[2/2] xfs: don't check for AG deadlock for realtime files in bunmapi

Message ID 89ad24852d1d14fcf834a9551fec503c24d31b44.1574799066.git.osandov@fb.com (mailing list archive)
State Accepted
Headers show
Series xfs: fixes for realtime file truncation | expand

Commit Message

Omar Sandoval Nov. 26, 2019, 8:13 p.m. UTC 
From: Omar Sandoval <osandov@fb.com>

Commit 5b094d6dac04 ("xfs: fix multi-AG deadlock in xfs_bunmapi") added
a check in __xfs_bunmapi() to stop early if we would touch multiple AGs
in the wrong order. However, this check isn't applicable for realtime
files. In most cases, it just makes us do unnecessary commits. However,
without the fix from the previous commit ("xfs: fix realtime file data
space leak"), if the last and second-to-last extents also happen to have
different "AG numbers", then the break actually causes __xfs_bunmapi()
to return without making any progress, which sends
xfs_itruncate_extents_flags() into an infinite loop.

Fixes: 5b094d6dac04 ("xfs: fix multi-AG deadlock in xfs_bunmapi")
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Darrick J. Wong Nov. 27, 2019, 12:36 a.m. UTC | #1 
On Tue, Nov 26, 2019 at 12:13:29PM -0800, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> Commit 5b094d6dac04 ("xfs: fix multi-AG deadlock in xfs_bunmapi") added
> a check in __xfs_bunmapi() to stop early if we would touch multiple AGs
> in the wrong order. However, this check isn't applicable for realtime
> files. In most cases, it just makes us do unnecessary commits. However,
> without the fix from the previous commit ("xfs: fix realtime file data
> space leak"), if the last and second-to-last extents also happen to have
> different "AG numbers", then the break actually causes __xfs_bunmapi()
> to return without making any progress, which sends
> xfs_itruncate_extents_flags() into an infinite loop.
> 
> Fixes: 5b094d6dac04 ("xfs: fix multi-AG deadlock in xfs_bunmapi")
> Signed-off-by: Omar Sandoval <osandov@fb.com>

Looks pretty straightforward,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/libxfs/xfs_bmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 6f8791a1e460..a11b6e7cb35f 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -5300,7 +5300,7 @@ __xfs_bunmapi(
>  		 * Make sure we don't touch multiple AGF headers out of order
>  		 * in a single transaction, as that could cause AB-BA deadlocks.
>  		 */
> -		if (!wasdel) {
> +		if (!wasdel && !isrt) {
>  			agno = XFS_FSB_TO_AGNO(mp, del.br_startblock);
>  			if (prev_agno != NULLAGNUMBER && prev_agno > agno)
>  				break;
> -- 
> 2.24.0
>
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 6f8791a1e460..a11b6e7cb35f 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5300,7 +5300,7 @@  __xfs_bunmapi(
 		 * Make sure we don't touch multiple AGF headers out of order
 		 * in a single transaction, as that could cause AB-BA deadlocks.
 		 */
-		if (!wasdel) {
+		if (!wasdel && !isrt) {
 			agno = XFS_FSB_TO_AGNO(mp, del.br_startblock);
 			if (prev_agno != NULLAGNUMBER && prev_agno > agno)
 				break;