Re: [PATCH v5 2/4] mm: Provide kernel parameter to allow disabling page init poisoning

[Dave Hansen]

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'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. :)

>> 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, an> 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).