Re: [PATCH] mm/hotplug: Only respect mem= parameter during boot stage

Baoquan He
2019-12-11T13:20:27Z

On 12/10/19 at 03:19pm, Michal Hocko wrote:
> On Tue 10-12-19 22:05:34, Baoquan He wrote:
> > On 12/10/19 at 02:32pm, Michal Hocko wrote:
> > > On Tue 10-12-19 20:55:57, Baoquan He wrote:
> > > [...]
> > > > Btw, as you said at above, I am confused by the '[KNL,BOOT]', what does
> > > > the 'BOOT' mean in the documentation of 'mem='? I checked all parameters
> > > > with 'BOOT', still don't get it clearly.
> > >
> > > This is a good question indeed. I have checked closer and this is what
> > > documentation says
> > > Documentation/admin-guide/kernel-parameters.rst
> > > "
> > > BOOT Is a boot loader parameter.
> > >
> > > Parameters denoted with BOOT are actually interpreted by the boot
> > > loader, and have no meaning to the kernel directly.
> > > "
> > >
> > > and that really doesn't fit, right? So I went to check the full history
> > > git tree just to get to 2.4.0-test5 and no explanation whatsoever.
> > > Fun, isn't it? ;)
> >
> > Yeah, very interesting. Finally I got their original purpose from
> > Documentation/x86/boot.rst.
> >
> >
> > Special Command Line Options
> > ============================
> >
> > If the command line provided by the boot loader is entered by the
> > user, the user may expect the following command line options to work.
> > They should normally not be deleted from the kernel command line even
> > though not all of them are actually meaningful to the kernel. Boot
> > loader authors who need additional command line options for the boot
> > loader itself should get them registered in
> > Documentation/admin-guide/kernel-parameters.rst to make sure they will not
> > conflict with actual kernel options now or in the future.
> >
> > ...
> >
> > So here, [KNL,BOOT], KNL means it's used for kernel, BOOT means it's
> > needed by boot loader.
>
> OK, that clarifies this a bit. Thanks for referencing to it!
> That should explain how the behavior is not boot time restricted at all
> and the current implementation is actually correct. So a change to it
> should clearly state the new usecase as we have already discussed. In
> case there are bootloaders which really rely on the original strict
> meaning then we should be able to compare cost/benfits of those two
> usecases.

Sounds reasonable to me. From the current parameters for x86, it only
impact the bootloader during boot-time, e.g 'mem= ' says bootloader need
this to place initrd. It might not give the trouble, anyway, we will
say.

Just a little more, we have test case for memory hotplug which only test
the DIMM added after boot. And the old 'mem= ' implementation in x86
only erazes memory regions above 'mem= ' in e820 table. That is why the
behaviour change immediately gave me a surprise when I noticed people
back ported Jurgen's patch to our distros.

So glad to see all is clear, thanks.