2020-10-27 14:41:59

by Mike Rapoport

[permalink] [raw]
Subject: [PATCH 11/13] m68k/mm: make node data and node setup depend on CONFIG_DISCONTIGMEM

From: Mike Rapoport <[email protected]>

The pg_data_t node structures and their initialization currently depends on
!CONFIG_SINGLE_MEMORY_CHUNK. Since they are required only for DISCONTIGMEM
make this dependency explicit and replace usage of
CONFIG_SINGLE_MEMORY_CHUNK with CONFIG_DISCONTIGMEM where appropriate.

The CONFIG_SINGLE_MEMORY_CHUNK was implicitly disabled on the ColdFire MMU
variant, although it always presumed a single memory bank. As there is no
actual need for DISCONTIGMEM in this case, make sure that ColdFire MMU
systems set CONFIG_SINGLE_MEMORY_CHUNK to 'y'.

Signed-off-by: Mike Rapoport <[email protected]>
---
arch/m68k/Kconfig.cpu | 6 +++---
arch/m68k/include/asm/page_mm.h | 2 +-
arch/m68k/mm/init.c | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/m68k/Kconfig.cpu b/arch/m68k/Kconfig.cpu
index 694c4fca9f5d..3af0fca03803 100644
--- a/arch/m68k/Kconfig.cpu
+++ b/arch/m68k/Kconfig.cpu
@@ -20,6 +20,7 @@ choice

config M68KCLASSIC
bool "Classic M68K CPU family support"
+ select NEED_MULTIPLE_NODES if DISCONTIGMEM

config COLDFIRE
bool "Coldfire CPU family support"
@@ -373,8 +374,7 @@ config RMW_INSNS
config SINGLE_MEMORY_CHUNK
bool "Use one physical chunk of memory only" if ADVANCED && !SUN3
depends on MMU
- default y if SUN3
- select NEED_MULTIPLE_NODES
+ default y if SUN3 || MMU_COLDFIRE
help
Ignore all but the first contiguous chunk of physical memory for VM
purposes. This will save a few bytes kernel size and may speed up
@@ -406,7 +406,7 @@ config M68K_L2_CACHE
config NODES_SHIFT
int
default "3"
- depends on !SINGLE_MEMORY_CHUNK
+ depends on DISCONTIGMEM

config CPU_HAS_NO_BITFIELDS
bool
diff --git a/arch/m68k/include/asm/page_mm.h b/arch/m68k/include/asm/page_mm.h
index e6b75992192b..0e794051d3bb 100644
--- a/arch/m68k/include/asm/page_mm.h
+++ b/arch/m68k/include/asm/page_mm.h
@@ -126,7 +126,7 @@ static inline void *__va(unsigned long x)

extern int m68k_virt_to_node_shift;

-#ifdef CONFIG_SINGLE_MEMORY_CHUNK
+#ifndef CONFIG_DISCONTIGMEM
#define __virt_to_node(addr) (&pg_data_map[0])
#else
extern struct pglist_data *pg_data_table[];
diff --git a/arch/m68k/mm/init.c b/arch/m68k/mm/init.c
index 53040857a9ed..4b46ceace3d3 100644
--- a/arch/m68k/mm/init.c
+++ b/arch/m68k/mm/init.c
@@ -47,14 +47,14 @@ EXPORT_SYMBOL(pg_data_map);

int m68k_virt_to_node_shift;

-#ifndef CONFIG_SINGLE_MEMORY_CHUNK
+#ifdef CONFIG_DISCONTIGMEM
pg_data_t *pg_data_table[65];
EXPORT_SYMBOL(pg_data_table);
#endif

void __init m68k_setup_node(int node)
{
-#ifndef CONFIG_SINGLE_MEMORY_CHUNK
+#ifdef CONFIG_DISCONTIGMEM
struct m68k_mem_info *info = m68k_memory + node;
int i, end;

--
2.28.0


2020-10-29 01:23:14

by Michael Schmitz

[permalink] [raw]
Subject: Re: [PATCH 11/13] m68k/mm: make node data and node setup depend on CONFIG_DISCONTIGMEM

Hi Mike,

On 29/10/20 12:16 AM, Mike Rapoport wrote:
> Hi Geert,
>
> On Wed, Oct 28, 2020 at 10:25:49AM +0100, Geert Uytterhoeven wrote:
>> Hi Mike,
>>
>> On Tue, Oct 27, 2020 at 12:31 PM Mike Rapoport <[email protected]> wrote:
>>> From: Mike Rapoport <[email protected]>
>>>
>>> The pg_data_t node structures and their initialization currently depends on
>>> !CONFIG_SINGLE_MEMORY_CHUNK. Since they are required only for DISCONTIGMEM
>>> make this dependency explicit and replace usage of
>>> CONFIG_SINGLE_MEMORY_CHUNK with CONFIG_DISCONTIGMEM where appropriate.
>>>
>>> The CONFIG_SINGLE_MEMORY_CHUNK was implicitly disabled on the ColdFire MMU
>>> variant, although it always presumed a single memory bank. As there is no
>>> actual need for DISCONTIGMEM in this case, make sure that ColdFire MMU
>>> systems set CONFIG_SINGLE_MEMORY_CHUNK to 'y'.
>>>
>>> Signed-off-by: Mike Rapoport <[email protected]>
>> Thanks for your patch!
>>
>>> ---
>>> arch/m68k/Kconfig.cpu | 6 +++---
>>> arch/m68k/include/asm/page_mm.h | 2 +-
>>> arch/m68k/mm/init.c | 4 ++--
>>> 3 files changed, 6 insertions(+), 6 deletions(-)
>> Is there any specific reason you didn't convert the checks for
>> CONFIG_SINGLE_MEMORY_CHUNK in arch/m68k/kernel/setup_mm.c
> In arch/m68k/kernel/setup_mm.c the CONFIG_SINGLE_MEMORY_CHUNK is needed
> for the case when a system has two banks, the kernel is loaded into the
> second bank and so the first bank cannot be used as normal memory. It
> does not matter what memory model will be used in this case.


That case used to be detected just fine at run time (by dint of the
second memory chunk having an address below the first; the chunk the
kernel resides in is always listed first), even without using
CONFIG_SINGLE_MEMORY_CHUNK.

Unless you changed that behaviour (and I see nothing in your patch that
would indicate that), this is still true.

Converting the check as Geert suggested, without also adding a test for
out-of-order memory bank addresses, would implicitly treat DISCONTIGMEM
as  SINGLE_MEMORY_CHUNK, regardless of bank ordering. I don't think that
is what we really want?

Cheers,

    Michael


>
>> and arch/m68k/include/asm/virtconvert.h?
>
> I remember I had build errors and troubles with include file
> dependencies if I changed it there, but I might be mistaken. I'll
> recheck again.
>
>> 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

2020-10-29 08:24:58

by Mike Rapoport

[permalink] [raw]
Subject: Re: [PATCH 11/13] m68k/mm: make node data and node setup depend on CONFIG_DISCONTIGMEM

On Thu, Oct 29, 2020 at 07:14:38AM +1300, Michael Schmitz wrote:
> Hi Mike,
>
> On 29/10/20 12:16 AM, Mike Rapoport wrote:
> > Hi Geert,
> >
> > On Wed, Oct 28, 2020 at 10:25:49AM +0100, Geert Uytterhoeven wrote:
> > > Hi Mike,
> > >
> > > On Tue, Oct 27, 2020 at 12:31 PM Mike Rapoport <[email protected]> wrote:
> > > > From: Mike Rapoport <[email protected]>
> > > >
> > > > The pg_data_t node structures and their initialization currently depends on
> > > > !CONFIG_SINGLE_MEMORY_CHUNK. Since they are required only for DISCONTIGMEM
> > > > make this dependency explicit and replace usage of
> > > > CONFIG_SINGLE_MEMORY_CHUNK with CONFIG_DISCONTIGMEM where appropriate.
> > > >
> > > > The CONFIG_SINGLE_MEMORY_CHUNK was implicitly disabled on the ColdFire MMU
> > > > variant, although it always presumed a single memory bank. As there is no
> > > > actual need for DISCONTIGMEM in this case, make sure that ColdFire MMU
> > > > systems set CONFIG_SINGLE_MEMORY_CHUNK to 'y'.
> > > >
> > > > Signed-off-by: Mike Rapoport <[email protected]>
> > > Thanks for your patch!
> > >
> > > > ---
> > > > arch/m68k/Kconfig.cpu | 6 +++---
> > > > arch/m68k/include/asm/page_mm.h | 2 +-
> > > > arch/m68k/mm/init.c | 4 ++--
> > > > 3 files changed, 6 insertions(+), 6 deletions(-)
> > > Is there any specific reason you didn't convert the checks for
> > > CONFIG_SINGLE_MEMORY_CHUNK in arch/m68k/kernel/setup_mm.c
> > In arch/m68k/kernel/setup_mm.c the CONFIG_SINGLE_MEMORY_CHUNK is needed
> > for the case when a system has two banks, the kernel is loaded into the
> > second bank and so the first bank cannot be used as normal memory. It
> > does not matter what memory model will be used in this case.
>
>
> That case used to be detected just fine at run time (by dint of the second
> memory chunk having an address below the first; the chunk the kernel resides
> in is always listed first), even without using CONFIG_SINGLE_MEMORY_CHUNK.

Right, CONFIG_SINGLE_MEMORY_CHUNK in arch/m68k/kernel/setup_mm.c is used
to force using a single bank of memory regardless of run time detection.

> Unless you changed that behaviour (and I see nothing in your patch that
> would indicate that), this is still true.
>
> Converting the check as Geert suggested, without also adding a test for
> out-of-order memory bank addresses, would implicitly treat DISCONTIGMEM as?
> SINGLE_MEMORY_CHUNK, regardless of bank ordering. I don't think that is what
> we really want?

It is in a way the case now when !SINGLE_MEMORY_CHUNK == DISCONTIGMEM.
So forcing SIGNLE_MEMORY_CHUNK at compile time would also mean forcing
FLATMEM.

After these changes I think SINGLE_MEMORY_CHUNK is not needed at all.

> Cheers,
>
> ??? Michael
>
>
> >
> > > and arch/m68k/include/asm/virtconvert.h?
> > I remember I had build errors and troubles with include file
> > dependencies if I changed it there, but I might be mistaken. I'll
> > recheck again.
> >
> > > 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

--
Sincerely yours,
Mike.

2020-10-29 08:40:55

by Mike Rapoport

[permalink] [raw]
Subject: Re: [PATCH 11/13] m68k/mm: make node data and node setup depend on CONFIG_DISCONTIGMEM

Hi Geert,

On Wed, Oct 28, 2020 at 10:25:49AM +0100, Geert Uytterhoeven wrote:
> Hi Mike,
>
> On Tue, Oct 27, 2020 at 12:31 PM Mike Rapoport <[email protected]> wrote:
> > From: Mike Rapoport <[email protected]>
> >
> > The pg_data_t node structures and their initialization currently depends on
> > !CONFIG_SINGLE_MEMORY_CHUNK. Since they are required only for DISCONTIGMEM
> > make this dependency explicit and replace usage of
> > CONFIG_SINGLE_MEMORY_CHUNK with CONFIG_DISCONTIGMEM where appropriate.
> >
> > The CONFIG_SINGLE_MEMORY_CHUNK was implicitly disabled on the ColdFire MMU
> > variant, although it always presumed a single memory bank. As there is no
> > actual need for DISCONTIGMEM in this case, make sure that ColdFire MMU
> > systems set CONFIG_SINGLE_MEMORY_CHUNK to 'y'.
> >
> > Signed-off-by: Mike Rapoport <[email protected]>
>
> Thanks for your patch!
>
> > ---
> > arch/m68k/Kconfig.cpu | 6 +++---
> > arch/m68k/include/asm/page_mm.h | 2 +-
> > arch/m68k/mm/init.c | 4 ++--
> > 3 files changed, 6 insertions(+), 6 deletions(-)
>
> Is there any specific reason you didn't convert the checks for
> CONFIG_SINGLE_MEMORY_CHUNK in arch/m68k/kernel/setup_mm.c

In arch/m68k/kernel/setup_mm.c the CONFIG_SINGLE_MEMORY_CHUNK is needed
for the case when a system has two banks, the kernel is loaded into the
second bank and so the first bank cannot be used as normal memory. It
does not matter what memory model will be used in this case.

> and arch/m68k/include/asm/virtconvert.h?

I remember I had build errors and troubles with include file
dependencies if I changed it there, but I might be mistaken. I'll
recheck again.

> 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

--
Sincerely yours,
Mike.

2020-10-29 10:01:58

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 11/13] m68k/mm: make node data and node setup depend on CONFIG_DISCONTIGMEM

Hi Mike,

On Tue, Oct 27, 2020 at 12:31 PM Mike Rapoport <[email protected]> wrote:
> From: Mike Rapoport <[email protected]>
>
> The pg_data_t node structures and their initialization currently depends on
> !CONFIG_SINGLE_MEMORY_CHUNK. Since they are required only for DISCONTIGMEM
> make this dependency explicit and replace usage of
> CONFIG_SINGLE_MEMORY_CHUNK with CONFIG_DISCONTIGMEM where appropriate.
>
> The CONFIG_SINGLE_MEMORY_CHUNK was implicitly disabled on the ColdFire MMU
> variant, although it always presumed a single memory bank. As there is no
> actual need for DISCONTIGMEM in this case, make sure that ColdFire MMU
> systems set CONFIG_SINGLE_MEMORY_CHUNK to 'y'.
>
> Signed-off-by: Mike Rapoport <[email protected]>

Thanks for your patch!

> ---
> arch/m68k/Kconfig.cpu | 6 +++---
> arch/m68k/include/asm/page_mm.h | 2 +-
> arch/m68k/mm/init.c | 4 ++--
> 3 files changed, 6 insertions(+), 6 deletions(-)

Is there any specific reason you didn't convert the checks for
CONFIG_SINGLE_MEMORY_CHUNK in arch/m68k/kernel/setup_mm.c
and arch/m68k/include/asm/virtconvert.h?

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

2020-11-01 16:57:27

by Mike Rapoport

[permalink] [raw]
Subject: Re: [PATCH 11/13] m68k/mm: make node data and node setup depend on CONFIG_DISCONTIGMEM

On Wed, Oct 28, 2020 at 01:16:41PM +0200, Mike Rapoport wrote:
> Hi Geert,
>
> On Wed, Oct 28, 2020 at 10:25:49AM +0100, Geert Uytterhoeven wrote:
> > Hi Mike,
> >
> > On Tue, Oct 27, 2020 at 12:31 PM Mike Rapoport <[email protected]> wrote:
> > > From: Mike Rapoport <[email protected]>
> > >
> > > The pg_data_t node structures and their initialization currently depends on
> > > !CONFIG_SINGLE_MEMORY_CHUNK. Since they are required only for DISCONTIGMEM
> > > make this dependency explicit and replace usage of
> > > CONFIG_SINGLE_MEMORY_CHUNK with CONFIG_DISCONTIGMEM where appropriate.
> > >
> > > The CONFIG_SINGLE_MEMORY_CHUNK was implicitly disabled on the ColdFire MMU
> > > variant, although it always presumed a single memory bank. As there is no
> > > actual need for DISCONTIGMEM in this case, make sure that ColdFire MMU
> > > systems set CONFIG_SINGLE_MEMORY_CHUNK to 'y'.
> > >
> > > Signed-off-by: Mike Rapoport <[email protected]>
> >
> > Thanks for your patch!
> >
> > > ---
> > > arch/m68k/Kconfig.cpu | 6 +++---
> > > arch/m68k/include/asm/page_mm.h | 2 +-
> > > arch/m68k/mm/init.c | 4 ++--
> > > 3 files changed, 6 insertions(+), 6 deletions(-)
> >
> > Is there any specific reason you didn't convert the checks for
> > CONFIG_SINGLE_MEMORY_CHUNK in arch/m68k/kernel/setup_mm.c
>
> In arch/m68k/kernel/setup_mm.c the CONFIG_SINGLE_MEMORY_CHUNK is needed
> for the case when a system has two banks, the kernel is loaded into the
> second bank and so the first bank cannot be used as normal memory. It
> does not matter what memory model will be used in this case.
>
> > and arch/m68k/include/asm/virtconvert.h?
>
> I remember I had build errors and troubles with include file
> dependencies if I changed it there, but I might be mistaken. I'll
> recheck again.

There indeed was an issue with SINGLE_MEMORY_CHUNK that selected
NEED_MULTIPLE_NODES for some reason.
With that fixed and removed CONFIG_SINGLE_MEMORY_CHUNK in
arch/m68k/include/asm/virtconvert.h I'm going to send v2 soon.

I've kept CONFIG_SINGLE_MEMORY_CHUNK for now for backwards compatibility
with the plan to remove it along with DISCONTIGMEM.

> > 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
>
> --
> Sincerely yours,
> Mike.

--
Sincerely yours,
Mike.