Re: [PATCH, realtime-rmap branch] xfs: lock the RT bitmap inode in xfs_efi_item_recover

["Darrick J. Wong" <djwong@xxxxxxxxxx>]

On Tue, Oct 31, 2023 at 10:50:38AM +0100, Christoph Hellwig wrote:
> xfs_trans_free_extent expects the rtbitmap and rtsum inodes to be locked.
> Ensure that is the case during log recovery as well.
> 
> Fixes: 3ea32f5cc5f9 ("xfs: support logging EFIs for realtime extents")
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> 
> I found this when testing the realtime-rmap branch with additional
> patches on top.  Probably makes sense to just fold it in for the next
> rebase of that branch.
> 
>  fs/xfs/xfs_extfree_item.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> index 5b3e7dca4e1ba0..070070b6401d66 100644
> --- a/fs/xfs/xfs_extfree_item.c
> +++ b/fs/xfs/xfs_extfree_item.c
> @@ -768,6 +768,8 @@ xfs_efi_item_recover(
>  
>  		if (!requeue_only) {
>  			xfs_extent_free_get_group(mp, &fake);
> +			if (xfs_efi_is_realtime(&fake))
> +				xfs_rtbitmap_lock(tp, mp);

Curious, I thought that patch "xfs: support logging EFIs for realtime
extents" already locked the rt bitmap?

	resv = xlog_recover_resv(&M_RES(mp)->tr_itruncate);
	error = xfs_trans_alloc(mp, &resv, 0, 0, 0, &tp);
	if (error)
		return error;
	efdp = xfs_trans_get_efd(tp, efip, efip->efi_format.efi_nextents);

	/* Lock the rt bitmap if we've any realtime extents to free. */
	for (i = 0; i < efip->efi_format.efi_nextents; i++) {
		struct xfs_extent		*extp;

		extp = &efip->efi_format.efi_extents[i];
		if (extp->ext_len & XFS_EFI_EXTLEN_REALTIME_EXT) {
			xfs_rtbitmap_lock(tp, mp);

Here			^^^^^^^^^

			break;
		}
	}

	for (i = 0; i < efip->efi_format.efi_nextents; i++) {
		struct xfs_extent_free_item	fake = {
			.xefi_owner		= XFS_RMAP_OWN_UNKNOWN,
			.xefi_agresv		= XFS_AG_RESV_NONE,
		};
		struct xfs_extent		*extp;
		unsigned int			len;

		extp = &efip->efi_format.efi_extents[i];

		fake.xefi_startblock = extp->ext_start;
		len = extp->ext_len;
		if (len & XFS_EFI_EXTLEN_REALTIME_EXT) {
			len &= ~XFS_EFI_EXTLEN_REALTIME_EXT;
			fake.xefi_flags |= XFS_EFI_REALTIME;
		}
		fake.xefi_blockcount = len;

		if (!requeue_only) {
			xfs_extent_free_get_group(mp, &fake);
			error = xfs_trans_free_extent(tp, efdp, &fake);
			xfs_extent_free_put_group(&fake);
		}

So that by the time we get to this part of the loop, the rtbitmap will
be locked already?

What sort of error message did you get?

<confused>

--D

>  			error = xfs_trans_free_extent(tp, efdp, &fake);
>  			xfs_extent_free_put_group(&fake);
>  		}
> -- 
> 2.39.2
>