Re: [PATCH v1] mm/gup: remove (VM_)BUG_ONs

[Lorenzo Stoakes]

On Fri, Jun 06, 2025 at 07:46:52PM +0100, Lorenzo Stoakes wrote:
> On Fri, Jun 06, 2025 at 03:42:12PM -0300, Jason Gunthorpe wrote:
> > On Fri, Jun 06, 2025 at 08:23:25PM +0200, David Hildenbrand wrote:
> > > > One last data point: I've often logged onto systems that were running
> > > > long enough that the dmesg had long since rolled over. And this makes
> > > > the WARN_ON_ONCE() items disappear.
> > >
> > > I think what would be *really* helpful would be quick access to the very
> > > first warning that triggered. At least that's what I usually dig for ... :)
> >
> > That's basically my point, it doesn't make sense to expose two APIs to
> > developers with a choice like this. The WARN_ON infrastructure should
> > deal with it consistently, maybe even configurable by the admin.
> >
> > Keeping the first warn in a buffer is definately a good option.
> >
> > Otherwise how is the patch author supposed to decide which API to
> > call in each case?
> >
> > Jason
>
> To clarify - are we talking the first instance of a specific warning, or
> the first warning in general?

OK sorry I'm being dumb, it is -per warning- reading the thread :P

So I guess you would have the macro establish a static buffer for each instance,
and then some interface for gathering those up and outputting them?

Always output to dmesg, but only populate if not already warned...

Or maybe rather than a static buffer (as those will add up quick and you'd have
to figure out how much space to take) it could be some dynamicly growing
kmalloc()'d thing because how often do you expect a warning, but guess you'd
have to be careful to ensure you're safely allocating given warning can be in
any context...

Or maybe another circular buffer just for this... hm yeah I guess that'd be the
most sane.

And I guess we'd not want a new interface for this like WARN_ON_ONCE_STORED()
because that'd be... weird and how would anyone think to use that and nearly all
cases wouldn't.

So I guess you'd want to convert WARN_ON() to use this, and maybe WARN_ON_ONCE()
to also use it but just not output if already seen.

It is quite a nice idea. Could be a debug feature though as would we want it for
prod?

It's late here and my brain is fried so if this is incoherent forgive me :))