Re: [PATCH 1/2] xfs: Clear W=1 warning in xfs_iwalk_run_callbacks():

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Apr 25, 2024 at 04:37:25PM +0100, John Garry wrote:
> On 25/04/2024 14:33, John Garry wrote:
> > > 
> > > (it also wasn't in the original patch and only got added working around
> > > some debug warnings)
> > 
> > Fine, I'll look to remove those ones as well, which I think is possible
> > with the same method you suggest.
> 
> It's a bit messy, as xfs_buf.b_addr is a void *:
> 
> From 1181afdac3d61b79813381d308b9ab2ebe30abca Mon Sep 17 00:00:00 2001
> From: John Garry <john.g.garry@oracle.com>
> Date: Thu, 25 Apr 2024 16:23:49 +0100
> Subject: [PATCH] xfs: Stop using __maybe_unused in xfs_alloc.c
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 9da52e92172a..5d84a97b4971 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -1008,13 +1008,13 @@ xfs_alloc_cur_finish(
>  	struct xfs_alloc_arg	*args,
>  	struct xfs_alloc_cur	*acur)
>  {
> -	struct xfs_agf __maybe_unused *agf = args->agbp->b_addr;

If you surround this declaration with #ifdef DEBUG, will the warning go
away...

>  	int			error;
> 
>  	ASSERT(acur->cnt && acur->bnolt);
>  	ASSERT(acur->bno >= acur->rec_bno);
>  	ASSERT(acur->bno + acur->len <= acur->rec_bno + acur->rec_len);
> -	ASSERT(acur->rec_bno + acur->rec_len <= be32_to_cpu(agf->agf_length));
> +	ASSERT(acur->rec_bno + acur->rec_len <=
> +		be32_to_cpu(((struct xfs_agf *)args->agbp->b_addr)->agf_length));

...without the need for this?

--D

> 
>  	error = xfs_alloc_fixup_trees(acur->cnt, acur->bnolt, acur->rec_bno,
>  				      acur->rec_len, acur->bno, acur->len, 0);
> @@ -1217,7 +1217,7 @@ STATIC int			/* error */
>  xfs_alloc_ag_vextent_exact(
>  	xfs_alloc_arg_t	*args)	/* allocation argument structure */
>  {
> -	struct xfs_agf __maybe_unused *agf = args->agbp->b_addr;
> +	struct xfs_buf	*agbp = args->agbp;
>  	struct xfs_btree_cur *bno_cur;/* by block-number btree cursor */
>  	struct xfs_btree_cur *cnt_cur;/* by count btree cursor */
>  	int		error;
> @@ -1234,8 +1234,7 @@ xfs_alloc_ag_vextent_exact(
>  	/*
>  	 * Allocate/initialize a cursor for the by-number freespace btree.
>  	 */
> -	bno_cur = xfs_bnobt_init_cursor(args->mp, args->tp, args->agbp,
> -					  args->pag);
> +	bno_cur = xfs_bnobt_init_cursor(args->mp, args->tp, agbp, args->pag);
> 
>  	/*
>  	 * Lookup bno and minlen in the btree (minlen is irrelevant, really).
> @@ -1295,9 +1294,9 @@ xfs_alloc_ag_vextent_exact(
>  	 * We are allocating agbno for args->len
>  	 * Allocate/initialize a cursor for the by-size btree.
>  	 */
> -	cnt_cur = xfs_cntbt_init_cursor(args->mp, args->tp, args->agbp,
> -					args->pag);
> -	ASSERT(args->agbno + args->len <= be32_to_cpu(agf->agf_length));
> +	cnt_cur = xfs_cntbt_init_cursor(args->mp, args->tp, agbp, args->pag);
> +	ASSERT(args->agbno + args->len <=
> +		be32_to_cpu(((struct xfs_agf *)agbp->b_addr)->agf_length));
>  	error = xfs_alloc_fixup_trees(cnt_cur, bno_cur, fbno, flen, args->agbno,
>  				      args->len, XFSA_FIXUP_BNO_OK);
>  	if (error) {
> -- 
> 2.35.3
> 
> 
> ---
> 
> There's a few ways to improve this, like make xfs_buf.b_addr a union, but I
> am not sure if it is worth it.
> 
>