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

[David Hildenbrand <david@redhat.com>]

On 06.06.25 12:13, Michal Hocko wrote:
> On Fri 06-06-25 11:01:18, David Hildenbrand wrote:
>> On 06.06.25 10:31, Michal Hocko wrote:
> [...]
>>> Turning them into VM_WARN_ON
>>> should be reasonably safe as they are not enabled in production
>>> environment anyway so we cannot really rely on those. Having them in
>>> WARN form would be still useful for debugging and those that really need
>>> a crash dump while debugging can achieve the same result.
>>
>> One question is if we should be VM_WARN_ON vs. VM_WARN_ON_ONCE ...
> 
> *WARN_ONCE ha a very limited use from code paths which are generally
> shared by many callers. You just see one and then nothing. Some time ago
> we have discussed an option to have _ONCE per call trace but I haven't
> see any follow up.

While true, getting a flood of the same events is absolutely not helpful 
in my experience.

Usually one cares about a single instance of any events, even if 
triggered by various code paths.

Sure, there are some other cases ... which brings me back of my original 
point of doing it stepwise.

> 
> Anyway starting without _ONCE seems like safer option because we are not
> losing potentially useful debugging information. Afterall this is
> debugging only thing. But no strong position on my side.

Yeah.

> 
>> VM_BUG_ON is essentially a "once" thing so far, but then, we don't continue
>> ... so probably most should be _ONCE.
>>
>>>
>>> So while I agree that many of them could be dropped or made more clear
>>> those could be dealt with after a mass move. An advantage of this would
>>> be that we can drop VM_BUG_ON* and stop new instances from being added.
>>
>> As a first step we could probably just #define them to go to the
>> VM_WARN_ON_* variants and see what happens.
> 
> You meand VM_BUG_ON expand to VM_WARN_ON by default?

That's one approach with little churn, that still allows us easily to go 
through them and re-evaluate them later as we actually remove them.

-- 
Cheers,

David / dhildenb