two small allocator cleanups

two small allocator cleanups

[Christoph Hellwig]

Simplify the high-level RT allocator a bit, and drop two unused
allocation types.

[PATCH 1/2] xfs: simplify xfs_rtallocate_extent

[Christoph Hellwig]

We can deduce the allocation type from the bno argument, and do the
return without prod much simpler internally.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/xfs_bmap_util.c | 13 ++++---------
 fs/xfs/xfs_rtalloc.c   | 27 ++++++++-------------------
 fs/xfs/xfs_rtalloc.h   |  1 -
 3 files changed, 12 insertions(+), 29 deletions(-)

diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index 7c3bfafffba8..c85dafed13c1 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -88,7 +88,6 @@ int
 xfs_bmap_rtalloc(
 	struct xfs_bmalloca	*ap)	/* bmap alloc argument struct */
 {
-	xfs_alloctype_t	atype = 0;	/* type for allocation routines */
 	int		error;		/* error return value */
 	xfs_mount_t	*mp;		/* mount point structure */
 	xfs_extlen_t	prod = 0;	/* product factor for allocators */
@@ -155,18 +154,14 @@ xfs_bmap_rtalloc(
 	/*
 	 * Realtime allocation, done through xfs_rtallocate_extent.
 	 */
-	atype = ap->blkno == 0 ?  XFS_ALLOCTYPE_ANY_AG : XFS_ALLOCTYPE_NEAR_BNO;
 	do_div(ap->blkno, mp->m_sb.sb_rextsize);
 	rtb = ap->blkno;
 	ap->length = ralen;
-	if ((error = xfs_rtallocate_extent(ap->tp, ap->blkno, 1, ap->length,
-				&ralen, atype, ap->wasdel, prod, &rtb)))
-		return error;
-	if (rtb == NULLFSBLOCK && prod > 1 &&
-	    (error = xfs_rtallocate_extent(ap->tp, ap->blkno, 1,
-					   ap->length, &ralen, atype,
-					   ap->wasdel, 1, &rtb)))
+	error = xfs_rtallocate_extent(ap->tp, ap->blkno, 1, 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;
diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index 802bcc326d9f..0b04846e3f60 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -1093,7 +1093,6 @@ xfs_rtallocate_extent(
 	xfs_extlen_t	minlen,		/* minimum length to allocate */
 	xfs_extlen_t	maxlen,		/* maximum length to allocate */
 	xfs_extlen_t	*len,		/* out: actual length allocated */
-	xfs_alloctype_t	type,		/* allocation type XFS_ALLOCTYPE... */
 	int		wasdel,		/* was a delayed allocation extent */
 	xfs_extlen_t	prod,		/* extent product factor */
 	xfs_rtblock_t	*rtblock)	/* out: start block allocated */
@@ -1106,7 +1105,7 @@ xfs_rtallocate_extent(
 
 	ASSERT(xfs_isilocked(mp->m_rbmip, XFS_ILOCK_EXCL));
 	ASSERT(minlen > 0 && minlen <= maxlen);
-
+	
 	/*
 	 * If prod is set then figure out what to do to minlen and maxlen.
 	 */
@@ -1123,29 +1122,15 @@ xfs_rtallocate_extent(
 		}
 	}
 
+retry:
 	sumbp = NULL;
-	/*
-	 * Allocate by size, or near another block, or exactly at some block.
-	 */
-	switch (type) {
-	case XFS_ALLOCTYPE_ANY_AG:
+	if (bno == 0) {
 		error = xfs_rtallocate_extent_size(mp, tp, minlen, maxlen, len,
 				&sumbp,	&sb, prod, &r);
-		break;
-	case XFS_ALLOCTYPE_NEAR_BNO:
+	} else {
 		error = xfs_rtallocate_extent_near(mp, tp, bno, minlen, maxlen,
 				len, &sumbp, &sb, prod, &r);
-		break;
-	case XFS_ALLOCTYPE_THIS_BNO:
-		error = xfs_rtallocate_extent_exact(mp, tp, bno, minlen, maxlen,
-				len, &sumbp, &sb, prod, &r);
-		break;
-	default:
-		error = -EIO;
-		ASSERT(0);
 	}
-	if (error)
-		return error;
 
 	/*
 	 * If it worked, update the superblock.
@@ -1158,7 +1143,11 @@ xfs_rtallocate_extent(
 			xfs_trans_mod_sb(tp, XFS_TRANS_SB_RES_FREXTENTS, -slen);
 		else
 			xfs_trans_mod_sb(tp, XFS_TRANS_SB_FREXTENTS, -slen);
+	} else if (prod > 1) {
+		prod = 1;
+		goto retry;
 	}
+
 	*rtblock = r;
 	return 0;
 }
diff --git a/fs/xfs/xfs_rtalloc.h b/fs/xfs/xfs_rtalloc.h
index 355dd9e1cb64..18c8e821a0e4 100644
--- a/fs/xfs/xfs_rtalloc.h
+++ b/fs/xfs/xfs_rtalloc.h
@@ -40,7 +40,6 @@ xfs_rtallocate_extent(
 	xfs_extlen_t		minlen,	/* minimum length to allocate */
 	xfs_extlen_t		maxlen,	/* maximum length to allocate */
 	xfs_extlen_t		*len,	/* out: actual length allocated */
-	xfs_alloctype_t		type,	/* allocation type XFS_ALLOCTYPE... */
 	int			wasdel,	/* was a delayed allocation extent */
 	xfs_extlen_t		prod,	/* extent product factor */
 	xfs_rtblock_t		*rtblock); /* out: start block allocated */
-- 
2.11.0

[PATCH 2/2] xfs: remove XFS_ALLOCTYPE_ANY_AG and XFS_ALLOCTYPE_START_AG

[Christoph Hellwig]

XFS_ALLOCTYPE_ANY_AG  was only used for the RT allocator and is unused now,
and XFS_ALLOCTYPE_START_AG has been unused for a while.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_alloc.c | 16 ++--------------
 fs/xfs/libxfs/xfs_alloc.h |  4 ----
 2 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index fe98fbc4adf1..369adcc18c02 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -2664,21 +2664,11 @@ xfs_alloc_vextent(
 		args->agbno = XFS_FSB_TO_AGBNO(mp, args->fsbno);
 		args->type = XFS_ALLOCTYPE_NEAR_BNO;
 		/* FALLTHROUGH */
-	case XFS_ALLOCTYPE_ANY_AG:
-	case XFS_ALLOCTYPE_START_AG:
 	case XFS_ALLOCTYPE_FIRST_AG:
 		/*
 		 * Rotate through the allocation groups looking for a winner.
 		 */
-		if (type == XFS_ALLOCTYPE_ANY_AG) {
-			/*
-			 * Start with the last place we left off.
-			 */
-			args->agno = sagno = (mp->m_agfrotor / rotorstep) %
-					mp->m_sb.sb_agcount;
-			args->type = XFS_ALLOCTYPE_THIS_AG;
-			flags = XFS_ALLOC_FLAG_TRYLOCK;
-		} else if (type == XFS_ALLOCTYPE_FIRST_AG) {
+		if (type == XFS_ALLOCTYPE_FIRST_AG) {
 			/*
 			 * Start with allocation group given by bno.
 			 */
@@ -2687,8 +2677,6 @@ xfs_alloc_vextent(
 			sagno = 0;
 			flags = 0;
 		} else {
-			if (type == XFS_ALLOCTYPE_START_AG)
-				args->type = XFS_ALLOCTYPE_THIS_AG;
 			/*
 			 * Start with the given allocation group.
 			 */
@@ -2756,7 +2744,7 @@ xfs_alloc_vextent(
 			}
 			xfs_perag_put(args->pag);
 		}
-		if (bump_rotor || (type == XFS_ALLOCTYPE_ANY_AG)) {
+		if (bump_rotor) {
 			if (args->agno == sagno)
 				mp->m_agfrotor = (mp->m_agfrotor + 1) %
 					(mp->m_sb.sb_agcount * rotorstep);
diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
index 1d0f48a501a3..2a8d0fa6fbbe 100644
--- a/fs/xfs/libxfs/xfs_alloc.h
+++ b/fs/xfs/libxfs/xfs_alloc.h
@@ -29,9 +29,7 @@ extern struct workqueue_struct *xfs_alloc_wq;
 /*
  * Freespace allocation types.  Argument to xfs_alloc_[v]extent.
  */
-#define XFS_ALLOCTYPE_ANY_AG	0x01	/* allocate anywhere, use rotor */
 #define XFS_ALLOCTYPE_FIRST_AG	0x02	/* ... start at ag 0 */
-#define XFS_ALLOCTYPE_START_AG	0x04	/* anywhere, start in this a.g. */
 #define XFS_ALLOCTYPE_THIS_AG	0x08	/* anywhere in this a.g. */
 #define XFS_ALLOCTYPE_START_BNO	0x10	/* near this block else anywhere */
 #define XFS_ALLOCTYPE_NEAR_BNO	0x20	/* in this a.g. and near this block */
@@ -41,9 +39,7 @@ extern struct workqueue_struct *xfs_alloc_wq;
 typedef unsigned int xfs_alloctype_t;
 
 #define XFS_ALLOC_TYPES \
-	{ XFS_ALLOCTYPE_ANY_AG,		"ANY_AG" }, \
 	{ XFS_ALLOCTYPE_FIRST_AG,	"FIRST_AG" }, \
-	{ XFS_ALLOCTYPE_START_AG,	"START_AG" }, \
 	{ XFS_ALLOCTYPE_THIS_AG,	"THIS_AG" }, \
 	{ XFS_ALLOCTYPE_START_BNO,	"START_BNO" }, \
 	{ XFS_ALLOCTYPE_NEAR_BNO,	"NEAR_BNO" }, \
-- 
2.11.0

Re: [PATCH 1/2] xfs: simplify xfs_rtallocate_extent

[Darrick J. Wong]

On Thu, Feb 16, 2017 at 10:21:09PM +0100, Christoph Hellwig wrote:
> We can deduce the allocation type from the bno argument, and do the
> return without prod much simpler internally.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/xfs_bmap_util.c | 13 ++++---------
>  fs/xfs/xfs_rtalloc.c   | 27 ++++++++-------------------
>  fs/xfs/xfs_rtalloc.h   |  1 -
>  3 files changed, 12 insertions(+), 29 deletions(-)
> 
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 7c3bfafffba8..c85dafed13c1 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -88,7 +88,6 @@ int
>  xfs_bmap_rtalloc(
>  	struct xfs_bmalloca	*ap)	/* bmap alloc argument struct */
>  {
> -	xfs_alloctype_t	atype = 0;	/* type for allocation routines */
>  	int		error;		/* error return value */
>  	xfs_mount_t	*mp;		/* mount point structure */
>  	xfs_extlen_t	prod = 0;	/* product factor for allocators */
> @@ -155,18 +154,14 @@ xfs_bmap_rtalloc(
>  	/*
>  	 * Realtime allocation, done through xfs_rtallocate_extent.
>  	 */
> -	atype = ap->blkno == 0 ?  XFS_ALLOCTYPE_ANY_AG : XFS_ALLOCTYPE_NEAR_BNO;
>  	do_div(ap->blkno, mp->m_sb.sb_rextsize);
>  	rtb = ap->blkno;
>  	ap->length = ralen;
> -	if ((error = xfs_rtallocate_extent(ap->tp, ap->blkno, 1, ap->length,
> -				&ralen, atype, ap->wasdel, prod, &rtb)))
> -		return error;
> -	if (rtb == NULLFSBLOCK && prod > 1 &&
> -	    (error = xfs_rtallocate_extent(ap->tp, ap->blkno, 1,
> -					   ap->length, &ralen, atype,
> -					   ap->wasdel, 1, &rtb)))
> +	error = xfs_rtallocate_extent(ap->tp, ap->blkno, 1, 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;
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index 802bcc326d9f..0b04846e3f60 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -1093,7 +1093,6 @@ xfs_rtallocate_extent(
>  	xfs_extlen_t	minlen,		/* minimum length to allocate */
>  	xfs_extlen_t	maxlen,		/* maximum length to allocate */
>  	xfs_extlen_t	*len,		/* out: actual length allocated */
> -	xfs_alloctype_t	type,		/* allocation type XFS_ALLOCTYPE... */
>  	int		wasdel,		/* was a delayed allocation extent */
>  	xfs_extlen_t	prod,		/* extent product factor */
>  	xfs_rtblock_t	*rtblock)	/* out: start block allocated */
> @@ -1106,7 +1105,7 @@ xfs_rtallocate_extent(
>  
>  	ASSERT(xfs_isilocked(mp->m_rbmip, XFS_ILOCK_EXCL));
>  	ASSERT(minlen > 0 && minlen <= maxlen);
> -
> +	

Inserted whitespace?

>  	/*
>  	 * If prod is set then figure out what to do to minlen and maxlen.
>  	 */
> @@ -1123,29 +1122,15 @@ xfs_rtallocate_extent(
>  		}
>  	}
>  
> +retry:
>  	sumbp = NULL;
> -	/*
> -	 * Allocate by size, or near another block, or exactly at some block.
> -	 */
> -	switch (type) {
> -	case XFS_ALLOCTYPE_ANY_AG:
> +	if (bno == 0) {
>  		error = xfs_rtallocate_extent_size(mp, tp, minlen, maxlen, len,
>  				&sumbp,	&sb, prod, &r);
> -		break;
> -	case XFS_ALLOCTYPE_NEAR_BNO:
> +	} else {
>  		error = xfs_rtallocate_extent_near(mp, tp, bno, minlen, maxlen,
>  				len, &sumbp, &sb, prod, &r);
> -		break;
> -	case XFS_ALLOCTYPE_THIS_BNO:
> -		error = xfs_rtallocate_extent_exact(mp, tp, bno, minlen, maxlen,
> -				len, &sumbp, &sb, prod, &r);
> -		break;
> -	default:
> -		error = -EIO;
> -		ASSERT(0);
>  	}
> -	if (error)
> -		return error;

We shouldn't be ignoring errors from xfs_rtallocate_extent_*, right?

--D

>  	/*
>  	 * If it worked, update the superblock.
> @@ -1158,7 +1143,11 @@ xfs_rtallocate_extent(
>  			xfs_trans_mod_sb(tp, XFS_TRANS_SB_RES_FREXTENTS, -slen);
>  		else
>  			xfs_trans_mod_sb(tp, XFS_TRANS_SB_FREXTENTS, -slen);
> +	} else if (prod > 1) {
> +		prod = 1;
> +		goto retry;
>  	}
> +
>  	*rtblock = r;
>  	return 0;
>  }
> diff --git a/fs/xfs/xfs_rtalloc.h b/fs/xfs/xfs_rtalloc.h
> index 355dd9e1cb64..18c8e821a0e4 100644
> --- a/fs/xfs/xfs_rtalloc.h
> +++ b/fs/xfs/xfs_rtalloc.h
> @@ -40,7 +40,6 @@ xfs_rtallocate_extent(
>  	xfs_extlen_t		minlen,	/* minimum length to allocate */
>  	xfs_extlen_t		maxlen,	/* maximum length to allocate */
>  	xfs_extlen_t		*len,	/* out: actual length allocated */
> -	xfs_alloctype_t		type,	/* allocation type XFS_ALLOCTYPE... */
>  	int			wasdel,	/* was a delayed allocation extent */
>  	xfs_extlen_t		prod,	/* extent product factor */
>  	xfs_rtblock_t		*rtblock); /* out: start block allocated */
> -- 
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] xfs: remove XFS_ALLOCTYPE_ANY_AG and XFS_ALLOCTYPE_START_AG

[Darrick J. Wong]

On Thu, Feb 16, 2017 at 10:21:10PM +0100, Christoph Hellwig wrote:
> XFS_ALLOCTYPE_ANY_AG  was only used for the RT allocator and is unused now,
> and XFS_ALLOCTYPE_START_AG has been unused for a while.
> 

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

I'm wondering for sake of historical context, has XFS ever used these?

--D

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_alloc.c | 16 ++--------------
>  fs/xfs/libxfs/xfs_alloc.h |  4 ----
>  2 files changed, 2 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index fe98fbc4adf1..369adcc18c02 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -2664,21 +2664,11 @@ xfs_alloc_vextent(
>  		args->agbno = XFS_FSB_TO_AGBNO(mp, args->fsbno);
>  		args->type = XFS_ALLOCTYPE_NEAR_BNO;
>  		/* FALLTHROUGH */
> -	case XFS_ALLOCTYPE_ANY_AG:
> -	case XFS_ALLOCTYPE_START_AG:
>  	case XFS_ALLOCTYPE_FIRST_AG:
>  		/*
>  		 * Rotate through the allocation groups looking for a winner.
>  		 */
> -		if (type == XFS_ALLOCTYPE_ANY_AG) {
> -			/*
> -			 * Start with the last place we left off.
> -			 */
> -			args->agno = sagno = (mp->m_agfrotor / rotorstep) %
> -					mp->m_sb.sb_agcount;
> -			args->type = XFS_ALLOCTYPE_THIS_AG;
> -			flags = XFS_ALLOC_FLAG_TRYLOCK;
> -		} else if (type == XFS_ALLOCTYPE_FIRST_AG) {
> +		if (type == XFS_ALLOCTYPE_FIRST_AG) {
>  			/*
>  			 * Start with allocation group given by bno.
>  			 */
> @@ -2687,8 +2677,6 @@ xfs_alloc_vextent(
>  			sagno = 0;
>  			flags = 0;
>  		} else {
> -			if (type == XFS_ALLOCTYPE_START_AG)
> -				args->type = XFS_ALLOCTYPE_THIS_AG;
>  			/*
>  			 * Start with the given allocation group.
>  			 */
> @@ -2756,7 +2744,7 @@ xfs_alloc_vextent(
>  			}
>  			xfs_perag_put(args->pag);
>  		}
> -		if (bump_rotor || (type == XFS_ALLOCTYPE_ANY_AG)) {
> +		if (bump_rotor) {
>  			if (args->agno == sagno)
>  				mp->m_agfrotor = (mp->m_agfrotor + 1) %
>  					(mp->m_sb.sb_agcount * rotorstep);
> diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
> index 1d0f48a501a3..2a8d0fa6fbbe 100644
> --- a/fs/xfs/libxfs/xfs_alloc.h
> +++ b/fs/xfs/libxfs/xfs_alloc.h
> @@ -29,9 +29,7 @@ extern struct workqueue_struct *xfs_alloc_wq;
>  /*
>   * Freespace allocation types.  Argument to xfs_alloc_[v]extent.
>   */
> -#define XFS_ALLOCTYPE_ANY_AG	0x01	/* allocate anywhere, use rotor */
>  #define XFS_ALLOCTYPE_FIRST_AG	0x02	/* ... start at ag 0 */
> -#define XFS_ALLOCTYPE_START_AG	0x04	/* anywhere, start in this a.g. */
>  #define XFS_ALLOCTYPE_THIS_AG	0x08	/* anywhere in this a.g. */
>  #define XFS_ALLOCTYPE_START_BNO	0x10	/* near this block else anywhere */
>  #define XFS_ALLOCTYPE_NEAR_BNO	0x20	/* in this a.g. and near this block */
> @@ -41,9 +39,7 @@ extern struct workqueue_struct *xfs_alloc_wq;
>  typedef unsigned int xfs_alloctype_t;
>  
>  #define XFS_ALLOC_TYPES \
> -	{ XFS_ALLOCTYPE_ANY_AG,		"ANY_AG" }, \
>  	{ XFS_ALLOCTYPE_FIRST_AG,	"FIRST_AG" }, \
> -	{ XFS_ALLOCTYPE_START_AG,	"START_AG" }, \
>  	{ XFS_ALLOCTYPE_THIS_AG,	"THIS_AG" }, \
>  	{ XFS_ALLOCTYPE_START_BNO,	"START_BNO" }, \
>  	{ XFS_ALLOCTYPE_NEAR_BNO,	"NEAR_BNO" }, \
> -- 
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] xfs: remove XFS_ALLOCTYPE_ANY_AG and XFS_ALLOCTYPE_START_AG

[Christoph Hellwig]

On Thu, Feb 16, 2017 at 05:05:28PM -0800, Darrick J. Wong wrote:
> On Thu, Feb 16, 2017 at 10:21:10PM +0100, Christoph Hellwig wrote:
> > XFS_ALLOCTYPE_ANY_AG  was only used for the RT allocator and is unused now,
> > and XFS_ALLOCTYPE_START_AG has been unused for a while.
> > 
> 
> Looks ok,
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> I'm wondering for sake of historical context, has XFS ever used these?

All the way back to 1996 XFS_ALLOCTYPE_START_AG seems to have been
unused, and XFS_ALLOCTYPE_ANY_AG seems to only have been used in the
RT allocator.

Re: [PATCH 1/2] xfs: simplify xfs_rtallocate_extent

[Christoph Hellwig]

On Thu, Feb 16, 2017 at 05:02:32PM -0800, Darrick J. Wong wrote:
> >  	ASSERT(xfs_isilocked(mp->m_rbmip, XFS_ILOCK_EXCL));
> >  	ASSERT(minlen > 0 && minlen <= maxlen);
> > -
> > +	
> 
> Inserted whitespace?

Yeah, fixed.

> > -	if (error)
> > -		return error;
> 
> We shouldn't be ignoring errors from xfs_rtallocate_extent_*, right?

No, we shouldn't.  I'll add the check back.