Let's use a calculation that's easier to understand and calculates the
same result. Reusing existing macros makes this look nicer.
We always want to have the number of pages (> 0) to the next section
boundary, starting from the current pfn.
Suggested-by: Segher Boessenkool <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Oscar Salvador <[email protected]>
Cc: Baoquan He <[email protected]>
Cc: Wei Yang <[email protected]>
Signed-off-by: David Hildenbrand <[email protected]>
---
mm/memory_hotplug.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 0a54ffac8c68..c30191183c04 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -528,7 +528,8 @@ void __remove_pages(unsigned long pfn, unsigned long nr_pages,
for (; pfn < end_pfn; pfn += cur_nr_pages) {
cond_resched();
/* Select all remaining pages up to the next section boundary */
- cur_nr_pages = min(end_pfn - pfn, -(pfn | PAGE_SECTION_MASK));
+ cur_nr_pages = min(end_pfn - pfn,
+ SECTION_ALIGN_UP(pfn + 1) - pfn);
__remove_section(pfn, cur_nr_pages, map_offset, altmap);
map_offset = 0;
}
--
2.24.1
On Wed, Feb 05, 2020 at 02:52:51PM +0100, David Hildenbrand wrote:
>Let's use a calculation that's easier to understand and calculates the
>same result. Reusing existing macros makes this look nicer.
>
>We always want to have the number of pages (> 0) to the next section
>boundary, starting from the current pfn.
>
>Suggested-by: Segher Boessenkool <[email protected]>
>Cc: Andrew Morton <[email protected]>
>Cc: Michal Hocko <[email protected]>
>Cc: Oscar Salvador <[email protected]>
>Cc: Baoquan He <[email protected]>
>Cc: Wei Yang <[email protected]>
>Signed-off-by: David Hildenbrand <[email protected]>
Reviewed-by: Wei Yang <[email protected]>
BTW, I got one question about hotplug size requirement.
I thought the hotplug range should be section size aligned, while taking a
look into current code function check_hotplug_memory_range() guard the range.
This function says the range should be block_size aligned. And if I am
correct, block size on x86 should be in the range
[MIN_MEMORY_BLOCK_SIZE, MEM_SIZE_FOR_LARGE_BLOCK]
And MIN_MEMORY_BLOCK_SIZE is section size.
Seems currently we support subsection hotplug? Then how a subsection range got
hotplug? Or this patch is a pre-requisite?
>---
> mm/memory_hotplug.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>index 0a54ffac8c68..c30191183c04 100644
>--- a/mm/memory_hotplug.c
>+++ b/mm/memory_hotplug.c
>@@ -528,7 +528,8 @@ void __remove_pages(unsigned long pfn, unsigned long nr_pages,
> for (; pfn < end_pfn; pfn += cur_nr_pages) {
> cond_resched();
> /* Select all remaining pages up to the next section boundary */
>- cur_nr_pages = min(end_pfn - pfn, -(pfn | PAGE_SECTION_MASK));
>+ cur_nr_pages = min(end_pfn - pfn,
>+ SECTION_ALIGN_UP(pfn + 1) - pfn);
> __remove_section(pfn, cur_nr_pages, map_offset, altmap);
> map_offset = 0;
> }
>--
>2.24.1
--
Wei Yang
Help you, Help me
On Thu, Feb 06, 2020 at 07:19:45AM +0800, Wei Yang wrote:
>On Wed, Feb 05, 2020 at 02:52:51PM +0100, David Hildenbrand wrote:
>>Let's use a calculation that's easier to understand and calculates the
>>same result. Reusing existing macros makes this look nicer.
>>
>>We always want to have the number of pages (> 0) to the next section
>>boundary, starting from the current pfn.
>>
>>Suggested-by: Segher Boessenkool <[email protected]>
>>Cc: Andrew Morton <[email protected]>
>>Cc: Michal Hocko <[email protected]>
>>Cc: Oscar Salvador <[email protected]>
>>Cc: Baoquan He <[email protected]>
>>Cc: Wei Yang <[email protected]>
>>Signed-off-by: David Hildenbrand <[email protected]>
>
>Reviewed-by: Wei Yang <[email protected]>
>
>BTW, I got one question about hotplug size requirement.
>
>I thought the hotplug range should be section size aligned, while taking a
>look into current code function check_hotplug_memory_range() guard the range.
>
>This function says the range should be block_size aligned. And if I am
>correct, block size on x86 should be in the range
>
> [MIN_MEMORY_BLOCK_SIZE, MEM_SIZE_FOR_LARGE_BLOCK]
>
>And MIN_MEMORY_BLOCK_SIZE is section size.
>
>Seems currently we support subsection hotplug? Then how a subsection range got
>hotplug? Or this patch is a pre-requisite?
>
One more question is we support hot-add subsection memory but not support
hot-online subsection memory.
Is my understanding correct?
--
Wei Yang
Help you, Help me
On 02/06/20 at 07:50am, Wei Yang wrote:
> On Thu, Feb 06, 2020 at 07:19:45AM +0800, Wei Yang wrote:
> >On Wed, Feb 05, 2020 at 02:52:51PM +0100, David Hildenbrand wrote:
> >>Let's use a calculation that's easier to understand and calculates the
> >>same result. Reusing existing macros makes this look nicer.
> >>
> >>We always want to have the number of pages (> 0) to the next section
> >>boundary, starting from the current pfn.
> >>
> >>Suggested-by: Segher Boessenkool <[email protected]>
> >>Cc: Andrew Morton <[email protected]>
> >>Cc: Michal Hocko <[email protected]>
> >>Cc: Oscar Salvador <[email protected]>
> >>Cc: Baoquan He <[email protected]>
> >>Cc: Wei Yang <[email protected]>
> >>Signed-off-by: David Hildenbrand <[email protected]>
> >
> >Reviewed-by: Wei Yang <[email protected]>
> >
> >BTW, I got one question about hotplug size requirement.
> >
> >I thought the hotplug range should be section size aligned, while taking a
> >look into current code function check_hotplug_memory_range() guard the range.
A good question. The current code should be block size aligned. I
remember in some places we assume each block comprise all the sections.
Can't imagine one or some of them are half section filled.
It truly has a risk that system ram is very huge to make the block
size is 2G, someone try to insert a 1G memory board. However, it should
only exist in experiment environment, e.g build a guest with enough ram,
then hot add 1G DIMM. In reality, we don't need to worry about it, at
least what I saw is 512G order of magnitude.
> >
> >This function says the range should be block_size aligned. And if I am
> >correct, block size on x86 should be in the range
> >
> > [MIN_MEMORY_BLOCK_SIZE, MEM_SIZE_FOR_LARGE_BLOCK]
> >
> >And MIN_MEMORY_BLOCK_SIZE is section size.
No, if I got it right, the range on x86 is
[MIN_MEMORY_BLOCK_SIZE, MAX_BLOCK_SIZE].
MEM_SIZE_FOR_LARGE_BLOCK is the starting point from which block size can
be adjusted. Otherwise it's MIN_MEMORY_BLOCK_SIZE.
/* Amount of ram needed to start using large blocks */
#define MEM_SIZE_FOR_LARGE_BLOCK (64UL << 30)
> >
> >Seems currently we support subsection hotplug? Then how a subsection range got
> >hotplug? Or this patch is a pre-requisite?
The sub-section hotplug feature was added by your colleague Dan
Williams. It intends to fix a nvdimms issue that nvdimms device could be
mapped into a non section size aligned starting address. And nvdimms
makes use of the existing memory hotplug mechanism to manage pages.
Not sure if we are saying the same thing.
> >
>
> One more question is we support hot-add subsection memory but not support
> hot-online subsection memory.
>
> Is my understanding correct?
>
> --
> Wei Yang
> Help you, Help me
>
On 02/06/20 at 08:13am, Baoquan He wrote:
> On 02/06/20 at 07:50am, Wei Yang wrote:
> > On Thu, Feb 06, 2020 at 07:19:45AM +0800, Wei Yang wrote:
> > >On Wed, Feb 05, 2020 at 02:52:51PM +0100, David Hildenbrand wrote:
> > >>Let's use a calculation that's easier to understand and calculates the
> > >>same result. Reusing existing macros makes this look nicer.
> > >>
> > >>We always want to have the number of pages (> 0) to the next section
> > >>boundary, starting from the current pfn.
> > >>
> > >>Suggested-by: Segher Boessenkool <[email protected]>
> > >>Cc: Andrew Morton <[email protected]>
> > >>Cc: Michal Hocko <[email protected]>
> > >>Cc: Oscar Salvador <[email protected]>
> > >>Cc: Baoquan He <[email protected]>
> > >>Cc: Wei Yang <[email protected]>
> > >>Signed-off-by: David Hildenbrand <[email protected]>
> > >
> > >Reviewed-by: Wei Yang <[email protected]>
> > >
> > >BTW, I got one question about hotplug size requirement.
> > >
> > >I thought the hotplug range should be section size aligned, while taking a
> > >look into current code function check_hotplug_memory_range() guard the range.
>
> A good question. The current code should be block size aligned. I
> remember in some places we assume each block comprise all the sections.
> Can't imagine one or some of them are half section filled.
I could be wrong, half filled block may not cause problem.
>
> It truly has a risk that system ram is very huge to make the block
> size is 2G, someone try to insert a 1G memory board. However, it should
> only exist in experiment environment, e.g build a guest with enough ram,
> then hot add 1G DIMM. In reality, we don't need to worry about it, at
> least what I saw is 512G order of magnitude.
>
> > >
> > >This function says the range should be block_size aligned. And if I am
> > >correct, block size on x86 should be in the range
> > >
> > > [MIN_MEMORY_BLOCK_SIZE, MEM_SIZE_FOR_LARGE_BLOCK]
> > >
> > >And MIN_MEMORY_BLOCK_SIZE is section size.
>
> No, if I got it right, the range on x86 is
> [MIN_MEMORY_BLOCK_SIZE, MAX_BLOCK_SIZE].
>
> MEM_SIZE_FOR_LARGE_BLOCK is the starting point from which block size can
> be adjusted. Otherwise it's MIN_MEMORY_BLOCK_SIZE.
>
> /* Amount of ram needed to start using large blocks */
> #define MEM_SIZE_FOR_LARGE_BLOCK (64UL << 30)
>
> > >
> > >Seems currently we support subsection hotplug? Then how a subsection range got
> > >hotplug? Or this patch is a pre-requisite?
>
> The sub-section hotplug feature was added by your colleague Dan
> Williams. It intends to fix a nvdimms issue that nvdimms device could be
> mapped into a non section size aligned starting address. And nvdimms
> makes use of the existing memory hotplug mechanism to manage pages.
> Not sure if we are saying the same thing.
>
> > >
> >
> > One more question is we support hot-add subsection memory but not support
> > hot-online subsection memory.
> >
> > Is my understanding correct?
> >
> > --
> > Wei Yang
> > Help you, Help me
> >
On 02/05/20 at 02:52pm, David Hildenbrand wrote:
> Let's use a calculation that's easier to understand and calculates the
> same result. Reusing existing macros makes this look nicer.
>
> We always want to have the number of pages (> 0) to the next section
> boundary, starting from the current pfn.
>
> Suggested-by: Segher Boessenkool <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Michal Hocko <[email protected]>
> Cc: Oscar Salvador <[email protected]>
> Cc: Baoquan He <[email protected]>
> Cc: Wei Yang <[email protected]>
> Signed-off-by: David Hildenbrand <[email protected]>
LGTM.
Reviewed-by: Baoquan He <[email protected]>
> ---
> mm/memory_hotplug.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 0a54ffac8c68..c30191183c04 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -528,7 +528,8 @@ void __remove_pages(unsigned long pfn, unsigned long nr_pages,
> for (; pfn < end_pfn; pfn += cur_nr_pages) {
> cond_resched();
> /* Select all remaining pages up to the next section boundary */
> - cur_nr_pages = min(end_pfn - pfn, -(pfn | PAGE_SECTION_MASK));
> + cur_nr_pages = min(end_pfn - pfn,
> + SECTION_ALIGN_UP(pfn + 1) - pfn);
> __remove_section(pfn, cur_nr_pages, map_offset, altmap);
> map_offset = 0;
> }
> --
> 2.24.1
>
On Thu, Feb 06, 2020 at 08:37:36AM +0800, Baoquan He wrote:
>On 02/06/20 at 08:13am, Baoquan He wrote:
>> On 02/06/20 at 07:50am, Wei Yang wrote:
>> > On Thu, Feb 06, 2020 at 07:19:45AM +0800, Wei Yang wrote:
>> > >On Wed, Feb 05, 2020 at 02:52:51PM +0100, David Hildenbrand wrote:
>> > >>Let's use a calculation that's easier to understand and calculates the
>> > >>same result. Reusing existing macros makes this look nicer.
>> > >>
>> > >>We always want to have the number of pages (> 0) to the next section
>> > >>boundary, starting from the current pfn.
>> > >>
>> > >>Suggested-by: Segher Boessenkool <[email protected]>
>> > >>Cc: Andrew Morton <[email protected]>
>> > >>Cc: Michal Hocko <[email protected]>
>> > >>Cc: Oscar Salvador <[email protected]>
>> > >>Cc: Baoquan He <[email protected]>
>> > >>Cc: Wei Yang <[email protected]>
>> > >>Signed-off-by: David Hildenbrand <[email protected]>
>> > >
>> > >Reviewed-by: Wei Yang <[email protected]>
>> > >
>> > >BTW, I got one question about hotplug size requirement.
>> > >
>> > >I thought the hotplug range should be section size aligned, while taking a
>> > >look into current code function check_hotplug_memory_range() guard the range.
>>
>> A good question. The current code should be block size aligned. I
>> remember in some places we assume each block comprise all the sections.
>> Can't imagine one or some of them are half section filled.
>
>I could be wrong, half filled block may not cause problem.
>
David must be angry about our flooding the mail list :-)
Check the code again, there are two memory range check:
* check_hotplug_memory_range(), block/section aligned
* check_pfn_span(), subsection aligned
The second check, check_pfn_span() in __add_pages(), enable the capability to
add a memory range with subsection size.
This means hotplug still keeps section alignment.
BTW, __add_pages() share the same logic as __remove_pages(). Why not change it
too? Do I miss something or I don't have the latest source code?
--
Wei Yang
Help you, Help me
On 02/06/20 at 02:26am, Wei Yang wrote:
> On Thu, Feb 06, 2020 at 08:37:36AM +0800, Baoquan He wrote:
> >On 02/06/20 at 08:13am, Baoquan He wrote:
> >> On 02/06/20 at 07:50am, Wei Yang wrote:
> >> > On Thu, Feb 06, 2020 at 07:19:45AM +0800, Wei Yang wrote:
> >> > >On Wed, Feb 05, 2020 at 02:52:51PM +0100, David Hildenbrand wrote:
> >> > >>Let's use a calculation that's easier to understand and calculates the
> >> > >>same result. Reusing existing macros makes this look nicer.
> >> > >>
> >> > >>We always want to have the number of pages (> 0) to the next section
> >> > >>boundary, starting from the current pfn.
> >> > >>
> >> > >>Suggested-by: Segher Boessenkool <[email protected]>
> >> > >>Cc: Andrew Morton <[email protected]>
> >> > >>Cc: Michal Hocko <[email protected]>
> >> > >>Cc: Oscar Salvador <[email protected]>
> >> > >>Cc: Baoquan He <[email protected]>
> >> > >>Cc: Wei Yang <[email protected]>
> >> > >>Signed-off-by: David Hildenbrand <[email protected]>
> >> > >
> >> > >Reviewed-by: Wei Yang <[email protected]>
> >> > >
> >> > >BTW, I got one question about hotplug size requirement.
> >> > >
> >> > >I thought the hotplug range should be section size aligned, while taking a
> >> > >look into current code function check_hotplug_memory_range() guard the range.
> >>
> >> A good question. The current code should be block size aligned. I
> >> remember in some places we assume each block comprise all the sections.
> >> Can't imagine one or some of them are half section filled.
> >
> >I could be wrong, half filled block may not cause problem.
> >
>
> David must be angry about our flooding the mail list :-)
Believe he won't, :-) If you like, we can talk off line.
>
> Check the code again, there are two memory range check:
>
> * check_hotplug_memory_range(), block/section aligned
> * check_pfn_span(), subsection aligned
>
> The second check, check_pfn_span() in __add_pages(), enable the capability to
> add a memory range with subsection size.
>
> This means hotplug still keeps section alignment.
memremap_pages() also call add_pages(), it doesn't have the
check_hotplug_memory_range() invocation. check_pfn_span() is made for
it specifically.
>
> BTW, __add_pages() share the same logic as __remove_pages(). Why not change it
> too? Do I miss something or I don't have the latest source code?
Good question, and I think it need. Just David is refactoring/cleaning
up the remove_pages() code path, this is found out by Segher from patch
reviewing.
On Thu, Feb 06, 2020 at 10:48:16AM +0800, Baoquan He wrote:
>On 02/06/20 at 02:26am, Wei Yang wrote:
>> On Thu, Feb 06, 2020 at 08:37:36AM +0800, Baoquan He wrote:
>> >On 02/06/20 at 08:13am, Baoquan He wrote:
>> >> On 02/06/20 at 07:50am, Wei Yang wrote:
>> >> > On Thu, Feb 06, 2020 at 07:19:45AM +0800, Wei Yang wrote:
>> >> > >On Wed, Feb 05, 2020 at 02:52:51PM +0100, David Hildenbrand wrote:
>> >> > >>Let's use a calculation that's easier to understand and calculates the
>> >> > >>same result. Reusing existing macros makes this look nicer.
>> >> > >>
>> >> > >>We always want to have the number of pages (> 0) to the next section
>> >> > >>boundary, starting from the current pfn.
>> >> > >>
>> >> > >>Suggested-by: Segher Boessenkool <[email protected]>
>> >> > >>Cc: Andrew Morton <[email protected]>
>> >> > >>Cc: Michal Hocko <[email protected]>
>> >> > >>Cc: Oscar Salvador <[email protected]>
>> >> > >>Cc: Baoquan He <[email protected]>
>> >> > >>Cc: Wei Yang <[email protected]>
>> >> > >>Signed-off-by: David Hildenbrand <[email protected]>
>> >> > >
>> >> > >Reviewed-by: Wei Yang <[email protected]>
>> >> > >
>> >> > >BTW, I got one question about hotplug size requirement.
>> >> > >
>> >> > >I thought the hotplug range should be section size aligned, while taking a
>> >> > >look into current code function check_hotplug_memory_range() guard the range.
>> >>
>> >> A good question. The current code should be block size aligned. I
>> >> remember in some places we assume each block comprise all the sections.
>> >> Can't imagine one or some of them are half section filled.
>> >
>> >I could be wrong, half filled block may not cause problem.
>> >
>>
>> David must be angry about our flooding the mail list :-)
>
>Believe he won't, :-) If you like, we can talk off line.
>
>>
>> Check the code again, there are two memory range check:
>>
>> * check_hotplug_memory_range(), block/section aligned
>> * check_pfn_span(), subsection aligned
>>
>> The second check, check_pfn_span() in __add_pages(), enable the capability to
>> add a memory range with subsection size.
>>
>> This means hotplug still keeps section alignment.
>
>memremap_pages() also call add_pages(), it doesn't have the
>check_hotplug_memory_range() invocation. check_pfn_span() is made for
>it specifically.
>
If my understanding is correct, memremap_pages() is used to add some dev
memory to system. This is the use case which Dan want to enable for
sub-section. Since memremap_pages() is not called in mem-hotplug path, this
doesn't affect the hotplug range alignment.
>>
>> BTW, __add_pages() share the same logic as __remove_pages(). Why not change it
>> too? Do I miss something or I don't have the latest source code?
>
>Good question, and I think it need. Just David is refactoring/cleaning
>up the remove_pages() code path, this is found out by Segher from patch
>reviewing.
Ah, we may need a following cleanup :-)
--
Wei Yang
Help you, Help me
On 02/06/20 at 04:34am, Wei Yang wrote:
> On Thu, Feb 06, 2020 at 10:48:16AM +0800, Baoquan He wrote:
> >On 02/06/20 at 02:26am, Wei Yang wrote:
> >> On Thu, Feb 06, 2020 at 08:37:36AM +0800, Baoquan He wrote:
> >> >On 02/06/20 at 08:13am, Baoquan He wrote:
> >> >> On 02/06/20 at 07:50am, Wei Yang wrote:
> >> >> > On Thu, Feb 06, 2020 at 07:19:45AM +0800, Wei Yang wrote:
> >> >> > >On Wed, Feb 05, 2020 at 02:52:51PM +0100, David Hildenbrand wrote:
> >> >> > >>Let's use a calculation that's easier to understand and calculates the
> >> >> > >>same result. Reusing existing macros makes this look nicer.
> >> >> > >>
> >> >> > >>We always want to have the number of pages (> 0) to the next section
> >> >> > >>boundary, starting from the current pfn.
> >> >> > >>
> >> >> > >>Suggested-by: Segher Boessenkool <[email protected]>
> >> >> > >>Cc: Andrew Morton <[email protected]>
> >> >> > >>Cc: Michal Hocko <[email protected]>
> >> >> > >>Cc: Oscar Salvador <[email protected]>
> >> >> > >>Cc: Baoquan He <[email protected]>
> >> >> > >>Cc: Wei Yang <[email protected]>
> >> >> > >>Signed-off-by: David Hildenbrand <[email protected]>
> >> >> > >
> >> >> > >Reviewed-by: Wei Yang <[email protected]>
> >> >> > >
> >> >> > >BTW, I got one question about hotplug size requirement.
> >> >> > >
> >> >> > >I thought the hotplug range should be section size aligned, while taking a
> >> >> > >look into current code function check_hotplug_memory_range() guard the range.
> >> >>
> >> >> A good question. The current code should be block size aligned. I
> >> >> remember in some places we assume each block comprise all the sections.
> >> >> Can't imagine one or some of them are half section filled.
> >> >
> >> >I could be wrong, half filled block may not cause problem.
> >> >
> >>
> >> David must be angry about our flooding the mail list :-)
> >
> >Believe he won't, :-) If you like, we can talk off line.
> >
> >>
> >> Check the code again, there are two memory range check:
> >>
> >> * check_hotplug_memory_range(), block/section aligned
> >> * check_pfn_span(), subsection aligned
> >>
> >> The second check, check_pfn_span() in __add_pages(), enable the capability to
> >> add a memory range with subsection size.
> >>
> >> This means hotplug still keeps section alignment.
> >
> >memremap_pages() also call add_pages(), it doesn't have the
> >check_hotplug_memory_range() invocation. check_pfn_span() is made for
> >it specifically.
> >
>
> If my understanding is correct, memremap_pages() is used to add some dev
> memory to system. This is the use case which Dan want to enable for
> sub-section. Since memremap_pages() is not called in mem-hotplug path, this
> doesn't affect the hotplug range alignment.
Yeah, we are on the same page.
>
> >>
> >> BTW, __add_pages() share the same logic as __remove_pages(). Why not change it
> >> too? Do I miss something or I don't have the latest source code?
> >
> >Good question, and I think it need. Just David is refactoring/cleaning
> >up the remove_pages() code path, this is found out by Segher from patch
> >reviewing.
>
> Ah, we may need a following cleanup :-)
Agree. See what David will say. Fold it into this patch, or anyone post
a new one.
On 06.02.20 05:39, Baoquan He wrote:
> On 02/06/20 at 04:34am, Wei Yang wrote:
>> On Thu, Feb 06, 2020 at 10:48:16AM +0800, Baoquan He wrote:
>>> On 02/06/20 at 02:26am, Wei Yang wrote:
>>>> On Thu, Feb 06, 2020 at 08:37:36AM +0800, Baoquan He wrote:
>>>>> On 02/06/20 at 08:13am, Baoquan He wrote:
>>>>>> On 02/06/20 at 07:50am, Wei Yang wrote:
>>>>>>> On Thu, Feb 06, 2020 at 07:19:45AM +0800, Wei Yang wrote:
>>>>>>>> On Wed, Feb 05, 2020 at 02:52:51PM +0100, David Hildenbrand wrote:
>>>>>>>>> Let's use a calculation that's easier to understand and calculates the
>>>>>>>>> same result. Reusing existing macros makes this look nicer.
>>>>>>>>>
>>>>>>>>> We always want to have the number of pages (> 0) to the next section
>>>>>>>>> boundary, starting from the current pfn.
>>>>>>>>>
>>>>>>>>> Suggested-by: Segher Boessenkool <[email protected]>
>>>>>>>>> Cc: Andrew Morton <[email protected]>
>>>>>>>>> Cc: Michal Hocko <[email protected]>
>>>>>>>>> Cc: Oscar Salvador <[email protected]>
>>>>>>>>> Cc: Baoquan He <[email protected]>
>>>>>>>>> Cc: Wei Yang <[email protected]>
>>>>>>>>> Signed-off-by: David Hildenbrand <[email protected]>
>>>>>>>>
>>>>>>>> Reviewed-by: Wei Yang <[email protected]>
>>>>>>>>
>>>>>>>> BTW, I got one question about hotplug size requirement.
>>>>>>>>
>>>>>>>> I thought the hotplug range should be section size aligned, while taking a
>>>>>>>> look into current code function check_hotplug_memory_range() guard the range.
>>>>>>
>>>>>> A good question. The current code should be block size aligned. I
>>>>>> remember in some places we assume each block comprise all the sections.
>>>>>> Can't imagine one or some of them are half section filled.
>>>>>
>>>>> I could be wrong, half filled block may not cause problem.
>>>>>
>>>>
>>>> David must be angry about our flooding the mail list :-)
>>>
>>> Believe he won't, :-) If you like, we can talk off line.
>>>
>>>>
>>>> Check the code again, there are two memory range check:
>>>>
>>>> * check_hotplug_memory_range(), block/section aligned
>>>> * check_pfn_span(), subsection aligned
>>>>
>>>> The second check, check_pfn_span() in __add_pages(), enable the capability to
>>>> add a memory range with subsection size.
>>>>
>>>> This means hotplug still keeps section alignment.
>>>
>>> memremap_pages() also call add_pages(), it doesn't have the
>>> check_hotplug_memory_range() invocation. check_pfn_span() is made for
>>> it specifically.
>>>
>>
>> If my understanding is correct, memremap_pages() is used to add some dev
>> memory to system. This is the use case which Dan want to enable for
>> sub-section. Since memremap_pages() is not called in mem-hotplug path, this
>> doesn't affect the hotplug range alignment.
>
> Yeah, we are on the same page.
We allow sub-section hoy-add only for device memory/hmm. BIOS often
align PMEM devices to sub-sections, and not supporting this makes life
difficult to support these devices. (You cannot simply cut off something
of a disk :) )
System memory can only be hotplugged in memory block granularity, the
same granularity onlining/offlining from user space will happen. Boot
memory, however, can be sub-section aligned, but can never be offlined
(as it contains holes) and therefore never be removed.
>>
>>>>
>>>> BTW, __add_pages() share the same logic as __remove_pages(). Why not change it
>>>> too? Do I miss something or I don't have the latest source code?
>>>
>>> Good question, and I think it need. Just David is refactoring/cleaning
>>> up the remove_pages() code path, this is found out by Segher from patch
>>> reviewing.
>>
>> Ah, we may need a following cleanup :-)
>
> Agree. See what David will say. Fold it into this patch, or anyone post
> a new one.
>
Yes, I only cleaned up __remove_pages() for now. I can send a similar
cleanup for __add_pages().
Will resend this patch, also taking care of __add_pages(). Thanks!
--
Thanks,
David / dhildenb
On Thu, Feb 06, 2020 at 10:01:21AM +0100, David Hildenbrand wrote:
>On 06.02.20 05:39, Baoquan He wrote:
>> On 02/06/20 at 04:34am, Wei Yang wrote:
>>> On Thu, Feb 06, 2020 at 10:48:16AM +0800, Baoquan He wrote:
>>>> On 02/06/20 at 02:26am, Wei Yang wrote:
>>>>> On Thu, Feb 06, 2020 at 08:37:36AM +0800, Baoquan He wrote:
>>>>>> On 02/06/20 at 08:13am, Baoquan He wrote:
>>>>>>> On 02/06/20 at 07:50am, Wei Yang wrote:
>>>>>>>> On Thu, Feb 06, 2020 at 07:19:45AM +0800, Wei Yang wrote:
>>>>>>>>> On Wed, Feb 05, 2020 at 02:52:51PM +0100, David Hildenbrand wrote:
>>>>>>>>>> Let's use a calculation that's easier to understand and calculates the
>>>>>>>>>> same result. Reusing existing macros makes this look nicer.
>>>>>>>>>>
>>>>>>>>>> We always want to have the number of pages (> 0) to the next section
>>>>>>>>>> boundary, starting from the current pfn.
>>>>>>>>>>
>>>>>>>>>> Suggested-by: Segher Boessenkool <[email protected]>
>>>>>>>>>> Cc: Andrew Morton <[email protected]>
>>>>>>>>>> Cc: Michal Hocko <[email protected]>
>>>>>>>>>> Cc: Oscar Salvador <[email protected]>
>>>>>>>>>> Cc: Baoquan He <[email protected]>
>>>>>>>>>> Cc: Wei Yang <[email protected]>
>>>>>>>>>> Signed-off-by: David Hildenbrand <[email protected]>
>>>>>>>>>
>>>>>>>>> Reviewed-by: Wei Yang <[email protected]>
>>>>>>>>>
>>>>>>>>> BTW, I got one question about hotplug size requirement.
>>>>>>>>>
>>>>>>>>> I thought the hotplug range should be section size aligned, while taking a
>>>>>>>>> look into current code function check_hotplug_memory_range() guard the range.
>>>>>>>
>>>>>>> A good question. The current code should be block size aligned. I
>>>>>>> remember in some places we assume each block comprise all the sections.
>>>>>>> Can't imagine one or some of them are half section filled.
>>>>>>
>>>>>> I could be wrong, half filled block may not cause problem.
>>>>>>
>>>>>
>>>>> David must be angry about our flooding the mail list :-)
>>>>
>>>> Believe he won't, :-) If you like, we can talk off line.
>>>>
>>>>>
>>>>> Check the code again, there are two memory range check:
>>>>>
>>>>> * check_hotplug_memory_range(), block/section aligned
>>>>> * check_pfn_span(), subsection aligned
>>>>>
>>>>> The second check, check_pfn_span() in __add_pages(), enable the capability to
>>>>> add a memory range with subsection size.
>>>>>
>>>>> This means hotplug still keeps section alignment.
>>>>
>>>> memremap_pages() also call add_pages(), it doesn't have the
>>>> check_hotplug_memory_range() invocation. check_pfn_span() is made for
>>>> it specifically.
>>>>
>>>
>>> If my understanding is correct, memremap_pages() is used to add some dev
>>> memory to system. This is the use case which Dan want to enable for
>>> sub-section. Since memremap_pages() is not called in mem-hotplug path, this
>>> doesn't affect the hotplug range alignment.
>>
>> Yeah, we are on the same page.
>
>We allow sub-section hoy-add only for device memory/hmm. BIOS often
>align PMEM devices to sub-sections, and not supporting this makes life
>difficult to support these devices. (You cannot simply cut off something
>of a disk :) )
>
>System memory can only be hotplugged in memory block granularity, the
>same granularity onlining/offlining from user space will happen. Boot
>memory, however, can be sub-section aligned, but can never be offlined
>(as it contains holes) and therefore never be removed.
>
This makes life easier.
Thanks for your explanation.
--
Wei Yang
Help you, Help me