diff mbox series

[next] xfs: remove the redundant xfs_alloc_log_agf

Message ID 20240930104217.2184941-1-leo.lilong@huawei.com (mailing list archive)
State Queued
Headers show
Series [next] xfs: remove the redundant xfs_alloc_log_agf | expand

Commit Message

Long Li Sept. 30, 2024, 10:42 a.m. UTC
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(-)

Comments

Carlos Maiolino Sept. 30, 2024, 12:55 p.m. UTC | #1
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
> 
>
Dave Chinner Sept. 30, 2024, 11:19 p.m. UTC | #2
[ 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>
Long Li Oct. 8, 2024, 2:19 a.m. UTC | #3
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
> > 
> >
Carlos Maiolino Oct. 11, 2024, 7:44 a.m. UTC | #4
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,
diff mbox series

Patch

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);