In commit 357b4da50a62 ("x86: respect memory size limiting via mem=
parameter") a global varialbe global max_mem_size is added to store
the value parsed from 'mem= ', then checked when memory region is
added. This truly stops those DIMM from being added into system memory
during boot-time.
However, it also limits the later memory hotplug functionality. Any
memory board can't be hot added any more if its region is beyond the
max_mem_size. System will print error like below:
[ 216.387164] acpi PNP0C80:02: add_memory failed
[ 216.389301] acpi PNP0C80:02: acpi_memory_enable_device() error
[ 216.392187] acpi PNP0C80:02: Enumeration failure
From document of 'mem= ' parameter, it should be a restriction during
boot, but not impact the system memory adding/removing after booting.
mem=nn[KMG] [KNL,BOOT] Force usage of a specific amount of memory
...
So fix it by also checking if it's during boot-time when restrict memory
adding. Otherwise, skip the restriction.
Fixes: 357b4da50a62 ("x86: respect memory size limiting via mem= parameter")
Signed-off-by: Baoquan He <[email protected]>
---
mm/memory_hotplug.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 55ac23ef11c1..989707295d15 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -105,7 +105,11 @@ static struct resource *register_memory_resource(u64 start, u64 size)
unsigned long flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
char *resource_name = "System RAM";
- if (start + size > max_mem_size)
+ /*
+ * Make sure value parsed from 'mem=' only restricts memory adding
+ * during boot-time, so that memory hotplug won't be impacted.
+ */
+ if (start + size > max_mem_size && system_state < SYSTEM_RUNNING)
return ERR_PTR(-E2BIG);
/*
--
2.17.2
On 10.12.19 09:44, Baoquan He wrote:
> In commit 357b4da50a62 ("x86: respect memory size limiting via mem=
> parameter") a global varialbe global max_mem_size is added to store
> the value parsed from 'mem= ', then checked when memory region is
> added. This truly stops those DIMM from being added into system memory
> during boot-time.
>
> However, it also limits the later memory hotplug functionality. Any
> memory board can't be hot added any more if its region is beyond the
> max_mem_size. System will print error like below:
>
> [ 216.387164] acpi PNP0C80:02: add_memory failed
> [ 216.389301] acpi PNP0C80:02: acpi_memory_enable_device() error
> [ 216.392187] acpi PNP0C80:02: Enumeration failure
>
> From document of 'mem= ' parameter, it should be a restriction during
> boot, but not impact the system memory adding/removing after booting.
>
> mem=nn[KMG] [KNL,BOOT] Force usage of a specific amount of memory
> ...
>
> So fix it by also checking if it's during boot-time when restrict memory
> adding. Otherwise, skip the restriction.
>
> Fixes: 357b4da50a62 ("x86: respect memory size limiting via mem= parameter")
> Signed-off-by: Baoquan He <[email protected]>
Reviewed-by: Juergen Gross <[email protected]>
Juergen
On 10/12/19 7:44 pm, Baoquan He wrote:
> In commit 357b4da50a62 ("x86: respect memory size limiting via mem=
> parameter") a global varialbe global max_mem_size is added to store
typo ^^^
> the value parsed from 'mem= ', then checked when memory region is
> added. This truly stops those DIMM from being added into system memory
> during boot-time.
>
> However, it also limits the later memory hotplug functionality. Any
> memory board can't be hot added any more if its region is beyond the
> max_mem_size. System will print error like below:
>
> [ 216.387164] acpi PNP0C80:02: add_memory failed
> [ 216.389301] acpi PNP0C80:02: acpi_memory_enable_device() error
> [ 216.392187] acpi PNP0C80:02: Enumeration failure
>
> From document of 'mem= ' parameter, it should be a restriction during
> boot, but not impact the system memory adding/removing after booting.
>
> mem=nn[KMG] [KNL,BOOT] Force usage of a specific amount of memory
> ...
>
> So fix it by also checking if it's during boot-time when restrict memory
> adding. Otherwise, skip the restriction.
>
The fix looks reasonable, but I don't get the use case. Booting with mem= is
generally a debug option, is this for debugging memory hotplug + limited memory?
Balbir
On 10.12.19 09:44, Baoquan He wrote:
> In commit 357b4da50a62 ("x86: respect memory size limiting via mem=
> parameter") a global varialbe global max_mem_size is added to store
Only some nits:
s/varialbe global/variable/
> the value parsed from 'mem= ', then checked when memory region is
> added. This truly stops those DIMM from being added into system memory
s/DIMM/DIMMs/
> during boot-time.
>
> However, it also limits the later memory hotplug functionality. Any
> memory board can't be hot added any more if its region is beyond the
s/memory board/DIMM/ ?
s/hot added/hotplugged/ ?
> max_mem_size. System will print error like below:
"We will get errors like:"
>
> [ 216.387164] acpi PNP0C80:02: add_memory failed
> [ 216.389301] acpi PNP0C80:02: acpi_memory_enable_device() error
> [ 216.392187] acpi PNP0C80:02: Enumeration failure
>
> From document of 'mem= ' parameter, it should be a restriction during
"the documentation of the"
> boot, but not impact the system memory adding/removing after booting.
"but not impact memory hotplug once booted."
>
> mem=nn[KMG] [KNL,BOOT] Force usage of a specific amount of memory
> ...
>
> So fix it by also checking if it's during boot-time when restrict memory
s/when restrict memory adding/restricting to add memory/
> adding. Otherwise, skip the restriction.
>
> Fixes: 357b4da50a62 ("x86: respect memory size limiting via mem= parameter")
> Signed-off-by: Baoquan He <[email protected]>
> ---
> mm/memory_hotplug.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 55ac23ef11c1..989707295d15 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -105,7 +105,11 @@ static struct resource *register_memory_resource(u64 start, u64 size)
> unsigned long flags = IORESOURCE_SYSTEM_RAM | IORESOURCE_BUSY;
> char *resource_name = "System RAM";
>
> - if (start + size > max_mem_size)
> + /*
> + * Make sure value parsed from 'mem=' only restricts memory adding
> + * during boot-time, so that memory hotplug won't be impacted.
s/during boot-time/while booting/
Only some nits, thanks!
Reviewed-by: David Hildenbrand <[email protected]>
--
Thanks,
David / dhildenb
On 12/10/19 at 08:24pm, Balbir Singh wrote:
>
>
> On 10/12/19 7:44 pm, Baoquan He wrote:
> > In commit 357b4da50a62 ("x86: respect memory size limiting via mem=
> > parameter") a global varialbe global max_mem_size is added to store
> typo ^^^
> > the value parsed from 'mem= ', then checked when memory region is
> > added. This truly stops those DIMM from being added into system memory
> > during boot-time.
> >
> > However, it also limits the later memory hotplug functionality. Any
> > memory board can't be hot added any more if its region is beyond the
> > max_mem_size. System will print error like below:
> >
> > [ 216.387164] acpi PNP0C80:02: add_memory failed
> > [ 216.389301] acpi PNP0C80:02: acpi_memory_enable_device() error
> > [ 216.392187] acpi PNP0C80:02: Enumeration failure
> >
> > From document of 'mem= ' parameter, it should be a restriction during
> > boot, but not impact the system memory adding/removing after booting.
> >
> > mem=nn[KMG] [KNL,BOOT] Force usage of a specific amount of memory
> > ...
> >
> > So fix it by also checking if it's during boot-time when restrict memory
> > adding. Otherwise, skip the restriction.
> >
>
> The fix looks reasonable, but I don't get the use case. Booting with mem= is
> generally a debug option, is this for debugging memory hotplug + limited memory?
Not really. My understanding is whether 'mem= ' is for debugging or not,
it should only take effect during boot-time. Even though people add
'mem= ' for debugging only, they will have to reboot if they want to do
futher testing, e.g memory hotplug, or any other feature which makes use
of the memory hotplug mechanism to implement their functionality, like
ballon, or pmem.
Thanks
Baoquan
On 10.12.19 10:24, Balbir Singh wrote:
>
>
> On 10/12/19 7:44 pm, Baoquan He wrote:
>> In commit 357b4da50a62 ("x86: respect memory size limiting via mem=
>> parameter") a global varialbe global max_mem_size is added to store
> typo ^^^
>> the value parsed from 'mem= ', then checked when memory region is
>> added. This truly stops those DIMM from being added into system memory
>> during boot-time.
>>
>> However, it also limits the later memory hotplug functionality. Any
>> memory board can't be hot added any more if its region is beyond the
>> max_mem_size. System will print error like below:
>>
>> [ 216.387164] acpi PNP0C80:02: add_memory failed
>> [ 216.389301] acpi PNP0C80:02: acpi_memory_enable_device() error
>> [ 216.392187] acpi PNP0C80:02: Enumeration failure
>>
>> From document of 'mem= ' parameter, it should be a restriction during
>> boot, but not impact the system memory adding/removing after booting.
>>
>> mem=nn[KMG] [KNL,BOOT] Force usage of a specific amount of memory
>> ...
>>
>> So fix it by also checking if it's during boot-time when restrict memory
>> adding. Otherwise, skip the restriction.
>>
>
> The fix looks reasonable, but I don't get the use case. Booting with mem= is
> generally a debug option, is this for debugging memory hotplug + limited memory?
Some people/companies use "mem=" along with KVM e.g., to avoid
allocating memmaps for guest backing memory and to not expose it to the
buddy across kexec's. The excluded physical memory is then memmap into
the hypervisor process and KVM can deal with that. I can imagine that
hotplug might be desirable as well for such use cases.
>
> Balbir
>
--
Thanks,
David / dhildenb
On 10.12.19 10:36, David Hildenbrand wrote:
> On 10.12.19 10:24, Balbir Singh wrote:
>>
>>
>> On 10/12/19 7:44 pm, Baoquan He wrote:
>>> In commit 357b4da50a62 ("x86: respect memory size limiting via mem=
>>> parameter") a global varialbe global max_mem_size is added to store
>> typo ^^^
>>> the value parsed from 'mem= ', then checked when memory region is
>>> added. This truly stops those DIMM from being added into system memory
>>> during boot-time.
>>>
>>> However, it also limits the later memory hotplug functionality. Any
>>> memory board can't be hot added any more if its region is beyond the
>>> max_mem_size. System will print error like below:
>>>
>>> [ 216.387164] acpi PNP0C80:02: add_memory failed
>>> [ 216.389301] acpi PNP0C80:02: acpi_memory_enable_device() error
>>> [ 216.392187] acpi PNP0C80:02: Enumeration failure
>>>
>>> From document of 'mem= ' parameter, it should be a restriction during
>>> boot, but not impact the system memory adding/removing after booting.
>>>
>>> mem=nn[KMG] [KNL,BOOT] Force usage of a specific amount of memory
>>> ...
>>>
>>> So fix it by also checking if it's during boot-time when restrict memory
>>> adding. Otherwise, skip the restriction.
>>>
>>
>> The fix looks reasonable, but I don't get the use case. Booting with mem= is
>> generally a debug option, is this for debugging memory hotplug + limited memory?
>
> Some people/companies use "mem=" along with KVM e.g., to avoid
> allocating memmaps for guest backing memory and to not expose it to the
> buddy across kexec's. The excluded physical memory is then memmap into
s/memmap/mmaped/
> the hypervisor process and KVM can deal with that. I can imagine that
> hotplug might be desirable as well for such use cases.
>
>>
>> Balbir
>>
>
>
--
Thanks,
David / dhildenb
On 10/12/19 8:37 pm, David Hildenbrand wrote:
> On 10.12.19 10:36, David Hildenbrand wrote:
>> On 10.12.19 10:24, Balbir Singh wrote:
>>>
>>>
>>> On 10/12/19 7:44 pm, Baoquan He wrote:
>>>> In commit 357b4da50a62 ("x86: respect memory size limiting via mem=
>>>> parameter") a global varialbe global max_mem_size is added to store
>>> typo ^^^
>>>> the value parsed from 'mem= ', then checked when memory region is
>>>> added. This truly stops those DIMM from being added into system memory
>>>> during boot-time.
>>>>
>>>> However, it also limits the later memory hotplug functionality. Any
>>>> memory board can't be hot added any more if its region is beyond the
>>>> max_mem_size. System will print error like below:
>>>>
>>>> [ 216.387164] acpi PNP0C80:02: add_memory failed
>>>> [ 216.389301] acpi PNP0C80:02: acpi_memory_enable_device() error
>>>> [ 216.392187] acpi PNP0C80:02: Enumeration failure
>>>>
>>>> From document of 'mem= ' parameter, it should be a restriction during
>>>> boot, but not impact the system memory adding/removing after booting.
>>>>
>>>> mem=nn[KMG] [KNL,BOOT] Force usage of a specific amount of memory
>>>> ...
>>>>
>>>> So fix it by also checking if it's during boot-time when restrict memory
>>>> adding. Otherwise, skip the restriction.
>>>>
>>>
>>> The fix looks reasonable, but I don't get the use case. Booting with mem= is
>>> generally a debug option, is this for debugging memory hotplug + limited memory?
>>
>> Some people/companies use "mem=" along with KVM e.g., to avoid
>> allocating memmaps for guest backing memory and to not expose it to the
>> buddy across kexec's. The excluded physical memory is then memmap into
>
> s/memmap/mmaped/
>
>> the hypervisor process and KVM can deal with that. I can imagine that
>> hotplug might be desirable as well for such use cases.
Makes sense, sounds hacky, but it seems like mem= is no longer a debug
option
Acked-by: Balbir Singh <[email protected]>
>>
>>>
>>> Balbir
>>>
>>
>>
>
>
On Tue 10-12-19 10:36:19, David Hildenbrand wrote:
> On 10.12.19 10:24, Balbir Singh wrote:
> >
> >
> > On 10/12/19 7:44 pm, Baoquan He wrote:
> >> In commit 357b4da50a62 ("x86: respect memory size limiting via mem=
> >> parameter") a global varialbe global max_mem_size is added to store
> > typo ^^^
> >> the value parsed from 'mem= ', then checked when memory region is
> >> added. This truly stops those DIMM from being added into system memory
> >> during boot-time.
> >>
> >> However, it also limits the later memory hotplug functionality. Any
> >> memory board can't be hot added any more if its region is beyond the
> >> max_mem_size. System will print error like below:
> >>
> >> [ 216.387164] acpi PNP0C80:02: add_memory failed
> >> [ 216.389301] acpi PNP0C80:02: acpi_memory_enable_device() error
> >> [ 216.392187] acpi PNP0C80:02: Enumeration failure
> >>
> >> From document of 'mem= ' parameter, it should be a restriction during
> >> boot, but not impact the system memory adding/removing after booting.
> >>
> >> mem=nn[KMG] [KNL,BOOT] Force usage of a specific amount of memory
> >> ...
> >>
> >> So fix it by also checking if it's during boot-time when restrict memory
> >> adding. Otherwise, skip the restriction.
> >>
> >
> > The fix looks reasonable, but I don't get the use case. Booting with mem= is
> > generally a debug option, is this for debugging memory hotplug + limited memory?
>
> Some people/companies use "mem=" along with KVM e.g., to avoid
> allocating memmaps for guest backing memory and to not expose it to the
> buddy across kexec's. The excluded physical memory is then memmap into
> the hypervisor process and KVM can deal with that. I can imagine that
> hotplug might be desirable as well for such use cases.
If this is really the usecase (it makes some sense to me) then it should
be folded into the changelog. Because the real semantic is not really
clear as I've pointed out in the previous version of this patch [1].
The restriction to BOOT is documented since ever long before the memory
hotplug was a thing.
Thanks!
[1] Btw. it would have been much better if you posted the version 2 only
after all the feedback got discussed properly.
--
Michal Hocko
SUSE Labs
On 12/10/19 at 10:50am, Michal Hocko wrote:
> On Tue 10-12-19 10:36:19, David Hildenbrand wrote:
> > On 10.12.19 10:24, Balbir Singh wrote:
> > >
> > >
> > > On 10/12/19 7:44 pm, Baoquan He wrote:
> > >> In commit 357b4da50a62 ("x86: respect memory size limiting via mem=
> > >> parameter") a global varialbe global max_mem_size is added to store
> > > typo ^^^
> > >> the value parsed from 'mem= ', then checked when memory region is
> > >> added. This truly stops those DIMM from being added into system memory
> > >> during boot-time.
> > >>
> > >> However, it also limits the later memory hotplug functionality. Any
> > >> memory board can't be hot added any more if its region is beyond the
> > >> max_mem_size. System will print error like below:
> > >>
> > >> [ 216.387164] acpi PNP0C80:02: add_memory failed
> > >> [ 216.389301] acpi PNP0C80:02: acpi_memory_enable_device() error
> > >> [ 216.392187] acpi PNP0C80:02: Enumeration failure
> > >>
> > >> From document of 'mem= ' parameter, it should be a restriction during
> > >> boot, but not impact the system memory adding/removing after booting.
> > >>
> > >> mem=nn[KMG] [KNL,BOOT] Force usage of a specific amount of memory
> > >> ...
> > >>
> > >> So fix it by also checking if it's during boot-time when restrict memory
> > >> adding. Otherwise, skip the restriction.
> > >>
> > >
> > > The fix looks reasonable, but I don't get the use case. Booting with mem= is
> > > generally a debug option, is this for debugging memory hotplug + limited memory?
> >
> > Some people/companies use "mem=" along with KVM e.g., to avoid
> > allocating memmaps for guest backing memory and to not expose it to the
> > buddy across kexec's. The excluded physical memory is then memmap into
> > the hypervisor process and KVM can deal with that. I can imagine that
> > hotplug might be desirable as well for such use cases.
>
> If this is really the usecase (it makes some sense to me) then it should
> be folded into the changelog. Because the real semantic is not really
> clear as I've pointed out in the previous version of this patch [1].
> The restriction to BOOT is documented since ever long before the memory
> hotplug was a thing.
I will hold a while and post v3 to take this into log.
>
> [1] Btw. it would have been much better if you posted the version 2 only
> after all the feedback got discussed properly.
You could have replied to wrong person.
I don't know the use case David told. My motivation was to make memory hotplug
not impacted after boot. I thought I have got your question and answered
it. I will wait a little longer when post next time.
Thanks
Baoquan
On Tue 10-12-19 18:11:00, Baoquan He wrote:
> On 12/10/19 at 10:50am, Michal Hocko wrote:
[...]
> > [1] Btw. it would have been much better if you posted the version 2 only
> > after all the feedback got discussed properly.
>
> You could have replied to wrong person.
>
> I don't know the use case David told. My motivation was to make memory hotplug
> not impacted after boot. I thought I have got your question and answered
> it. I will wait a little longer when post next time.
let me reply to your original email.
--
Michal Hocko
SUSE Labs