Re: [PATCH v2 1/2] xfs: Move ASSERTion location in xfs_rtcopy_summary()

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

On Mon, Feb 02, 2026 at 05:51:39PM +0100, Carlos Maiolino wrote:
> > > > > > What am I missing here? This looks weird...
> > > > > > We execute the block if sum is lower than 0, and then we assert it's
> > > > > > greater or equal than zero? This looks the assert will never fire as it
> > > > > > will only be checked when sum is always negative.
> > > > > Ugh, nvm, I'll grab more coffee. On the other hand, this still looks
> > > > > confusing, it would be better if we just ASSERT(0) there.
> > > > Well, the idea (as discussed in [1] and [2]) was that we should log that sum
> > > > has been assigned an illegal negative value (using an ASSERT) and then bail
> > > > out.
> > > > [1] https://lore.kernel.org/all/20260122181148.GE5945@frogsfrogsfrogs/
> > > > 
> > > > [2] https://lore.kernel.org/all/20260128161447.GV5945@frogsfrogsfrogs/
> > > I see. I honestly think this is really ugly, pointless, and confusing at
> > > a first glance (at least for me). The assert location is logged anyway
> > > when it fire.
> > > 
> > > If I'm the only one who finds this confusing, then fine, otherwise I'd
> > > rather see ASSERT(0) in there.
> > 
> > Sure, Carlos. ASSERT(0); sounds okay to me. Darrick, do you have any hard
> > preferences between ASSERT(0); and ASSERT(sum < 0); ? If not, then I can go
> > ahead, make the change and send a revision with the suggested change here.

Personally I think it's kinda dumb to log debugging messages that tell
you very little:

XFS: Assertion failed: 0, file: <whatever>

when you could actually say what the problem is:

XFS: Assertion failed: sum >= 0, file: <whatever>

but cem already said yes so I'm really just talking into the internet
here.

--D

> 
> Thanks!
>