diff mbox series

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

Message ID 162007768898.836421.2999725068692265955.stgit@magnolia (mailing list archive)
State Accepted
Headers show
Series xfs: realtime allocator fixes | expand

Commit Message

Darrick J. Wong May 3, 2021, 9:34 p.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

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@kernel.org>
---
 fs/xfs/xfs_bmap_util.c |   81 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 56 insertions(+), 25 deletions(-)

Comments

Allison Henderson May 6, 2021, 5:26 a.m. UTC | #1 
On 5/3/21 2:34 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> 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@kernel.org>
Ok, makes sense.  Thanks for the comments
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>   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 */
>
diff mbox series

Patch

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 */