On Thu, Feb 11, 2021 at 09:39:11AM -0500, Brian Foster wrote:
> 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@xxxxxxxxxx>
Seems like a lot of cleanup for the amount of actual code change; you
might've sent them separately but I'll merge it anyway...
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> ---
> 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 c4d7a9241dc3..b56ff451adce 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.26.2
>
