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

[David Hildenbrand <david@redhat.com>]

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:
>> [...]
>>>> 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.
>>
>> 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.
>>
>>> 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?
> 
> Sorry to interject in the conversation, but to boldly throw my two English pence
> into the mix:
> 
> As the "king of churn" (TM) you'll not be surprised to hear that I'm in favour
> of us just doing a big patch and convert all VM_BUG_ON() -> VM_WARN_ON_ONCE()
> and remove VM_BUG_ON*().
> 
> Pull the band-aid off... I think better than a #define if this indeed what you
> meant David.
> 
> But of course, you'd expect me to have this opinion ;)

See my reply to Michal regarding keeping VM_BUG_ON() until we actually 
decided what the right cleanup is.

I don't have a very strong opinion on any of this ... as long as 
VM_BUG_ON() goes away one way or the other.

-- 
Cheers,

David / dhildenb