2017-09-04 11:22:18

by Michal Hocko

[permalink] [raw]
Subject: [PATCH] mm, sparse: fix typo in online_mem_sections

From: Michal Hocko <[email protected]>

online_mem_sections accidentally marks online only the first section in
the given range. This is a typo which hasn't been noticed because I
haven't tested large 2GB blocks previously. All users of
pfn_to_online_page would get confused on the the rest of the pfn range
in the block.

All we need to fix this is to use iterator (pfn) rather than start_pfn.

Fixes: 2d070eab2e82 ("mm: consider zone which is not fully populated to have holes")
Cc: stable
Signed-off-by: Michal Hocko <[email protected]>
---
mm/sparse.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/sparse.c b/mm/sparse.c
index a9783acf2bb9..83b3bf6461af 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -626,7 +626,7 @@ void online_mem_sections(unsigned long start_pfn, unsigned long end_pfn)
unsigned long pfn;

for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
- unsigned long section_nr = pfn_to_section_nr(start_pfn);
+ unsigned long section_nr = pfn_to_section_nr(pfn);
struct mem_section *ms;

/* onlining code should never touch invalid ranges */
--
2.14.1


2017-09-05 07:02:41

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [PATCH] mm, sparse: fix typo in online_mem_sections

On 09/04/2017 04:52 PM, Michal Hocko wrote:
> From: Michal Hocko <[email protected]>
>
> online_mem_sections accidentally marks online only the first section in
> the given range. This is a typo which hasn't been noticed because I
> haven't tested large 2GB blocks previously. All users of

Section sizes are normally less than 2GB. Could you please elaborate
why this never got noticed before ?

2017-09-05 07:28:40

by Michal Hocko

[permalink] [raw]
Subject: Re: [PATCH] mm, sparse: fix typo in online_mem_sections

On Tue 05-09-17 12:32:28, Anshuman Khandual wrote:
> On 09/04/2017 04:52 PM, Michal Hocko wrote:
> > From: Michal Hocko <[email protected]>
> >
> > online_mem_sections accidentally marks online only the first section in
> > the given range. This is a typo which hasn't been noticed because I
> > haven't tested large 2GB blocks previously. All users of
>
> Section sizes are normally less than 2GB. Could you please elaborate
> why this never got noticed before ?

Section size is 128MB which is the default block size as well. So we
have one section per block. But if the amount of memory is very large
(64GB - see probe_memory_block_size) then we have a 2GB memory blocks
so multiple sections per block.
--
Michal Hocko
SUSE Labs

2017-09-05 07:37:36

by Michal Hocko

[permalink] [raw]
Subject: Re: [PATCH] mm, sparse: fix typo in online_mem_sections

On Tue 05-09-17 09:28:36, Michal Hocko wrote:
> On Tue 05-09-17 12:32:28, Anshuman Khandual wrote:
> > On 09/04/2017 04:52 PM, Michal Hocko wrote:
> > > From: Michal Hocko <[email protected]>
> > >
> > > online_mem_sections accidentally marks online only the first section in
> > > the given range. This is a typo which hasn't been noticed because I
> > > haven't tested large 2GB blocks previously. All users of
> >
> > Section sizes are normally less than 2GB. Could you please elaborate
> > why this never got noticed before ?
>
> Section size is 128MB which is the default block size as well. So we
> have one section per block. But if the amount of memory is very large
> (64GB - see probe_memory_block_size) then we have a 2GB memory blocks
> so multiple sections per block.

And just to clarify. Not that 64G would be too large but the original
patch has been merged in 4.13 so nobody probably managed to hit that
_yet_.
--
Michal Hocko
SUSE Labs

2017-09-05 08:44:53

by Anshuman Khandual

[permalink] [raw]
Subject: Re: [PATCH] mm, sparse: fix typo in online_mem_sections

On 09/05/2017 01:07 PM, Michal Hocko wrote:
> On Tue 05-09-17 09:28:36, Michal Hocko wrote:
>> On Tue 05-09-17 12:32:28, Anshuman Khandual wrote:
>>> On 09/04/2017 04:52 PM, Michal Hocko wrote:
>>>> From: Michal Hocko <[email protected]>
>>>>
>>>> online_mem_sections accidentally marks online only the first section in
>>>> the given range. This is a typo which hasn't been noticed because I
>>>> haven't tested large 2GB blocks previously. All users of
>>>
>>> Section sizes are normally less than 2GB. Could you please elaborate
>>> why this never got noticed before ?
>>
>> Section size is 128MB which is the default block size as well. So we
>> have one section per block. But if the amount of memory is very large
>> (64GB - see probe_memory_block_size) then we have a 2GB memory blocks
>> so multiple sections per block.
>
> And just to clarify. Not that 64G would be too large but the original
> patch has been merged in 4.13 so nobody probably managed to hit that
> _yet_.

Got it. Section size is 16MB and block size is 256MB on most of the
POWER platforms. Hence this could have affected them as well.

2017-09-06 08:18:23

by Vlastimil Babka

[permalink] [raw]
Subject: Re: [PATCH] mm, sparse: fix typo in online_mem_sections

On 09/04/2017 01:22 PM, Michal Hocko wrote:
> From: Michal Hocko <[email protected]>
>
> online_mem_sections accidentally marks online only the first section in
> the given range. This is a typo which hasn't been noticed because I
> haven't tested large 2GB blocks previously. All users of
> pfn_to_online_page would get confused on the the rest of the pfn range
> in the block.
>
> All we need to fix this is to use iterator (pfn) rather than start_pfn.
>
> Fixes: 2d070eab2e82 ("mm: consider zone which is not fully populated to have holes")
> Cc: stable
> Signed-off-by: Michal Hocko <[email protected]>

Acked-by: Vlastimil Babka <[email protected]>

> ---
> mm/sparse.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/sparse.c b/mm/sparse.c
> index a9783acf2bb9..83b3bf6461af 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -626,7 +626,7 @@ void online_mem_sections(unsigned long start_pfn, unsigned long end_pfn)
> unsigned long pfn;
>
> for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
> - unsigned long section_nr = pfn_to_section_nr(start_pfn);
> + unsigned long section_nr = pfn_to_section_nr(pfn);
> struct mem_section *ms;
>
> /* onlining code should never touch invalid ranges */
>