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

From: Jinliang Zheng

Date: Thu Jul 02 2026 - 23:33:35 EST


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@xxxxxxxxxx>

Thanks for the review!

Jinliang Zheng