[PATCH] xfs: use xfs_csn_t for xlog_cil_push_now() push_seq parameter

[PATCH] xfs: use xfs_csn_t for xlog_cil_push_now() push_seq parameter

[alexjlzheng]

From: Jinliang Zheng <alexjlzheng@tencent.com>

The push_seq argument to xlog_cil_push_now() carries a CIL checkpoint
sequence number, not a log sequence number (LSN).

Change the parameter type from xfs_lsn_t to xfs_csn_t to correctly
reflect its semantics and match the surrounding types. Both types are
int64_t under the hood, so this is a type-annotation fix with no
behavioural change.

Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
---
 fs/xfs/xfs_log_cil.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index edc368938f30..639f875a8fb2 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -1710,7 +1710,7 @@ xlog_cil_push_background(
 static void
 xlog_cil_push_now(
 	struct xlog	*log,
-	xfs_lsn_t	push_seq,
+	xfs_csn_t	push_seq,
 	bool		async)
 {
 	struct xfs_cil	*cil = log->l_cilp;
-- 
2.43.7

Re: [PATCH] xfs: use xfs_csn_t for xlog_cil_push_now() push_seq parameter

[Darrick J. Wong]

On Thu, Jul 02, 2026 at 07:56:11PM +0800, alexjlzheng@gmail.com wrote:
> From: Jinliang Zheng <alexjlzheng@tencent.com>
> 
> The push_seq argument to xlog_cil_push_now() carries a CIL checkpoint
> sequence number, not a log sequence number (LSN).
> 
> Change the parameter type from xfs_lsn_t to xfs_csn_t to correctly
> reflect its semantics and match the surrounding types. Both types are
> int64_t under the hood, so this is a type-annotation fix with no
> behavioural change.
> 
> Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>

I wonder, did you find with some tool?  And would it be useful to add
__bitwise to the typedef so that static checkers will find the places
where we screw this up?

Regardless, this is correct so
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/xfs_log_cil.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index edc368938f30..639f875a8fb2 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -1710,7 +1710,7 @@ xlog_cil_push_background(
>  static void
>  xlog_cil_push_now(
>  	struct xlog	*log,
> -	xfs_lsn_t	push_seq,
> +	xfs_csn_t	push_seq,
>  	bool		async)
>  {
>  	struct xfs_cil	*cil = log->l_cilp;
> -- 
> 2.43.7
> 
> 

Re: [PATCH] xfs: use xfs_csn_t for xlog_cil_push_now() push_seq parameter

[Jinliang Zheng]

On Thu, Jul 02, 2026 at 08:53:05AM -0700, Darrick J. Wong wrote:
> I wonder, did you find with some tool?  And would it be useful to add
> __bitwise to the typedef so that static checkers will find the places
> where we screw this up?

No tool -- I stumbled on it while reading the log code for an unrelated
xfs issue, and noticed xlog_cil_push_now() was taking its CIL sequence
argument as xfs_lsn_t.

I did try the __bitwise idea and measured it: on a clean tree "make C=2
fs/xfs/" reports 0 sparse warnings, and adding __bitwise to both
xfs_lsn_t and xfs_csn_t produces 286 new ones across 26 files. Only 11
of them involve xfs_csn_t (all in xfs_log_cil.c); the other 275 are
xfs_lsn_t.

Almost none are real bugs -- they're legitimate uses that __bitwise
disallows: sequence numbers being compared/incremented ("degrades to
integer"), the cycle/block splitting in _lsn_cmp()/CYCLE_LSN()/
BLOCK_LSN() (xfs_log.h alone accounts for 120), and every trace event
that prints an lsn/csn (~49 in xfs_trace.h).

Clearing those would need a fair number of __force casts plus rework of
the LSN helper macros, and spreading __force that widely tends to hide
the very mistakes we'd want to catch. So my own preference would be to
not add __bitwise: the churn and the pervasive __force casts outweigh
the benefit of catching this fairly rare class of mistake. That said,
I'm happy to revisit if you think a more targeted approach (e.g.
wrapping the sequence comparisons in typed helpers) would be worthwhile.

> Regardless, this is correct so
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

Thanks for the review!

Jinliang Zheng