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

[Lorenzo Stoakes]

Overall,

Since David and I are somewhat indifferent on this point and you would very
much prefer a VM_WARN_ON() - when I drop the churnageddon I'll go with
VM_WARN_ON() :)

And we can obvious adjust case-by-case after that if needed (probably none
of these ever trigger tbh).

I think the general feeling in the room re: VM_BUG_ON() is 'kill it with
fire I don't care how' :P

And you know, it's understandable...

On Fri, Jun 06, 2025 at 08:23:25PM +0200, David Hildenbrand wrote:
> On 06.06.25 20:21, John Hubbard wrote:
> >
> >
> > On 6/6/25 11:15 AM, David Hildenbrand wrote:
> > > On 06.06.25 20:06, Lorenzo Stoakes wrote:
> > > > On Fri, Jun 06, 2025 at 10:57:44AM -0700, John Hubbard wrote:
> > > > > On 6/6/25 4:04 AM, Lorenzo Stoakes wrote:
> > > > > > On Fri, Jun 06, 2025 at 12:28:28PM +0200, David Hildenbrand wrote:
> > > > > > > On 06.06.25 12:19, Lorenzo Stoakes wrote:
> > > > > > > > On Fri, Jun 06, 2025 at 12:13:27PM +0200, Michal Hocko wrote:
> > > > > > > > > On Fri 06-06-25 11:01:18, David Hildenbrand wrote:
> > > > > > > > > > On 06.06.25 10:31, Michal Hocko wrote:
> > > > > > > > > [...]
> > > > > > So to me the only assessment needed is 'do we want to warn on this or not?'.
> > > > > >
> > > > > > And as you say, really WARN_ON_ONCE() seems appropriate, because nearly always
> > > > > > we will get flooded with useless information.
> > > > > >
> > > > >
> > > > > As yet another victim of such WARN_ON() floods at times, I've followed
> > > > > this thread with great interest. And after reflecting on it a bit, I believe
> > > > > that, surprisingly enough, WARN_ON() is a better replacement for VM_BUG_ON()
> > > > > than WARN_ON_ONCE(), because:
> > > >
> > > > Right, these shouldn't be happening _at all_.
> > >   > > I'm easy on this point, I'd say in that case VM_WARN_ON() is the most
> > > > _conservative_ approach, since these are things that must not happen, and
> > > > so it's not unreasonable to fail to repress repetitions of the 'impossible'
> > > > :)
> > > >
> > > > But I get the general point about ...WARN_ON_ONCE() avoiding floods.
> > > >
> > > > David, what do you think?
> > >
> > > Well, in this patch here I deliberately want _ONCE for the unpin sanity
> > > checks. Because if they start happening (IOW, now after 5 years observed
> > > for the first time?) I *absolutely don't* want to get flooded and
> > > *really* figure out what is going on by seeing what else failed.
> > >
> > > And crashing on VM_BUG_ON() and not observing anything else was also not
> > > particularly helpful :)
> > >
> > > Because ... they shouldn't be happening ...
> > >
> > > (well, it goes back to my initial point about requiring individual
> > > decisions etc ...)
> > >
> > > Not sure what's best now in the general case, in the end I don't care
> > > that much.
> > >
> > > Roll a dice? ;)
> >
> > 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 ... :)

YES!

I wonder if there's some systemd thingy that does this somehow...

>
> --
> Cheers,
>
> David / dhildenb
>