Re: [PATCH 1/2] xfs: adjust rt allocation minlen when extszhint > rtextsize

[Allison Henderson <allison.henderson@xxxxxxxxxx>]

On 5/3/21 2:34 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> xfs_bmap_rtalloc doesn't handle realtime extent files with extent size
> hints larger than the rt volume's extent size properly, because
> xfs_bmap_extsize_align can adjust the offset/length parameters to try to
> fit the extent size hint.
>
> Under these conditions, minlen has to be large enough so that any
> allocation returned by xfs_rtallocate_extent will be large enough to
> cover at least one of the blocks that the caller asked for.  If the
> allocation is too short, bmapi_write will return no mapping for the
> requested range, which causes ENOSPC errors in other parts of the
> filesystem.
>
> Therefore, adjust minlen upwards to fix this.  This can be found by
> running generic/263 (g/127 or g/522) with a realtime extent size hint
> that's larger than the rt volume extent size.
>
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
Ok, makes sense.  Thanks for the comments
Reviewed-by: Allison Henderson <allison.henderson@xxxxxxxxxx>

> ---
>   fs/xfs/xfs_bmap_util.c |   81 +++++++++++++++++++++++++++++++++---------------
>   1 file changed, 56 insertions(+), 25 deletions(-)
>
>
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index a5e9d7d34023..c9381bf4f04b 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -71,18 +71,23 @@ xfs_zero_extent(
>   #ifdef CONFIG_XFS_RT
>   int
>   xfs_bmap_rtalloc(
> -	struct xfs_bmalloca	*ap)	/* bmap alloc argument struct */
> +	struct xfs_bmalloca	*ap)
>   {
> -	int		error;		/* error return value */
> -	xfs_mount_t	*mp;		/* mount point structure */
> -	xfs_extlen_t	prod = 0;	/* product factor for allocators */
> -	xfs_extlen_t	mod = 0;	/* product factor for allocators */
> -	xfs_extlen_t	ralen = 0;	/* realtime allocation length */
> -	xfs_extlen_t	align;		/* minimum allocation alignment */
> -	xfs_rtblock_t	rtb;
> +	struct xfs_mount	*mp = ap->ip->i_mount;
> +	xfs_fileoff_t		orig_offset = ap->offset;
> +	xfs_rtblock_t		rtb;
> +	xfs_extlen_t		prod = 0;  /* product factor for allocators */
> +	xfs_extlen_t		mod = 0;   /* product factor for allocators */
> +	xfs_extlen_t		ralen = 0; /* realtime allocation length */
> +	xfs_extlen_t		align;     /* minimum allocation alignment */
> +	xfs_extlen_t		orig_length = ap->length;
> +	xfs_extlen_t		minlen = mp->m_sb.sb_rextsize;
> +	xfs_extlen_t		raminlen;
> +	bool			rtlocked = false;
> +	int			error;
>   
> -	mp = ap->ip->i_mount;
>   	align = xfs_get_extsz_hint(ap->ip);
> +retry:
>   	prod = align / mp->m_sb.sb_rextsize;
>   	error = xfs_bmap_extsize_align(mp, &ap->got, &ap->prev,
>   					align, 1, ap->eof, 0,
> @@ -92,6 +97,15 @@ xfs_bmap_rtalloc(
>   	ASSERT(ap->length);
>   	ASSERT(ap->length % mp->m_sb.sb_rextsize == 0);
>   
> +	/*
> +	 * If we shifted the file offset downward to satisfy an extent size
> +	 * hint, increase minlen by that amount so that the allocator won't
> +	 * give us an allocation that's too short to cover at least one of the
> +	 * blocks that the caller asked for.
> +	 */
> +	if (ap->offset != orig_offset)
> +		minlen += orig_offset - ap->offset;
> +
>   	/*
>   	 * If the offset & length are not perfectly aligned
>   	 * then kill prod, it will just get us in trouble.
> @@ -116,10 +130,13 @@ xfs_bmap_rtalloc(
>   	/*
>   	 * Lock out modifications to both the RT bitmap and summary inodes
>   	 */
> -	xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL|XFS_ILOCK_RTBITMAP);
> -	xfs_trans_ijoin(ap->tp, mp->m_rbmip, XFS_ILOCK_EXCL);
> -	xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL|XFS_ILOCK_RTSUM);
> -	xfs_trans_ijoin(ap->tp, mp->m_rsumip, XFS_ILOCK_EXCL);
> +	if (!rtlocked) {
> +		xfs_ilock(mp->m_rbmip, XFS_ILOCK_EXCL|XFS_ILOCK_RTBITMAP);
> +		xfs_trans_ijoin(ap->tp, mp->m_rbmip, XFS_ILOCK_EXCL);
> +		xfs_ilock(mp->m_rsumip, XFS_ILOCK_EXCL|XFS_ILOCK_RTSUM);
> +		xfs_trans_ijoin(ap->tp, mp->m_rsumip, XFS_ILOCK_EXCL);
> +		rtlocked = true;
> +	}
>   
>   	/*
>   	 * If it's an allocation to an empty file at offset 0,
> @@ -144,30 +161,44 @@ xfs_bmap_rtalloc(
>   	do_div(ap->blkno, mp->m_sb.sb_rextsize);
>   	rtb = ap->blkno;
>   	ap->length = ralen;
> -	error = xfs_rtallocate_extent(ap->tp, ap->blkno, 1, ap->length,
> -				&ralen, ap->wasdel, prod, &rtb);
> +	raminlen = max_t(xfs_extlen_t, 1, minlen / mp->m_sb.sb_rextsize);
> +	error = xfs_rtallocate_extent(ap->tp, ap->blkno, raminlen, ap->length,
> +			&ralen, ap->wasdel, prod, &rtb);
>   	if (error)
>   		return error;
>   
> -	ap->blkno = rtb;
> -	if (ap->blkno != NULLFSBLOCK) {
> -		ap->blkno *= mp->m_sb.sb_rextsize;
> -		ralen *= mp->m_sb.sb_rextsize;
> -		ap->length = ralen;
> -		ap->ip->i_nblocks += ralen;
> +	if (rtb != NULLRTBLOCK) {
> +		ap->blkno = rtb * mp->m_sb.sb_rextsize;
> +		ap->length = ralen * mp->m_sb.sb_rextsize;
> +		ap->ip->i_nblocks += ap->length;
>   		xfs_trans_log_inode(ap->tp, ap->ip, XFS_ILOG_CORE);
>   		if (ap->wasdel)
> -			ap->ip->i_delayed_blks -= ralen;
> +			ap->ip->i_delayed_blks -= ap->length;
>   		/*
>   		 * Adjust the disk quota also. This was reserved
>   		 * earlier.
>   		 */
>   		xfs_trans_mod_dquot_byino(ap->tp, ap->ip,
>   			ap->wasdel ? XFS_TRANS_DQ_DELRTBCOUNT :
> -					XFS_TRANS_DQ_RTBCOUNT, (long) ralen);
> -	} else {
> -		ap->length = 0;
> +					XFS_TRANS_DQ_RTBCOUNT, ap->length);
> +		return 0;
>   	}
> +
> +	if (align > mp->m_sb.sb_rextsize) {
> +		/*
> +		 * We previously enlarged the request length to try to satisfy
> +		 * an extent size hint.  The allocator didn't return anything,
> +		 * so reset the parameters to the original values and try again
> +		 * without alignment criteria.
> +		 */
> +		ap->offset = orig_offset;
> +		ap->length = orig_length;
> +		minlen = align = mp->m_sb.sb_rextsize;
> +		goto retry;
> +	}
> +
> +	ap->blkno = NULLFSBLOCK;
> +	ap->length = 0;
>   	return 0;
>   }
>   #endif /* CONFIG_XFS_RT */