2015-11-15 11:44:08

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 0/4] m68k/mm: Add missing initialization of max_pfn and {min,max}_low_pfn

Hi,

This patch series adds missing initialization of max_pfn, min_low_pfn,
and max_low_pfn on various m68k platforms.

This was exposed by failing selftests/vm/mlock2-tests.

Note that several other architectures lack a proper initialization of
max_pfn. On some of them it's completely missing, on others max_pfn is a
local variable, hence it hides the global max_pfn, which is thus not
initialized neither.

On platforms with MMU, this can easily be verified by reading the
following virtual files (CONFIG_PROC_PAGE_MONITOR=y):

/proc/kpagecount
/proc/kpageflags
/proc/kpagecgroup (CONFIG_MEMCG=y)

If max_pfn is not initialized, all three virtual files are empty.

Besides the above, max_pfn is also used to calculate DMA masks for block
devices. An uninitialized (zero) value means all RAM is suitable for
DMA.

Absence of initialization of min_low_pfn and max_low_pfn is more subtle.
(are there any bad side-effects?).

Geert Uytterhoeven (4):
m68k/mm: motorola - Add missing initialization of max_pfn
m68k/mm: m54xx - Add missing initialization of max_pfn
m68k/mm: sun3 - Add missing initialization of max_pfn and
{min,max}_low_pfn
m68knommu: Add missing initialization of max_pfn and {min,max}_low_pfn

arch/m68k/coldfire/m54xx.c | 2 +-
arch/m68k/kernel/setup_no.c | 9 ++++++---
arch/m68k/mm/motorola.c | 2 +-
arch/m68k/sun3/config.c | 4 ++--
4 files changed, 10 insertions(+), 7 deletions(-)

--
1.9.1

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


2015-11-15 11:43:56

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 1/4] m68k/mm: motorola - Add missing initialization of max_pfn

If max_pfn is not initialized, the various /proc/kpage* files are empty,
and selftests/vm/mlock2-tests will fail. max_pfn is also used by the
block layer to calculate DMA masks.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
arch/m68k/mm/motorola.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/m68k/mm/motorola.c b/arch/m68k/mm/motorola.c
index b958916e5eac96b2..8f37fdd80be9e9cc 100644
--- a/arch/m68k/mm/motorola.c
+++ b/arch/m68k/mm/motorola.c
@@ -250,7 +250,7 @@ void __init paging_init(void)
high_memory = phys_to_virt(max_addr);

min_low_pfn = availmem >> PAGE_SHIFT;
- max_low_pfn = max_addr >> PAGE_SHIFT;
+ max_pfn = max_low_pfn = max_addr >> PAGE_SHIFT;

for (i = 0; i < m68k_num_memory; i++) {
addr = m68k_memory[i].addr;
--
1.9.1

2015-11-15 11:43:59

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 2/4] m68k/mm: m54xx - Add missing initialization of max_pfn

If max_pfn is not initialized, the various /proc/kpage* files are empty,
and selftests/vm/mlock2-tests will fail. max_pfn is also used by the
block layer to calculate DMA masks.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Compile-tested only.
---
arch/m68k/coldfire/m54xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/m68k/coldfire/m54xx.c b/arch/m68k/coldfire/m54xx.c
index f7836c6a6b60eb24..c32f76791f488ae1 100644
--- a/arch/m68k/coldfire/m54xx.c
+++ b/arch/m68k/coldfire/m54xx.c
@@ -98,7 +98,7 @@ static void __init mcf54xx_bootmem_alloc(void)
memstart = PAGE_ALIGN(_ramstart);
min_low_pfn = PFN_DOWN(_rambase);
start_pfn = PFN_DOWN(memstart);
- max_low_pfn = PFN_DOWN(_ramend);
+ max_pfn = max_low_pfn = PFN_DOWN(_ramend);
high_memory = (void *)_ramend;

m68k_virt_to_node_shift = fls(_ramend - _rambase - 1) - 6;
--
1.9.1

2015-11-15 11:44:11

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 3/4] m68k/mm: sun3 - Add missing initialization of max_pfn and {min,max}_low_pfn

If max_pfn is not initialized, the various /proc/kpage* files are empty,
and selftests/vm/mlock2-tests will fail. max_pfn is also used by the
block layer to calculate DMA masks.

Switch from init_bootmem_node() to init_bootmem(), as there's only one
memory node on Sun-3. This will initialize min_low_pfn and max_low_pfn,
which was also not done before.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Compile-tested only.
---
arch/m68k/sun3/config.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/m68k/sun3/config.c b/arch/m68k/sun3/config.c
index a8b942bf71638c97..2a5f43a68ae3d73d 100644
--- a/arch/m68k/sun3/config.c
+++ b/arch/m68k/sun3/config.c
@@ -118,13 +118,13 @@ static void __init sun3_bootmem_alloc(unsigned long memory_start,
memory_end = memory_end & PAGE_MASK;

start_page = __pa(memory_start) >> PAGE_SHIFT;
- num_pages = __pa(memory_end) >> PAGE_SHIFT;
+ max_pfn = num_pages = __pa(memory_end) >> PAGE_SHIFT;

high_memory = (void *)memory_end;
availmem = memory_start;

m68k_setup_node(0);
- availmem += init_bootmem_node(NODE_DATA(0), start_page, 0, num_pages);
+ availmem += init_bootmem(start_page, num_pages);
availmem = (availmem + (PAGE_SIZE-1)) & PAGE_MASK;

free_bootmem(__pa(availmem), memory_end - (availmem));
--
1.9.1

2015-11-15 11:44:03

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 4/4] m68knommu: Add missing initialization of max_pfn and {min,max}_low_pfn

If max_pfn is not initialized, the block layer may use wrong DMA masks.

Replace open-coded shifts by PFN_DOWN() while we're at it.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Compile-tested only.
---
arch/m68k/kernel/setup_no.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/m68k/kernel/setup_no.c b/arch/m68k/kernel/setup_no.c
index 88c27d94a7214c95..29b44e69f0f47375 100644
--- a/arch/m68k/kernel/setup_no.c
+++ b/arch/m68k/kernel/setup_no.c
@@ -238,11 +238,14 @@ void __init setup_arch(char **cmdline_p)
* Give all the memory to the bootmap allocator, tell it to put the
* boot mem_map at the start of memory.
*/
+ min_low_pfn = PFN_DOWN(memory_start);
+ max_pfn = max_low_pfn = PFN_DOWN(memory_end);
+
bootmap_size = init_bootmem_node(
NODE_DATA(0),
- memory_start >> PAGE_SHIFT, /* map goes here */
- PAGE_OFFSET >> PAGE_SHIFT, /* 0 on coldfire */
- memory_end >> PAGE_SHIFT);
+ min_low_pfn, /* map goes here */
+ PFN_DOWN(PAGE_OFFSET), /* 0 on coldfire */
+ max_pfn);
/*
* Free the usable memory, we have to make sure we do not free
* the bootmem bitmap so we then reserve it after freeing it :-)
--
1.9.1

2015-11-16 12:18:23

by Greg Ungerer

[permalink] [raw]
Subject: Re: [PATCH 4/4] m68knommu: Add missing initialization of max_pfn and {min,max}_low_pfn

Hi Geert,

On 15/11/15 21:04, Geert Uytterhoeven wrote:
> If max_pfn is not initialized, the block layer may use wrong DMA masks.
>
> Replace open-coded shifts by PFN_DOWN() while we're at it.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> Compile-tested only.
> ---
> arch/m68k/kernel/setup_no.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/m68k/kernel/setup_no.c b/arch/m68k/kernel/setup_no.c
> index 88c27d94a7214c95..29b44e69f0f47375 100644
> --- a/arch/m68k/kernel/setup_no.c
> +++ b/arch/m68k/kernel/setup_no.c
> @@ -238,11 +238,14 @@ void __init setup_arch(char **cmdline_p)
> * Give all the memory to the bootmap allocator, tell it to put the
> * boot mem_map at the start of memory.
> */
> + min_low_pfn = PFN_DOWN(memory_start);
> + max_pfn = max_low_pfn = PFN_DOWN(memory_end);
> +
> bootmap_size = init_bootmem_node(
> NODE_DATA(0),
> - memory_start >> PAGE_SHIFT, /* map goes here */
> - PAGE_OFFSET >> PAGE_SHIFT, /* 0 on coldfire */
> - memory_end >> PAGE_SHIFT);
> + min_low_pfn, /* map goes here */
> + PFN_DOWN(PAGE_OFFSET), /* 0 on coldfire */
> + max_pfn);
> /*
> * Free the usable memory, we have to make sure we do not free
> * the bootmem bitmap so we then reserve it after freeing it :-)

Should this be changed to use init_bootmem() as per your changes
in patch 3 ("m68k/mm: sun3 - Add missing initialization of max_pfn and
{min,max}_low_pfn")? For the same reason?

Regards
Greg

2015-11-16 13:06:00

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 4/4] m68knommu: Add missing initialization of max_pfn and {min,max}_low_pfn

Hi Greg,

On Mon, Nov 16, 2015 at 1:18 PM, Greg Ungerer <[email protected]> wrote:
>> --- a/arch/m68k/kernel/setup_no.c
>> +++ b/arch/m68k/kernel/setup_no.c
>> @@ -238,11 +238,14 @@ void __init setup_arch(char **cmdline_p)
>> * Give all the memory to the bootmap allocator, tell it to put
>> the
>> * boot mem_map at the start of memory.
>> */
>> + min_low_pfn = PFN_DOWN(memory_start);
>> + max_pfn = max_low_pfn = PFN_DOWN(memory_end);
>> +
>> bootmap_size = init_bootmem_node(
>> NODE_DATA(0),
>> - memory_start >> PAGE_SHIFT, /* map goes here */
>> - PAGE_OFFSET >> PAGE_SHIFT, /* 0 on coldfire
>> */
>> - memory_end >> PAGE_SHIFT);
>> + min_low_pfn, /* map goes here */
>> + PFN_DOWN(PAGE_OFFSET), /* 0 on coldfire */
>> + max_pfn);
>> /*
>> * Free the usable memory, we have to make sure we do not free
>> * the bootmem bitmap so we then reserve it after freeing it :-)
>
> Should this be changed to use init_bootmem() as per your changes
> in patch 3 ("m68k/mm: sun3 - Add missing initialization of max_pfn and
> {min,max}_low_pfn")? For the same reason?

No. PAGE_OFFSET = PAGE_OFFSET_RAW = CONFIG_RAMBASE.
As the comment says, this is zero on coldfire, but not on all m68knommu
platforms.

Upon closer look, this isn't even true on all Coldfire boards:

arch/m68k/configs/m5208evb_defconfig:CONFIG_RAMBASE=0x40000000

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2015-11-16 23:46:39

by Greg Ungerer

[permalink] [raw]
Subject: Re: [PATCH 4/4] m68knommu: Add missing initialization of max_pfn and {min,max}_low_pfn

Hi Geert,

On 16/11/15 23:05, Geert Uytterhoeven wrote:
> On Mon, Nov 16, 2015 at 1:18 PM, Greg Ungerer <[email protected]> wrote:
>>> --- a/arch/m68k/kernel/setup_no.c
>>> +++ b/arch/m68k/kernel/setup_no.c
>>> @@ -238,11 +238,14 @@ void __init setup_arch(char **cmdline_p)
>>> * Give all the memory to the bootmap allocator, tell it to put
>>> the
>>> * boot mem_map at the start of memory.
>>> */
>>> + min_low_pfn = PFN_DOWN(memory_start);
>>> + max_pfn = max_low_pfn = PFN_DOWN(memory_end);
>>> +
>>> bootmap_size = init_bootmem_node(
>>> NODE_DATA(0),
>>> - memory_start >> PAGE_SHIFT, /* map goes here */
>>> - PAGE_OFFSET >> PAGE_SHIFT, /* 0 on coldfire
>>> */
>>> - memory_end >> PAGE_SHIFT);
>>> + min_low_pfn, /* map goes here */
>>> + PFN_DOWN(PAGE_OFFSET), /* 0 on coldfire */
>>> + max_pfn);
>>> /*
>>> * Free the usable memory, we have to make sure we do not free
>>> * the bootmem bitmap so we then reserve it after freeing it :-)
>>
>> Should this be changed to use init_bootmem() as per your changes
>> in patch 3 ("m68k/mm: sun3 - Add missing initialization of max_pfn and
>> {min,max}_low_pfn")? For the same reason?
>
> No. PAGE_OFFSET = PAGE_OFFSET_RAW = CONFIG_RAMBASE.
> As the comment says, this is zero on coldfire, but not on all m68knommu
> platforms.
>
> Upon closer look, this isn't even true on all Coldfire boards:
>
> arch/m68k/configs/m5208evb_defconfig:CONFIG_RAMBASE=0x40000000

Yes, that comment is definitely wrong!

Regards
Greg

2015-11-16 23:54:15

by Greg Ungerer

[permalink] [raw]
Subject: Re: [PATCH 4/4] m68knommu: Add missing initialization of max_pfn and {min,max}_low_pfn

Hi Geert,

On 15/11/15 21:04, Geert Uytterhoeven wrote:
> If max_pfn is not initialized, the block layer may use wrong DMA masks.
>
> Replace open-coded shifts by PFN_DOWN() while we're at it.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>

Tested and working fine on m68knommu. So:

Tested-By: Greg Ungerer <[email protected]>

If you respin this patch for any reason I wouldn't object
to removing the "/* 0 on coldfire */" comment...

Regards
Greg



> ---
> Compile-tested only.
> ---
> arch/m68k/kernel/setup_no.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/m68k/kernel/setup_no.c b/arch/m68k/kernel/setup_no.c
> index 88c27d94a7214c95..29b44e69f0f47375 100644
> --- a/arch/m68k/kernel/setup_no.c
> +++ b/arch/m68k/kernel/setup_no.c
> @@ -238,11 +238,14 @@ void __init setup_arch(char **cmdline_p)
> * Give all the memory to the bootmap allocator, tell it to put the
> * boot mem_map at the start of memory.
> */
> + min_low_pfn = PFN_DOWN(memory_start);
> + max_pfn = max_low_pfn = PFN_DOWN(memory_end);
> +
> bootmap_size = init_bootmem_node(
> NODE_DATA(0),
> - memory_start >> PAGE_SHIFT, /* map goes here */
> - PAGE_OFFSET >> PAGE_SHIFT, /* 0 on coldfire */
> - memory_end >> PAGE_SHIFT);
> + min_low_pfn, /* map goes here */
> + PFN_DOWN(PAGE_OFFSET), /* 0 on coldfire */
> + max_pfn);
> /*
> * Free the usable memory, we have to make sure we do not free
> * the bootmem bitmap so we then reserve it after freeing it :-)
>

2015-11-17 00:16:13

by Greg Ungerer

[permalink] [raw]
Subject: Re: [PATCH 2/4] m68k/mm: m54xx - Add missing initialization of max_pfn

Hi Geert,

On 15/11/15 21:04, Geert Uytterhoeven wrote:
> If max_pfn is not initialized, the various /proc/kpage* files are empty,
> and selftests/vm/mlock2-tests will fail. max_pfn is also used by the
> block layer to calculate DMA masks.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> Compile-tested only.

Tested and seems to work fine on MMU ColdFire (I didn't check
the actual entries for accuracy - but /proc/kpageflags and
/proc/kpagecount look to be reporting correctly now).

Tested-by: Greg Ungerer <[email protected]>

Regards
Greg



> ---
> arch/m68k/coldfire/m54xx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/m68k/coldfire/m54xx.c b/arch/m68k/coldfire/m54xx.c
> index f7836c6a6b60eb24..c32f76791f488ae1 100644
> --- a/arch/m68k/coldfire/m54xx.c
> +++ b/arch/m68k/coldfire/m54xx.c
> @@ -98,7 +98,7 @@ static void __init mcf54xx_bootmem_alloc(void)
> memstart = PAGE_ALIGN(_ramstart);
> min_low_pfn = PFN_DOWN(_rambase);
> start_pfn = PFN_DOWN(memstart);
> - max_low_pfn = PFN_DOWN(_ramend);
> + max_pfn = max_low_pfn = PFN_DOWN(_ramend);
> high_memory = (void *)_ramend;
>
> m68k_virt_to_node_shift = fls(_ramend - _rambase - 1) - 6;
>

2015-11-17 00:17:45

by Greg Ungerer

[permalink] [raw]
Subject: Re: [PATCH 0/4] m68k/mm: Add missing initialization of max_pfn and {min,max}_low_pfn

Hi Geert,

On 15/11/15 21:04, Geert Uytterhoeven wrote:
> This patch series adds missing initialization of max_pfn, min_low_pfn,
> and max_low_pfn on various m68k platforms.
>
> This was exposed by failing selftests/vm/mlock2-tests.
>
> Note that several other architectures lack a proper initialization of
> max_pfn. On some of them it's completely missing, on others max_pfn is a
> local variable, hence it hides the global max_pfn, which is thus not
> initialized neither.
>
> On platforms with MMU, this can easily be verified by reading the
> following virtual files (CONFIG_PROC_PAGE_MONITOR=y):
>
> /proc/kpagecount
> /proc/kpageflags
> /proc/kpagecgroup (CONFIG_MEMCG=y)
>
> If max_pfn is not initialized, all three virtual files are empty.
>
> Besides the above, max_pfn is also used to calculate DMA masks for block
> devices. An uninitialized (zero) value means all RAM is suitable for
> DMA.

All looks good to me. Tested-by acks set separately. But otherwise

Acked-by: Greg Ungerer <[email protected]>

Regards
Greg


> Absence of initialization of min_low_pfn and max_low_pfn is more subtle.
> (are there any bad side-effects?).
>
> Geert Uytterhoeven (4):
> m68k/mm: motorola - Add missing initialization of max_pfn
> m68k/mm: m54xx - Add missing initialization of max_pfn
> m68k/mm: sun3 - Add missing initialization of max_pfn and
> {min,max}_low_pfn
> m68knommu: Add missing initialization of max_pfn and {min,max}_low_pfn
>
> arch/m68k/coldfire/m54xx.c | 2 +-
> arch/m68k/kernel/setup_no.c | 9 ++++++---
> arch/m68k/mm/motorola.c | 2 +-
> arch/m68k/sun3/config.c | 4 ++--
> 4 files changed, 10 insertions(+), 7 deletions(-)
>

2015-11-22 10:38:11

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 4/4] m68knommu: Add missing initialization of max_pfn and {min,max}_low_pfn

Hi Greg,

On Tue, Nov 17, 2015 at 12:54 AM, Greg Ungerer <[email protected]> wrote:
> On 15/11/15 21:04, Geert Uytterhoeven wrote:
>> If max_pfn is not initialized, the block layer may use wrong DMA masks.
>>
>> Replace open-coded shifts by PFN_DOWN() while we're at it.
>>
>> Signed-off-by: Geert Uytterhoeven <[email protected]>
>
> Tested and working fine on m68knommu. So:
>
> Tested-By: Greg Ungerer <[email protected]>
>
> If you respin this patch for any reason I wouldn't object
> to removing the "/* 0 on coldfire */" comment...

Thanks, done, applied, and queued for v4.4.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds