All of lore.kernel.org
 help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Sasha Levin <sashal@kernel.org>,
	Dave Chinner <david@fromorbit.com>,
	"Darrick J . Wong" <djwong@kernel.org>,
	Christoph Hellwig <hch@lst.de>, Brian Foster <bfoster@redhat.com>,
	Christian Brauner <brauner@kernel.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Leah Rumancik <leah.rumancik@gmail.com>,
	Adam Manzanares <a.manzanares@samsung.com>,
	linux-xfs@vger.kernel.org, stable@vger.kernel.org
Subject: [PATCH 5.10 v2 7/8] xfs: consider shutdown in bmapbt cursor delete assert
Date: Mon,  6 Jun 2022 17:32:54 +0300	[thread overview]
Message-ID: <20220606143255.685988-8-amir73il@gmail.com> (raw)
In-Reply-To: <20220606143255.685988-1-amir73il@gmail.com>

From: Brian Foster <bfoster@redhat.com>

commit 1cd738b13ae9b29e03d6149f0246c61f76e81fcf upstream.

The assert in xfs_btree_del_cursor() checks that the bmapbt block
allocation field has been handled correctly before the cursor is
freed. This field is used for accurate calculation of indirect block
reservation requirements (for delayed allocations), for example.
generic/019 reproduces a scenario where this assert fails because
the filesystem has shutdown while in the middle of a bmbt record
insertion. This occurs after a bmbt block has been allocated via the
cursor but before the higher level bmap function (i.e.
xfs_bmap_add_extent_hole_real()) completes and resets the field.

Update the assert to accommodate the transient state if the
filesystem has shutdown. While here, clean up the indentation and
comments in the function.

Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/xfs/libxfs/xfs_btree.c | 33 ++++++++++++---------------------
 1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 2d25bab68764..9f9f9feccbcd 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -353,20 +353,17 @@ xfs_btree_free_block(
  */
 void
 xfs_btree_del_cursor(
-	xfs_btree_cur_t	*cur,		/* btree cursor */
-	int		error)		/* del because of error */
+	struct xfs_btree_cur	*cur,		/* btree cursor */
+	int			error)		/* del because of error */
 {
-	int		i;		/* btree level */
+	int			i;		/* btree level */
 
 	/*
-	 * Clear the buffer pointers, and release the buffers.
-	 * If we're doing this in the face of an error, we
-	 * need to make sure to inspect all of the entries
-	 * in the bc_bufs array for buffers to be unlocked.
-	 * This is because some of the btree code works from
-	 * level n down to 0, and if we get an error along
-	 * the way we won't have initialized all the entries
-	 * down to 0.
+	 * Clear the buffer pointers and release the buffers. If we're doing
+	 * this because of an error, inspect all of the entries in the bc_bufs
+	 * array for buffers to be unlocked. This is because some of the btree
+	 * code works from level n down to 0, and if we get an error along the
+	 * way we won't have initialized all the entries down to 0.
 	 */
 	for (i = 0; i < cur->bc_nlevels; i++) {
 		if (cur->bc_bufs[i])
@@ -374,17 +371,11 @@ xfs_btree_del_cursor(
 		else if (!error)
 			break;
 	}
-	/*
-	 * Can't free a bmap cursor without having dealt with the
-	 * allocated indirect blocks' accounting.
-	 */
-	ASSERT(cur->bc_btnum != XFS_BTNUM_BMAP ||
-	       cur->bc_ino.allocated == 0);
-	/*
-	 * Free the cursor.
-	 */
+
+	ASSERT(cur->bc_btnum != XFS_BTNUM_BMAP || cur->bc_ino.allocated == 0 ||
+	       XFS_FORCED_SHUTDOWN(cur->bc_mp));
 	if (unlikely(cur->bc_flags & XFS_BTREE_STAGING))
-		kmem_free((void *)cur->bc_ops);
+		kmem_free(cur->bc_ops);
 	kmem_cache_free(xfs_btree_cur_zone, cur);
 }
 
-- 
2.25.1


  parent reply	other threads:[~2022-06-06 14:33 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-06 14:32 [PATCH 5.10 v2 0/8] xfs fixes for 5.10.y (part 2) Amir Goldstein
2022-06-06 14:32 ` [PATCH 5.10 v2 1/8] xfs: set inode size after creating symlink Amir Goldstein
2022-06-06 14:32 ` [PATCH 5.10 v2 2/8] xfs: sync lazy sb accounting on quiesce of read-only mounts Amir Goldstein
2022-06-06 14:32 ` [PATCH 5.10 v2 3/8] xfs: fix chown leaking delalloc quota blocks when fssetxattr fails Amir Goldstein
2022-06-06 14:32 ` [PATCH 5.10 v2 4/8] xfs: fix incorrect root dquot corruption error when switching group/project quota types Amir Goldstein
2022-06-06 14:32 ` [PATCH 5.10 v2 5/8] xfs: restore shutdown check in mapped write fault path Amir Goldstein
2022-06-06 14:32 ` [PATCH 5.10 v2 6/8] xfs: force log and push AIL to clear pinned inodes when aborting mount Amir Goldstein
2022-06-06 14:32 ` Amir Goldstein [this message]
2022-06-06 14:32 ` [PATCH 5.10 v2 8/8] xfs: assert in xfs_btree_del_cursor should take into account error Amir Goldstein
2022-06-06 21:30   ` Dave Chinner
2022-06-06 22:33     ` Amir Goldstein
2022-06-07  3:01       ` Dave Chinner
2022-06-07  4:47         ` Greg Kroah-Hartman
2022-06-07  4:56           ` Amir Goldstein
2022-06-07  6:09         ` Amir Goldstein
2022-06-07 18:35           ` Darrick J. Wong
2022-06-07 19:12             ` Luis Chamberlain
2022-06-08  4:37             ` XFS stable maintenance (Was: Re: [PATCH 5.10 v2 8/8] xfs: assert in xfs_btree_del_cursor should take into account error) Amir Goldstein
2022-06-06 17:01 ` [PATCH 5.10 v2 0/8] xfs fixes for 5.10.y (part 2) Greg Kroah-Hartman

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=20220606143255.685988-8-amir73il@gmail.com \
    --to=amir73il@gmail.com \
    --cc=a.manzanares@samsung.com \
    --cc=bfoster@redhat.com \
    --cc=brauner@kernel.org \
    --cc=david@fromorbit.com \
    --cc=djwong@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@lst.de \
    --cc=leah.rumancik@gmail.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=sashal@kernel.org \
    --cc=stable@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.