On 9/25/2018 3:14 PM, Dave Hansen wrote:
On 09/25/2018 01:38 PM, Alexander Duyck wrote:
On 9/25/2018 1:26 PM, Dave Hansen wrote:
On 09/25/2018 01:20 PM, Alexander Duyck wrote:
+ vm_debug[=options] [KNL] Available with CONFIG_DEBUG_VM=y.
+ May slow down system boot speed, especially when
+ enabled on systems with a large amount of memory.
+ All options are enabled by default, and this
+ interface is meant to allow for selectively
+ enabling or disabling specific virtual memory
+ debugging features.
+
+ Available options are:
+ P Enable page structure init time poisoning
+ - Disable all of the above options
Can we have vm_debug=off for turning things off, please? That seems to
be pretty standard.
No. The simple reason for that is that you had requested this work like
the slub_debug. If we are going to do that then each individual letter
represents a feature. That is why the "-" represents off. We cannot have
letters represent flags, and letters put together into words. For
example slub_debug=OFF would turn on sanity checks and turn off
debugging for caches that would have causes higher minimum slab orders.
We don't have to have the same letters mean the same things for both
options. We also can live without 'o' and 'f' being valid. We can
*also* just say "don't do 'off'" if you want to enable things.
I'm not saying we do either. I would prefer it if we stuck to similar
behavior though. If we are going to do a slub_debug style parameter then
we should stick with similar behavior where "-" is used to indicate all
features off.
I'd much rather have vm_debug=off do the right thing than have
per-feature enable/disable. I know I'll *never* remember vm_debug=- and
doing it this way will subject me to innumerable trips to Documentation/
during my few remaining years.
Surely you can make vm_debug=off happen. :)
I could, but then it is going to confuse people even more. I really feel
that if we want to do a slub_debug style interface we should use the
same switch for turning off all the features that they do for slub_debug.
we need to document the defaults. I think the default is "all
debug options are enabled", but it would be nice to document that.
In the description I call out "All options are enabled by default, and this interface is meant to allow for selectively enabling or disabling".
I found "all options are enabled by default" really confusing. Maybe:
"Control debug features which become available when CONFIG_DEBUG_VM=y.
When this option is not specified, all debug features are enabled. Use
this option enable a specific subset."
Then, let's actually say what the options do, and what their impact is:
P Enable 'struct page' poisoning at initialization.
(Slows down boot time).
From my perspective I just don't see how this changes much since it
conveys the same message I had conveyed in my description. Since it
looks like Andrew applied the patch feel free to submit your suggestion
here as a follow-up patch and I would be willing to review/ack it.
