On 09.09.21 11:07, Vlastimil Babka wrote:
On 9/9/21 10:56, David Hildenbrand wrote:
On 09.09.21 00:42, John Hubbard wrote:
On 9/7/21 2:56 AM, David Hildenbrand wrote:
...
If this can be handled gracefully, then I'd rather go with VM_WARN_ON.
Maybe even WARN_ON_ONCE?
I think either VM_BUG_ON() or VM_WARN_ON() -- compiling the runtime
checks out -- should be good
enough.
I'd just go with VM_BUG_ON(), because anybody messing with
__isolate_free_page() should clearly spot
that we expect the current handling. But no strong opinion.
If in doubt, WARN*() should be preferred over BUG*(). There's a pretty long
history of "don't kill the machine unless you have to" emails about this, let
me dig up one...OK, maybe not the best example, but the tip of the iceberg:
Please note the subtle difference between BUG_ON and VM_BUG_ON. We expect
VM_BUG_ON to be compiled out on any production system. So it's really only a
mean to identify things that really shouldn't be like that during
debugging/testing.
IIRC Fedora used to have CONFIG_DEBUG_VM enabled, did it change?
Excellent question. Apparently you are right. Fortunately it's not a
distro to use in production ;)
In kernel-ark:
redhat/configs/fedora/generic/CONFIG_DEBUG_VM:CONFIG_DEBUG_VM=y
While for ARK (rhel-next so to say)
redhat/configs/ark/generic/CONFIG_DEBUG_VM:# CONFIG_DEBUG_VM is not set
So yes, the VM_WARN_ON would then be preferred in that case. But it's
something that should never ever happen unless reviewers and developers
really mess up, so I don't actually would sleep over that. We have other
WARN... that can trigger more easily.
Using WARN... instead of VM_BUG_ON is even worse for production systems.
There are distros that set panic_on_warn, essentially converting WARN...
into BUG...
Uh, does any distro really do that?
Apparently, so I was told by Greg a year ago or so when wanting to add
WARN_ON(). The advisory is to us pr_warn_once() instead. I rememebr it
was a debian based distro.
--
Thanks,
David / dhildenb
