public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 50/50] xfs: fix low space alloc deadlock
Date: Sat, 11 Jun 2022 11:26:59 +1000	[thread overview]
Message-ID: <20220611012659.3418072-51-david@fromorbit.com> (raw)
In-Reply-To: <20220611012659.3418072-1-david@fromorbit.com>

From: Dave Chinner <dchinner@redhat.com>

This is fixing a g/476 AGF ABBA deadlock that occurs reliably
with this patchset so far. Allocation is attempted in AG 2, which
has no space, then AG 3 which fails with "nominleft" search error.
THis then returns to the caller with AGF 3 still locked, and the
caller then does a retry with more restricted allocation criteria.
This then starts again at AG 2, which then deadlocks because it's
the wrong AGF locking order.

What I can't work out is how the existing TOT code isn't hitting
this every time g/476 is run. I can't see where it unlocks AGF 3,
nor can I see how it avoids the out of order locking on nospace
retries. So this looks like a pre-existing bug, but it takes this
allocator rework to expose it?

More work investigation needed here.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/libxfs/xfs_alloc.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index abf78453d155..c0af59e5e935 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -3332,8 +3332,27 @@ xfs_alloc_vextent_iterate_ags(
 		args->pag = NULL;
 		return error;
 	}
-	if (args->agbp)
+	if (args->agbp) {
+		/*
+		 * XXX: this looks like a pre-existing bug. alloc_size fails
+		 * because of nominleft, and we return here with the AGF locked
+		 * with args->agbno == NULLAGBLOCK If this happens with any AG
+		 * higher than the start_agno, if the caller then tries to allocate
+		 * again with more restricted parameters, we try locking from
+		 * start_agno again and we deadlock because we've already got a
+		 * higher AGF locked. Hence we need to drop the AGF lock if
+		 * we failed to allocate here. g/476 triggers this reliably.
+		 */
+		if (args->agbno == NULLAGBLOCK) {
+			/*
+			 * XXX: This is not a reliable workaround if the AGF was
+			 * modified!
+			 */
+			xfs_trans_brelse(args->tp, args->agbp);
+			args->agbp = NULL;
+		}
 		return 0;
+	}
 
 	/*
 	 * We didn't find an AG we can alloation from. If we were given
-- 
2.35.1


  parent reply	other threads:[~2022-06-11  1:27 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-11  1:26 [RFC] [PATCH 00/50] xfs: per-ag centric allocation alogrithms Dave Chinner
2022-06-11  1:26 ` [PATCH 01/50] xfs: make last AG grow/shrink perag centric Dave Chinner
2022-06-16  7:30   ` Christoph Hellwig
2022-06-11  1:26 ` [PATCH 02/50] xfs: kill xfs_ialloc_pagi_init() Dave Chinner
2022-06-16  7:32   ` Christoph Hellwig
2022-06-11  1:26 ` [PATCH 03/50] xfs: pass perag to xfs_ialloc_read_agi() Dave Chinner
2022-06-16  7:34   ` Christoph Hellwig
2022-06-11  1:26 ` [PATCH 04/50] xfs: kill xfs_alloc_pagf_init() Dave Chinner
2022-06-16  7:35   ` Christoph Hellwig
2022-06-11  1:26 ` [PATCH 05/50] xfs: pass perag to xfs_alloc_read_agf() Dave Chinner
2022-06-11  2:37   ` kernel test robot
2022-06-11 12:04   ` kernel test robot
2022-06-11 13:46   ` kernel test robot
2022-06-14 12:17   ` kernel test robot
2022-06-16  7:38   ` Christoph Hellwig
2022-06-11  1:26 ` [PATCH 06/50] xfs: pass perag to xfs_read_agi Dave Chinner
2022-06-16  7:39   ` Christoph Hellwig
2022-06-16  7:39   ` Christoph Hellwig
2022-06-11  1:26 ` [PATCH 07/50] xfs: pass perag to xfs_read_agf Dave Chinner
2022-06-16  7:40   ` Christoph Hellwig
2022-06-11  1:26 ` [PATCH 08/50] xfs: pass perag to xfs_alloc_get_freelist Dave Chinner
2022-06-16  7:40   ` Christoph Hellwig
2022-06-11  1:26 ` [PATCH 09/50] xfs: pass perag to xfs_alloc_put_freelist Dave Chinner
2022-06-16  7:40   ` Christoph Hellwig
2022-06-11  1:26 ` [PATCH 10/50] xfs: pass perag to xfs_alloc_read_agfl Dave Chinner
2022-06-16  7:41   ` Christoph Hellwig
2022-06-11  1:26 ` [PATCH 11/50] xfs: Pre-calculate per-AG agbno geometry Dave Chinner
2022-06-11  1:26 ` [PATCH 12/50] xfs: Pre-calculate per-AG agino geometry Dave Chinner
2022-06-11  3:08   ` kernel test robot
2022-06-11  1:26 ` [PATCH 13/50] xfs: replace xfs_ag_block_count() with perag accesses Dave Chinner
2022-06-11  1:26 ` [PATCH 14/50] xfs: make is_log_ag() a first class helper Dave Chinner
2022-06-11  1:26 ` [PATCH 15/50] xfs: active perag reference counting Dave Chinner
2022-06-11  1:26 ` [PATCH 16/50] xfs: rework the perag trace points to be perag centric Dave Chinner
2022-06-11  1:26 ` [PATCH 17/50] xfs: convert xfs_imap() to take a perag Dave Chinner
2022-06-11  1:26 ` [PATCH 18/50] xfs: use active perag references for inode allocation Dave Chinner
2022-06-11  1:26 ` [PATCH 19/50] xfs: inobt can use perags in many more places than it does Dave Chinner
2022-06-11  1:26 ` [PATCH 20/50] xfs: convert xfs_ialloc_next_ag() to an atomic Dave Chinner
2022-06-11  1:26 ` [PATCH 21/50] xfs: perags need atomic operational state Dave Chinner
2022-06-11  1:26 ` [PATCH 22/50] xfs: introduce xfs_for_each_perag_wrap() Dave Chinner
2022-06-11  1:26 ` [PATCH 23/50] xfs: rework xfs_alloc_vextent() Dave Chinner
2022-06-11  1:26 ` [PATCH 24/50] xfs: use xfs_alloc_vextent_this_ag() in _iterate_ags() Dave Chinner
2022-06-11  1:26 ` [PATCH 25/50] xfs: combine __xfs_alloc_vextent_this_ag and xfs_alloc_ag_vextent Dave Chinner
2022-06-11  1:26 ` [PATCH 26/50] xfs: use xfs_alloc_vextent_this_ag() where appropriate Dave Chinner
2022-06-11  1:26 ` [PATCH 27/50] xfs: factor xfs_bmap_btalloc() Dave Chinner
2022-06-11  1:26 ` [PATCH 28/50] xfs: use xfs_alloc_vextent_first_ag() where appropriate Dave Chinner
2022-06-11  1:26 ` [PATCH 29/50] xfs: use xfs_alloc_vextent_start_bno() " Dave Chinner
2022-06-11  1:26 ` [PATCH 30/50] xfs: introduce xfs_alloc_vextent_near_bno() Dave Chinner
2022-06-11  1:26 ` [PATCH 31/50] xfs: introduce xfs_alloc_vextent_exact_bno() Dave Chinner
2022-06-11  1:26 ` [PATCH 32/50] xfs: introduce xfs_alloc_vextent_prepare() Dave Chinner
2022-06-11  1:26 ` [PATCH 33/50] xfs: move allocation accounting to xfs_alloc_vextent_set_fsbno() Dave Chinner
2022-06-11  1:26 ` [PATCH 34/50] xfs: fold xfs_alloc_ag_vextent() into callers Dave Chinner
2022-06-11  1:26 ` [PATCH 35/50] xfs: convert xfs_alloc_vextent_iterate_ags() to use perag walker Dave Chinner
2022-06-11  1:26 ` [PATCH 36/50] xfs: convert trim to use for_each_perag_range Dave Chinner
2022-06-11  1:26 ` [PATCH 37/50] xfs: factor out filestreams from xfs_bmap_btalloc_nullfb Dave Chinner
2022-06-11  1:26 ` [PATCH 38/50] xfs: get rid of notinit from xfs_bmap_longest_free_extent Dave Chinner
2022-06-11  1:26 ` [PATCH 39/50] xfs: use xfs_bmap_longest_free_extent() in filestreams Dave Chinner
2022-06-11  1:26 ` [PATCH 40/50] xfs: move xfs_bmap_btalloc_filestreams() to xfs_filestreams.c Dave Chinner
2022-06-11  1:26 ` [PATCH 41/50] xfs: merge filestream AG lookup into xfs_filestream_select_ag() Dave Chinner
2022-06-11  1:26 ` [PATCH 42/50] xfs: merge new filestream AG selection " Dave Chinner
2022-06-11  1:26 ` [PATCH 43/50] xfs: remove xfs_filestream_select_ag() longest extent check Dave Chinner
2022-06-11  1:26 ` [PATCH 44/50] xfs: factor out MRU hit case in xfs_filestream_select_ag Dave Chinner
2022-06-11  1:26 ` [PATCH 45/50] xfs: track an active perag reference in filestreams Dave Chinner
2022-06-11  1:26 ` [PATCH 46/50] xfs: use for_each_perag_wrap in xfs_filestream_pick_ag Dave Chinner
2022-06-11  1:26 ` [PATCH 47/50] xfs: pass perag to filestreams tracing Dave Chinner
2022-06-11  1:26 ` [PATCH 48/50] xfs: return a referenced perag from filestreams allocator Dave Chinner
2022-06-11  1:26 ` [PATCH 49/50] xfs: refactor the filestreams allocator pick functions Dave Chinner
2022-06-11  1:26 ` Dave Chinner [this message]
2022-06-16 12:01 ` [RFC] [PATCH 00/50] xfs: per-ag centric allocation alogrithms Christoph Hellwig
2022-06-21  2:08   ` Dave Chinner

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=20220611012659.3418072-51-david@fromorbit.com \
    --to=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    /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