diff mbox series

[v2,6/6] xfs: kill ialloced in xfs_dialloc()

Message ID 20201203161028.1900929-7-hsiangkao@redhat.com (mailing list archive)
State Superseded, archived
Headers show
Series xfs: some xfs_dialloc() cleanup | expand

Commit Message

Gao Xiang Dec. 3, 2020, 4:10 p.m. UTC
It's enough to just use return code, and get rid of an argument.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
---
 fs/xfs/libxfs/xfs_ialloc.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

Comments

Darrick J. Wong Dec. 3, 2020, 7:08 p.m. UTC | #1
On Fri, Dec 04, 2020 at 12:10:28AM +0800, Gao Xiang wrote:
> It's enough to just use return code, and get rid of an argument.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Gao Xiang <hsiangkao@redhat.com>

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

--D

> ---
>  fs/xfs/libxfs/xfs_ialloc.c | 24 +++++++++---------------
>  1 file changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 597629353d4d..ec63afb59156 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -607,13 +607,13 @@ xfs_inobt_insert_sprec(
>  
>  /*
>   * Allocate new inodes in the allocation group specified by agbp.
> - * Return 0 for success, else error code.
> + * Returns 0 if inodes were allocated in this AG; 1 if there was no space
> + * in this AG; or the usual negative error code.
>   */
>  STATIC int
>  xfs_ialloc_ag_alloc(
>  	struct xfs_trans	*tp,
> -	struct xfs_buf		*agbp,
> -	int			*alloc)
> +	struct xfs_buf		*agbp)
>  {
>  	struct xfs_agi		*agi;
>  	struct xfs_alloc_arg	args;
> @@ -795,10 +795,9 @@ xfs_ialloc_ag_alloc(
>  		allocmask = (1 << (newlen / XFS_INODES_PER_HOLEMASK_BIT)) - 1;
>  	}
>  
> -	if (args.fsbno == NULLFSBLOCK) {
> -		*alloc = 0;
> -		return 0;
> -	}
> +	if (args.fsbno == NULLFSBLOCK)
> +		return 1;
> +
>  	ASSERT(args.len == args.minlen);
>  
>  	/*
> @@ -903,7 +902,6 @@ xfs_ialloc_ag_alloc(
>  	 */
>  	xfs_trans_mod_sb(tp, XFS_TRANS_SB_ICOUNT, (long)newlen);
>  	xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, (long)newlen);
> -	*alloc = 1;
>  	return 0;
>  }
>  
> @@ -1749,7 +1747,6 @@ xfs_dialloc_select_ag(
>  	struct xfs_buf		*agbp;
>  	xfs_agnumber_t		agno;
>  	int			error;
> -	int			ialloced;
>  	bool			noroom = false;
>  	xfs_agnumber_t		start_agno;
>  	struct xfs_perag	*pag;
> @@ -1823,17 +1820,14 @@ xfs_dialloc_select_ag(
>  		if (!okalloc)
>  			goto nextag_relse_buffer;
>  
> -
> -		error = xfs_ialloc_ag_alloc(*tpp, agbp, &ialloced);
> -		if (error) {
> +		error = xfs_ialloc_ag_alloc(*tpp, agbp);
> +		if (error < 0) {
>  			xfs_trans_brelse(*tpp, agbp);
>  
>  			if (error == -ENOSPC)
>  				error = 0;
>  			break;
> -		}
> -
> -		if (ialloced) {
> +		} else if (error == 0) {
>  			/*
>  			 * We successfully allocated some inodes, so roll the
>  			 * transaction and return the locked AGI buffer to the
> -- 
> 2.18.4
>
Dave Chinner Dec. 3, 2020, 8:33 p.m. UTC | #2
On Fri, Dec 04, 2020 at 12:10:28AM +0800, Gao Xiang wrote:
> It's enough to just use return code, and get rid of an argument.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_ialloc.c | 24 +++++++++---------------
>  1 file changed, 9 insertions(+), 15 deletions(-)

Look fine.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 597629353d4d..ec63afb59156 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -607,13 +607,13 @@  xfs_inobt_insert_sprec(
 
 /*
  * Allocate new inodes in the allocation group specified by agbp.
- * Return 0 for success, else error code.
+ * Returns 0 if inodes were allocated in this AG; 1 if there was no space
+ * in this AG; or the usual negative error code.
  */
 STATIC int
 xfs_ialloc_ag_alloc(
 	struct xfs_trans	*tp,
-	struct xfs_buf		*agbp,
-	int			*alloc)
+	struct xfs_buf		*agbp)
 {
 	struct xfs_agi		*agi;
 	struct xfs_alloc_arg	args;
@@ -795,10 +795,9 @@  xfs_ialloc_ag_alloc(
 		allocmask = (1 << (newlen / XFS_INODES_PER_HOLEMASK_BIT)) - 1;
 	}
 
-	if (args.fsbno == NULLFSBLOCK) {
-		*alloc = 0;
-		return 0;
-	}
+	if (args.fsbno == NULLFSBLOCK)
+		return 1;
+
 	ASSERT(args.len == args.minlen);
 
 	/*
@@ -903,7 +902,6 @@  xfs_ialloc_ag_alloc(
 	 */
 	xfs_trans_mod_sb(tp, XFS_TRANS_SB_ICOUNT, (long)newlen);
 	xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, (long)newlen);
-	*alloc = 1;
 	return 0;
 }
 
@@ -1749,7 +1747,6 @@  xfs_dialloc_select_ag(
 	struct xfs_buf		*agbp;
 	xfs_agnumber_t		agno;
 	int			error;
-	int			ialloced;
 	bool			noroom = false;
 	xfs_agnumber_t		start_agno;
 	struct xfs_perag	*pag;
@@ -1823,17 +1820,14 @@  xfs_dialloc_select_ag(
 		if (!okalloc)
 			goto nextag_relse_buffer;
 
-
-		error = xfs_ialloc_ag_alloc(*tpp, agbp, &ialloced);
-		if (error) {
+		error = xfs_ialloc_ag_alloc(*tpp, agbp);
+		if (error < 0) {
 			xfs_trans_brelse(*tpp, agbp);
 
 			if (error == -ENOSPC)
 				error = 0;
 			break;
-		}
-
-		if (ialloced) {
+		} else if (error == 0) {
 			/*
 			 * We successfully allocated some inodes, so roll the
 			 * transaction and return the locked AGI buffer to the