[next] xfs: remove the redundant xfs_alloc_log_agf

[next] xfs: remove the redundant xfs_alloc_log_agf

[Long Li]

There are two invocations of xfs_alloc_log_agf in xfs_alloc_put_freelist.
The AGF does not change between the two calls. Although this does not pose
any practical problems, it seems like a small mistake. Therefore, fix it
by removing the first xfs_alloc_log_agf invocation.

Signed-off-by: Long Li <leo.lilong@huawei.com>
---
 fs/xfs/libxfs/xfs_alloc.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 59326f84f6a5..cce32b2f3ffd 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -3159,8 +3159,6 @@ xfs_alloc_put_freelist(
 		logflags |= XFS_AGF_BTREEBLKS;
 	}
 
-	xfs_alloc_log_agf(tp, agbp, logflags);
-
 	ASSERT(be32_to_cpu(agf->agf_flcount) <= xfs_agfl_size(mp));
 
 	agfl_bno = xfs_buf_to_agfl_bno(agflbp);
-- 
2.39.2

Re: [next] xfs: remove the redundant xfs_alloc_log_agf

[Carlos Maiolino]

Hello.

What do you mean with the [next] tag in the subject, instead of usual [PATCH]
tag?

On Mon, Sep 30, 2024 at 06:42:17PM GMT, Long Li wrote:
> There are two invocations of xfs_alloc_log_agf in xfs_alloc_put_freelist.
> The AGF does not change between the two calls. Although this does not pose
> any practical problems, it seems like a small mistake. Therefore, fix it
> by removing the first xfs_alloc_log_agf invocation.
> 
> Signed-off-by: Long Li <leo.lilong@huawei.com>
> ---
>  fs/xfs/libxfs/xfs_alloc.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 59326f84f6a5..cce32b2f3ffd 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -3159,8 +3159,6 @@ xfs_alloc_put_freelist(
>  		logflags |= XFS_AGF_BTREEBLKS;
>  	}
>  
> -	xfs_alloc_log_agf(tp, agbp, logflags);
> -

Hmm.. Isn't this logged twice because of lazy-count?


Carlos


>  	ASSERT(be32_to_cpu(agf->agf_flcount) <= xfs_agfl_size(mp));
>  
>  	agfl_bno = xfs_buf_to_agfl_bno(agflbp);
> -- 
> 2.39.2
> 
> 

Re: [next] xfs: remove the redundant xfs_alloc_log_agf

[Long Li]

On Mon, Sep 30, 2024 at 02:55:47PM +0200, Carlos Maiolino wrote:
> Hello.
> 
> What do you mean with the [next] tag in the subject, instead of usual [PATCH]
> tag?

I intended to use [PATCH-next] in the subject, but I made a mistake. Do I
need to resend?

> 
> On Mon, Sep 30, 2024 at 06:42:17PM GMT, Long Li wrote:
> > There are two invocations of xfs_alloc_log_agf in xfs_alloc_put_freelist.
> > The AGF does not change between the two calls. Although this does not pose
> > any practical problems, it seems like a small mistake. Therefore, fix it
> > by removing the first xfs_alloc_log_agf invocation.
> > 
> > Signed-off-by: Long Li <leo.lilong@huawei.com>
> > ---
> >  fs/xfs/libxfs/xfs_alloc.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> > index 59326f84f6a5..cce32b2f3ffd 100644
> > --- a/fs/xfs/libxfs/xfs_alloc.c
> > +++ b/fs/xfs/libxfs/xfs_alloc.c
> > @@ -3159,8 +3159,6 @@ xfs_alloc_put_freelist(
> >  		logflags |= XFS_AGF_BTREEBLKS;
> >  	}
> >  
> > -	xfs_alloc_log_agf(tp, agbp, logflags);
> > -
> 
> Hmm.. Isn't this logged twice because of lazy-count?
> 
> 
> Carlos
> 

I think that logging twice is unrelated to the lazy superblock count,
as the lazy superblock count is rebuilt using information from the AGF.
We only need to log AGF normally, not twice.

Thanks,
Long Li

> 
> >  	ASSERT(be32_to_cpu(agf->agf_flcount) <= xfs_agfl_size(mp));
> >  
> >  	agfl_bno = xfs_buf_to_agfl_bno(agflbp);
> > -- 
> > 2.39.2
> > 
> > 

Re: [next] xfs: remove the redundant xfs_alloc_log_agf

[Dave Chinner]

[ Your email is being being classified as spam by gmail because it
does not have a valid DKIM authentication signature.  Hence it
doesn't get delivered to anyone who's mail is backed by gmail.... ]

On Mon, Sep 30, 2024 at 06:42:17PM +0800, Long Li wrote:
> There are two invocations of xfs_alloc_log_agf in xfs_alloc_put_freelist.
> The AGF does not change between the two calls. Although this does not pose
> any practical problems, it seems like a small mistake. Therefore, fix it
> by removing the first xfs_alloc_log_agf invocation.
> 
> Signed-off-by: Long Li <leo.lilong@huawei.com>
> ---
>  fs/xfs/libxfs/xfs_alloc.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 59326f84f6a5..cce32b2f3ffd 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -3159,8 +3159,6 @@ xfs_alloc_put_freelist(
>  		logflags |= XFS_AGF_BTREEBLKS;
>  	}
>  
> -	xfs_alloc_log_agf(tp, agbp, logflags);
> -

Looks fine. That's been there since commit 92821e2ba4ae ("[XFS] Lazy
Superblock Counters") was merged back in 2007...

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

-- 
Dave Chinner
david@fromorbit.com

Re: [next] xfs: remove the redundant xfs_alloc_log_agf

[Carlos Maiolino]

On Mon, 30 Sep 2024 18:42:17 +0800, Long Li wrote:
> There are two invocations of xfs_alloc_log_agf in xfs_alloc_put_freelist.
> The AGF does not change between the two calls. Although this does not pose
> any practical problems, it seems like a small mistake. Therefore, fix it
> by removing the first xfs_alloc_log_agf invocation.
> 
> 

Applied to for-next, thanks!

[1/1] xfs: remove the redundant xfs_alloc_log_agf
      (no commit info)

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>