Re: [PATCH] x86: mtrr cleanup for converting continuous to discrete layout v3

[Ingo Molnar <mingo@elte.hu>]

* Yinghai Lu <yhlu.kernel@gmail.com> wrote:

> On Mon, Apr 28, 2008 at 1:15 PM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> >  * Yinghai Lu <yhlu.kernel.send@gmail.com> wrote:
> >
> >  > some BIOS like to use continus MTRR layout, and some X driver can not
> >  > add WB entries for graphical cards when 4g or more RAM installed.
> >  >
> >  > the patch will change MTRR to discrete.
> >  >
> >  > mtrr_chunk_size= could be used to have smaller continuous block to
> >  > hold holes. default is 256m, could be set according to size of
> >  > graphics card memory.
> >  >
> >  > v2: fix -1 for UC checking
> >
> >  > v3: default to disable, and need use enable_mtrr_cleanup to enable
> >  >     this feature
> >  >     skip the var state change warning.
> >  >     remove next_basek in range_to_mtrr()
> >
> >  a boot option is often inconvenient though - could you perhaps also make
> >  this a Kconfig option, with it defaulting to off? Something like
> >  CONFIG_MTRR_SANITIZE=y or so? That way distros can make a conscious
> >  decision as well whether they want this feature (for the Xorg they pick)
> >  or not.
> 
> use disable_mtrr_cleanup and CONFIG_MTRR_SANITIZER?

yeah, please keep both.

in general, the best configurability for any particular new kernel 
feature is a trio of options:

  - .config option (disabled/boot-enabled/boot-disabled)
  - sysctl option
  - boot option

since your new feature runs during early bootup the sysctl vector is 
meaningless - that leaves the boot option and the .config. The .config 
can have 3 states:

   completely-disabled
   disabled-but-boot-option-enable-able
   enabled-but-boot-option-disable-able

depending on how the feature works out, people and distros will 
gravitate towards one of these combinations. We usually do not know it 
ahead of time which one that will be.

	Ingo