All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs: drop the AGI being passed to xfs_check_agi_freecount
@ 2021-06-07  4:15 Dave Chinner
  2021-06-07 10:46 ` Carlos Maiolino
  2021-06-08 16:05 ` Darrick J. Wong
  0 siblings, 2 replies; 3+ messages in thread
From: Dave Chinner @ 2021-06-07  4:15 UTC (permalink / raw)
  To: linux-xfs

From: Dave Chinner <dchinner@redhat.com>

Stephen Rothwell reported this compiler warning from linux-next:

fs/xfs/libxfs/xfs_ialloc.c: In function 'xfs_difree_finobt':
fs/xfs/libxfs/xfs_ialloc.c:2032:20: warning: unused variable 'agi' [-Wunused-variable]
 2032 |  struct xfs_agi   *agi = agbp->b_addr;

Which is fallout from agno -> perag conversions that were done in
this function. xfs_check_agi_freecount() is the only user of "agi"
in xfs_difree_finobt() now, and it only uses the agi to get the
current free inode count. We hold that in the perag structure, so
there's not need to directly reference the raw AGI to get this
information.

The btree cursor being passed to xfs_check_agi_freecount() has a
reference to the perag being operated on, so use that directly in
xfs_check_agi_freecount() rather than passing an AGI.

Fixes: 7b13c5155182 ("xfs: use perag for ialloc btree cursors")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/libxfs/xfs_ialloc.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 2ed6de6faf8a..654a8d9681e1 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -214,10 +214,9 @@ xfs_inobt_insert(
  * Verify that the number of free inodes in the AGI is correct.
  */
 #ifdef DEBUG
-STATIC int
+static int
 xfs_check_agi_freecount(
-	struct xfs_btree_cur	*cur,
-	struct xfs_agi		*agi)
+	struct xfs_btree_cur	*cur)
 {
 	if (cur->bc_nlevels == 1) {
 		xfs_inobt_rec_incore_t rec;
@@ -243,12 +242,12 @@ xfs_check_agi_freecount(
 		} while (i == 1);
 
 		if (!XFS_FORCED_SHUTDOWN(cur->bc_mp))
-			ASSERT(freecount == be32_to_cpu(agi->agi_freecount));
+			ASSERT(freecount == cur->bc_ag.pag->pagi_freecount);
 	}
 	return 0;
 }
 #else
-#define xfs_check_agi_freecount(cur, agi)	0
+#define xfs_check_agi_freecount(cur)	0
 #endif
 
 /*
@@ -1014,7 +1013,7 @@ xfs_dialloc_ag_inobt(
 	if (!pagino)
 		pagino = be32_to_cpu(agi->agi_newino);
 
-	error = xfs_check_agi_freecount(cur, agi);
+	error = xfs_check_agi_freecount(cur);
 	if (error)
 		goto error0;
 
@@ -1234,7 +1233,7 @@ xfs_dialloc_ag_inobt(
 	xfs_ialloc_log_agi(tp, agbp, XFS_AGI_FREECOUNT);
 	pag->pagi_freecount--;
 
-	error = xfs_check_agi_freecount(cur, agi);
+	error = xfs_check_agi_freecount(cur);
 	if (error)
 		goto error0;
 
@@ -1461,7 +1460,7 @@ xfs_dialloc_ag(
 
 	cur = xfs_inobt_init_cursor(mp, tp, agbp, pag, XFS_BTNUM_FINO);
 
-	error = xfs_check_agi_freecount(cur, agi);
+	error = xfs_check_agi_freecount(cur);
 	if (error)
 		goto error_cur;
 
@@ -1504,7 +1503,7 @@ xfs_dialloc_ag(
 	 */
 	icur = xfs_inobt_init_cursor(mp, tp, agbp, pag, XFS_BTNUM_INO);
 
-	error = xfs_check_agi_freecount(icur, agi);
+	error = xfs_check_agi_freecount(icur);
 	if (error)
 		goto error_icur;
 
@@ -1522,10 +1521,10 @@ xfs_dialloc_ag(
 
 	xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, -1);
 
-	error = xfs_check_agi_freecount(icur, agi);
+	error = xfs_check_agi_freecount(icur);
 	if (error)
 		goto error_icur;
-	error = xfs_check_agi_freecount(cur, agi);
+	error = xfs_check_agi_freecount(cur);
 	if (error)
 		goto error_icur;
 
@@ -1911,7 +1910,7 @@ xfs_difree_inobt(
 	 */
 	cur = xfs_inobt_init_cursor(mp, tp, agbp, pag, XFS_BTNUM_INO);
 
-	error = xfs_check_agi_freecount(cur, agi);
+	error = xfs_check_agi_freecount(cur);
 	if (error)
 		goto error0;
 
@@ -2004,7 +2003,7 @@ xfs_difree_inobt(
 		xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, 1);
 	}
 
-	error = xfs_check_agi_freecount(cur, agi);
+	error = xfs_check_agi_freecount(cur);
 	if (error)
 		goto error0;
 
@@ -2029,7 +2028,6 @@ xfs_difree_finobt(
 	xfs_agino_t			agino,
 	struct xfs_inobt_rec_incore	*ibtrec) /* inobt record */
 {
-	struct xfs_agi			*agi = agbp->b_addr;
 	struct xfs_btree_cur		*cur;
 	struct xfs_inobt_rec_incore	rec;
 	int				offset = agino - ibtrec->ir_startino;
@@ -2114,7 +2112,7 @@ xfs_difree_finobt(
 	}
 
 out:
-	error = xfs_check_agi_freecount(cur, agi);
+	error = xfs_check_agi_freecount(cur);
 	if (error)
 		goto error;
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] xfs: drop the AGI being passed to xfs_check_agi_freecount
  2021-06-07  4:15 [PATCH] xfs: drop the AGI being passed to xfs_check_agi_freecount Dave Chinner
@ 2021-06-07 10:46 ` Carlos Maiolino
  2021-06-08 16:05 ` Darrick J. Wong
  1 sibling, 0 replies; 3+ messages in thread
From: Carlos Maiolino @ 2021-06-07 10:46 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-xfs

On Mon, Jun 07, 2021 at 02:15:29PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Stephen Rothwell reported this compiler warning from linux-next:
> 
> fs/xfs/libxfs/xfs_ialloc.c: In function 'xfs_difree_finobt':
> fs/xfs/libxfs/xfs_ialloc.c:2032:20: warning: unused variable 'agi' [-Wunused-variable]
>  2032 |  struct xfs_agi   *agi = agbp->b_addr;
> 
> Which is fallout from agno -> perag conversions that were done in
> this function. xfs_check_agi_freecount() is the only user of "agi"
> in xfs_difree_finobt() now, and it only uses the agi to get the
> current free inode count. We hold that in the perag structure, so
> there's not need to directly reference the raw AGI to get this
> information.
> 
> The btree cursor being passed to xfs_check_agi_freecount() has a
> reference to the perag being operated on, so use that directly in
> xfs_check_agi_freecount() rather than passing an AGI.
> 
> Fixes: 7b13c5155182 ("xfs: use perag for ialloc btree cursors")
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

Looks good:
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  fs/xfs/libxfs/xfs_ialloc.c | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 2ed6de6faf8a..654a8d9681e1 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -214,10 +214,9 @@ xfs_inobt_insert(
>   * Verify that the number of free inodes in the AGI is correct.
>   */
>  #ifdef DEBUG
> -STATIC int
> +static int
>  xfs_check_agi_freecount(
> -	struct xfs_btree_cur	*cur,
> -	struct xfs_agi		*agi)
> +	struct xfs_btree_cur	*cur)
>  {
>  	if (cur->bc_nlevels == 1) {
>  		xfs_inobt_rec_incore_t rec;
> @@ -243,12 +242,12 @@ xfs_check_agi_freecount(
>  		} while (i == 1);
>  
>  		if (!XFS_FORCED_SHUTDOWN(cur->bc_mp))
> -			ASSERT(freecount == be32_to_cpu(agi->agi_freecount));
> +			ASSERT(freecount == cur->bc_ag.pag->pagi_freecount);
>  	}
>  	return 0;
>  }
>  #else
> -#define xfs_check_agi_freecount(cur, agi)	0
> +#define xfs_check_agi_freecount(cur)	0
>  #endif
>  
>  /*
> @@ -1014,7 +1013,7 @@ xfs_dialloc_ag_inobt(
>  	if (!pagino)
>  		pagino = be32_to_cpu(agi->agi_newino);
>  
> -	error = xfs_check_agi_freecount(cur, agi);
> +	error = xfs_check_agi_freecount(cur);
>  	if (error)
>  		goto error0;
>  
> @@ -1234,7 +1233,7 @@ xfs_dialloc_ag_inobt(
>  	xfs_ialloc_log_agi(tp, agbp, XFS_AGI_FREECOUNT);
>  	pag->pagi_freecount--;
>  
> -	error = xfs_check_agi_freecount(cur, agi);
> +	error = xfs_check_agi_freecount(cur);
>  	if (error)
>  		goto error0;
>  
> @@ -1461,7 +1460,7 @@ xfs_dialloc_ag(
>  
>  	cur = xfs_inobt_init_cursor(mp, tp, agbp, pag, XFS_BTNUM_FINO);
>  
> -	error = xfs_check_agi_freecount(cur, agi);
> +	error = xfs_check_agi_freecount(cur);
>  	if (error)
>  		goto error_cur;
>  
> @@ -1504,7 +1503,7 @@ xfs_dialloc_ag(
>  	 */
>  	icur = xfs_inobt_init_cursor(mp, tp, agbp, pag, XFS_BTNUM_INO);
>  
> -	error = xfs_check_agi_freecount(icur, agi);
> +	error = xfs_check_agi_freecount(icur);
>  	if (error)
>  		goto error_icur;
>  
> @@ -1522,10 +1521,10 @@ xfs_dialloc_ag(
>  
>  	xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, -1);
>  
> -	error = xfs_check_agi_freecount(icur, agi);
> +	error = xfs_check_agi_freecount(icur);
>  	if (error)
>  		goto error_icur;
> -	error = xfs_check_agi_freecount(cur, agi);
> +	error = xfs_check_agi_freecount(cur);
>  	if (error)
>  		goto error_icur;
>  
> @@ -1911,7 +1910,7 @@ xfs_difree_inobt(
>  	 */
>  	cur = xfs_inobt_init_cursor(mp, tp, agbp, pag, XFS_BTNUM_INO);
>  
> -	error = xfs_check_agi_freecount(cur, agi);
> +	error = xfs_check_agi_freecount(cur);
>  	if (error)
>  		goto error0;
>  
> @@ -2004,7 +2003,7 @@ xfs_difree_inobt(
>  		xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, 1);
>  	}
>  
> -	error = xfs_check_agi_freecount(cur, agi);
> +	error = xfs_check_agi_freecount(cur);
>  	if (error)
>  		goto error0;
>  
> @@ -2029,7 +2028,6 @@ xfs_difree_finobt(
>  	xfs_agino_t			agino,
>  	struct xfs_inobt_rec_incore	*ibtrec) /* inobt record */
>  {
> -	struct xfs_agi			*agi = agbp->b_addr;
>  	struct xfs_btree_cur		*cur;
>  	struct xfs_inobt_rec_incore	rec;
>  	int				offset = agino - ibtrec->ir_startino;
> @@ -2114,7 +2112,7 @@ xfs_difree_finobt(
>  	}
>  
>  out:
> -	error = xfs_check_agi_freecount(cur, agi);
> +	error = xfs_check_agi_freecount(cur);
>  	if (error)
>  		goto error;
>  
> -- 
> 2.31.1
> 

-- 
Carlos


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] xfs: drop the AGI being passed to xfs_check_agi_freecount
  2021-06-07  4:15 [PATCH] xfs: drop the AGI being passed to xfs_check_agi_freecount Dave Chinner
  2021-06-07 10:46 ` Carlos Maiolino
@ 2021-06-08 16:05 ` Darrick J. Wong
  1 sibling, 0 replies; 3+ messages in thread
From: Darrick J. Wong @ 2021-06-08 16:05 UTC (permalink / raw)
  To: Dave Chinner; +Cc: linux-xfs

On Mon, Jun 07, 2021 at 02:15:29PM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Stephen Rothwell reported this compiler warning from linux-next:
> 
> fs/xfs/libxfs/xfs_ialloc.c: In function 'xfs_difree_finobt':
> fs/xfs/libxfs/xfs_ialloc.c:2032:20: warning: unused variable 'agi' [-Wunused-variable]
>  2032 |  struct xfs_agi   *agi = agbp->b_addr;
> 
> Which is fallout from agno -> perag conversions that were done in
> this function. xfs_check_agi_freecount() is the only user of "agi"
> in xfs_difree_finobt() now, and it only uses the agi to get the
> current free inode count. We hold that in the perag structure, so
> there's not need to directly reference the raw AGI to get this
> information.
> 
> The btree cursor being passed to xfs_check_agi_freecount() has a
> reference to the perag being operated on, so use that directly in
> xfs_check_agi_freecount() rather than passing an AGI.
> 
> Fixes: 7b13c5155182 ("xfs: use perag for ialloc btree cursors")
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>

Looks ok to me,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_ialloc.c | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 2ed6de6faf8a..654a8d9681e1 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -214,10 +214,9 @@ xfs_inobt_insert(
>   * Verify that the number of free inodes in the AGI is correct.
>   */
>  #ifdef DEBUG
> -STATIC int
> +static int
>  xfs_check_agi_freecount(
> -	struct xfs_btree_cur	*cur,
> -	struct xfs_agi		*agi)
> +	struct xfs_btree_cur	*cur)
>  {
>  	if (cur->bc_nlevels == 1) {
>  		xfs_inobt_rec_incore_t rec;
> @@ -243,12 +242,12 @@ xfs_check_agi_freecount(
>  		} while (i == 1);
>  
>  		if (!XFS_FORCED_SHUTDOWN(cur->bc_mp))
> -			ASSERT(freecount == be32_to_cpu(agi->agi_freecount));
> +			ASSERT(freecount == cur->bc_ag.pag->pagi_freecount);
>  	}
>  	return 0;
>  }
>  #else
> -#define xfs_check_agi_freecount(cur, agi)	0
> +#define xfs_check_agi_freecount(cur)	0
>  #endif
>  
>  /*
> @@ -1014,7 +1013,7 @@ xfs_dialloc_ag_inobt(
>  	if (!pagino)
>  		pagino = be32_to_cpu(agi->agi_newino);
>  
> -	error = xfs_check_agi_freecount(cur, agi);
> +	error = xfs_check_agi_freecount(cur);
>  	if (error)
>  		goto error0;
>  
> @@ -1234,7 +1233,7 @@ xfs_dialloc_ag_inobt(
>  	xfs_ialloc_log_agi(tp, agbp, XFS_AGI_FREECOUNT);
>  	pag->pagi_freecount--;
>  
> -	error = xfs_check_agi_freecount(cur, agi);
> +	error = xfs_check_agi_freecount(cur);
>  	if (error)
>  		goto error0;
>  
> @@ -1461,7 +1460,7 @@ xfs_dialloc_ag(
>  
>  	cur = xfs_inobt_init_cursor(mp, tp, agbp, pag, XFS_BTNUM_FINO);
>  
> -	error = xfs_check_agi_freecount(cur, agi);
> +	error = xfs_check_agi_freecount(cur);
>  	if (error)
>  		goto error_cur;
>  
> @@ -1504,7 +1503,7 @@ xfs_dialloc_ag(
>  	 */
>  	icur = xfs_inobt_init_cursor(mp, tp, agbp, pag, XFS_BTNUM_INO);
>  
> -	error = xfs_check_agi_freecount(icur, agi);
> +	error = xfs_check_agi_freecount(icur);
>  	if (error)
>  		goto error_icur;
>  
> @@ -1522,10 +1521,10 @@ xfs_dialloc_ag(
>  
>  	xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, -1);
>  
> -	error = xfs_check_agi_freecount(icur, agi);
> +	error = xfs_check_agi_freecount(icur);
>  	if (error)
>  		goto error_icur;
> -	error = xfs_check_agi_freecount(cur, agi);
> +	error = xfs_check_agi_freecount(cur);
>  	if (error)
>  		goto error_icur;
>  
> @@ -1911,7 +1910,7 @@ xfs_difree_inobt(
>  	 */
>  	cur = xfs_inobt_init_cursor(mp, tp, agbp, pag, XFS_BTNUM_INO);
>  
> -	error = xfs_check_agi_freecount(cur, agi);
> +	error = xfs_check_agi_freecount(cur);
>  	if (error)
>  		goto error0;
>  
> @@ -2004,7 +2003,7 @@ xfs_difree_inobt(
>  		xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, 1);
>  	}
>  
> -	error = xfs_check_agi_freecount(cur, agi);
> +	error = xfs_check_agi_freecount(cur);
>  	if (error)
>  		goto error0;
>  
> @@ -2029,7 +2028,6 @@ xfs_difree_finobt(
>  	xfs_agino_t			agino,
>  	struct xfs_inobt_rec_incore	*ibtrec) /* inobt record */
>  {
> -	struct xfs_agi			*agi = agbp->b_addr;
>  	struct xfs_btree_cur		*cur;
>  	struct xfs_inobt_rec_incore	rec;
>  	int				offset = agino - ibtrec->ir_startino;
> @@ -2114,7 +2112,7 @@ xfs_difree_finobt(
>  	}
>  
>  out:
> -	error = xfs_check_agi_freecount(cur, agi);
> +	error = xfs_check_agi_freecount(cur);
>  	if (error)
>  		goto error;
>  
> -- 
> 2.31.1
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-06-08 16:05 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-07  4:15 [PATCH] xfs: drop the AGI being passed to xfs_check_agi_freecount Dave Chinner
2021-06-07 10:46 ` Carlos Maiolino
2021-06-08 16:05 ` Darrick J. Wong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.