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

[Lorenzo Stoakes]

On Sat, Jun 07, 2025 at 10:42:14AM -0300, Jason Gunthorpe wrote:
> On Fri, Jun 06, 2025 at 08:03:15PM +0100, Lorenzo Stoakes wrote:
> > 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?
>
> Honestly, that seems unnecessary, just a single buffer for the first
> global warning. Maybe with a 1 min rate limit for replacement or
> something.
>
> The kernel doesn't run around spewing warnings as a general rule.

Well, if you have WARN_ON()'s you tend to get that in loops if one goes.

But then in that case obviously you only usually truly care about the first.

>
> > 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.
>
> No! Delete WARN_ON_ONCE and say the new global mechanism is good
> enough to capture the first WARN_ON, everyone always uses it always
> and then nobody needs to think about this anymore when writing new
> code.
>
> Jason

Well that is simpler :)

I have encountered situations where I've had more than one and needed 2nd+
but it is rare as you say.

My late night incoherent babbling yesterday was perhaps because I
misunderstood David/John as to what they encountered in the past... maybe
they can clarify...

I do find myself grepping dmesg to find the first warning and it's
_annoying_ to do so, so this would be handy.

But I'm not sure it'd justify getting rid of WARN_ON_ONCE() when you are in
a loop or something and now your dmesg is going to go to hell, still useful
not to do that, esp. if you know there's no value to further warnings