Discussion:
[PATCH v2 01/15] mm: add setup_initial_init_mm() helper
Kefeng Wang
2021-06-04 07:06:19 UTC
Permalink
Add setup_initial_init_mm() helper to setup kernel text,
data and brk.

Cc: linux-snps-***@lists.infradead.org
Cc: linux-arm-***@lists.infradead.org
Cc: linux-***@vger.kernel.org
Cc: uclinux-h8-***@lists.sourceforge.jp
Cc: linux-***@lists.linux-m68k.org
Cc: ***@lists.librecores.org
Cc: linuxppc-***@lists.ozlabs.org
Cc: linux-***@lists.infradead.org
Cc: linux-***@vger.kernel.org
Cc: linux-***@vger.kernel.org
Cc: ***@kernel.org
Signed-off-by: Kefeng Wang <***@huawei.com>
---
include/linux/mm_types.h | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 5aacc1c10a45..e1d2429089a4 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -572,6 +572,14 @@ struct mm_struct {
};

extern struct mm_struct init_mm;
+static inline void setup_initial_init_mm(void *start_code, void *end_code,
+ void *end_data, void *brk)
+{
+ init_mm.start_code = (unsigned long)start_code;
+ init_mm.end_code = (unsigned long)end_code;
+ init_mm.end_data = (unsigned long)end_data;
+ init_mm.brk = (unsigned long)brk;
+}

/* Pointer magic because the dynamic array size confuses some compilers. */
static inline void mm_init_cpumask(struct mm_struct *mm)
--
2.26.2
Kefeng Wang
2021-06-04 07:06:29 UTC
Permalink
Use setup_initial_init_mm() helper to simplify code.

Note klimit is (unsigned long) _end, with new helper,
will use _end directly.

Cc: Michael Ellerman <***@ellerman.id.au>
Cc: Benjamin Herrenschmidt <***@kernel.crashing.org>
Cc: linuxppc-***@lists.ozlabs.org
Signed-off-by: Kefeng Wang <***@huawei.com>
---
arch/powerpc/kernel/setup-common.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 74a98fff2c2f..96697c6e1e16 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -927,10 +927,7 @@ void __init setup_arch(char **cmdline_p)

klp_init_thread_info(&init_task);

- init_mm.start_code = (unsigned long)_stext;
- init_mm.end_code = (unsigned long) _etext;
- init_mm.end_data = (unsigned long) _edata;
- init_mm.brk = klimit;
+ setup_initial_init_mm(_stext, _etext, _edata, _end);

mm_iommu_init(&init_mm);
irqstack_early_init();
--
2.26.2
Mike Rapoport
2021-06-06 21:29:09 UTC
Permalink
Hello Kefeng,
Add setup_initial_init_mm() helper, then use it
to cleanup the text, data and brk setup code.
- change argument from "char *" to "void *" setup_initial_init_mm()
suggested by Geert Uytterhoeven
- use NULL instead of (void *)0 on h8300 and m68k
- collect ACKs
mm: add setup_initial_init_mm() helper
arc: convert to setup_initial_init_mm()
arm: convert to setup_initial_init_mm()
arm64: convert to setup_initial_init_mm()
csky: convert to setup_initial_init_mm()
h8300: convert to setup_initial_init_mm()
m68k: convert to setup_initial_init_mm()
nds32: convert to setup_initial_init_mm()
nios2: convert to setup_initial_init_mm()
openrisc: convert to setup_initial_init_mm()
powerpc: convert to setup_initial_init_mm()
riscv: convert to setup_initial_init_mm()
s390: convert to setup_initial_init_mm()
sh: convert to setup_initial_init_mm()
x86: convert to setup_initial_init_mm()
I might be missing something, but AFAIU the init_mm.start_code and other
fields are not used really early so the new setup_initial_init_mm()
function can be called in the generic code outside setup_arch(), e.g in
mm_init().
arch/arc/mm/init.c | 5 +----
arch/arm/kernel/setup.c | 5 +----
arch/arm64/kernel/setup.c | 5 +----
arch/csky/kernel/setup.c | 5 +----
arch/h8300/kernel/setup.c | 5 +----
arch/m68k/kernel/setup_mm.c | 5 +----
arch/m68k/kernel/setup_no.c | 5 +----
arch/nds32/kernel/setup.c | 5 +----
arch/nios2/kernel/setup.c | 5 +----
arch/openrisc/kernel/setup.c | 5 +----
arch/powerpc/kernel/setup-common.c | 5 +----
arch/riscv/kernel/setup.c | 5 +----
arch/s390/kernel/setup.c | 5 +----
arch/sh/kernel/setup.c | 5 +----
arch/x86/kernel/setup.c | 5 +----
include/linux/mm_types.h | 8 ++++++++
16 files changed, 23 insertions(+), 60 deletions(-)
--
2.26.2
_______________________________________________
linux-riscv mailing list
http://lists.infradead.org/mailman/listinfo/linux-riscv
--
Sincerely yours,
Mike.
Kefeng Wang
2021-06-07 00:55:54 UTC
Permalink
Post by Mike Rapoport
Hello Kefeng,
Add setup_initial_init_mm() helper, then use it
to cleanup the text, data and brk setup code.
- change argument from "char *" to "void *" setup_initial_init_mm()
suggested by Geert Uytterhoeven
- use NULL instead of (void *)0 on h8300 and m68k
- collect ACKs
mm: add setup_initial_init_mm() helper
arc: convert to setup_initial_init_mm()
arm: convert to setup_initial_init_mm()
arm64: convert to setup_initial_init_mm()
csky: convert to setup_initial_init_mm()
h8300: convert to setup_initial_init_mm()
m68k: convert to setup_initial_init_mm()
nds32: convert to setup_initial_init_mm()
nios2: convert to setup_initial_init_mm()
openrisc: convert to setup_initial_init_mm()
powerpc: convert to setup_initial_init_mm()
riscv: convert to setup_initial_init_mm()
s390: convert to setup_initial_init_mm()
sh: convert to setup_initial_init_mm()
x86: convert to setup_initial_init_mm()
I might be missing something, but AFAIU the init_mm.start_code and other
fields are not used really early so the new setup_initial_init_mm()
function can be called in the generic code outside setup_arch(), e.g in
mm_init().
Hi Mike, each architecture has their own value, not the same, eg m68K and

h8300, also the name of the text/code/brk is different in some arch, so
I keep

unchanged.
Post by Mike Rapoport
arch/arc/mm/init.c | 5 +----
arch/arm/kernel/setup.c | 5 +----
arch/arm64/kernel/setup.c | 5 +----
arch/csky/kernel/setup.c | 5 +----
arch/h8300/kernel/setup.c | 5 +----
arch/m68k/kernel/setup_mm.c | 5 +----
arch/m68k/kernel/setup_no.c | 5 +----
arch/nds32/kernel/setup.c | 5 +----
arch/nios2/kernel/setup.c | 5 +----
arch/openrisc/kernel/setup.c | 5 +----
arch/powerpc/kernel/setup-common.c | 5 +----
arch/riscv/kernel/setup.c | 5 +----
arch/s390/kernel/setup.c | 5 +----
arch/sh/kernel/setup.c | 5 +----
arch/x86/kernel/setup.c | 5 +----
include/linux/mm_types.h | 8 ++++++++
16 files changed, 23 insertions(+), 60 deletions(-)
--
2.26.2
_______________________________________________
linux-riscv mailing list
http://lists.infradead.org/mailman/listinfo/linux-riscv
Christophe Leroy
2021-06-07 05:48:54 UTC
Permalink
Hi Kefeng,
Post by Kefeng Wang
Post by Mike Rapoport
Hello Kefeng,
Add setup_initial_init_mm() helper, then use it
to cleanup the text, data and brk setup code.
- change argument from "char *" to "void *" setup_initial_init_mm()
   suggested by Geert Uytterhoeven
- use NULL instead of (void *)0 on h8300 and m68k
- collect ACKs
   mm: add setup_initial_init_mm() helper
   arc: convert to setup_initial_init_mm()
   arm: convert to setup_initial_init_mm()
   arm64: convert to setup_initial_init_mm()
   csky: convert to setup_initial_init_mm()
   h8300: convert to setup_initial_init_mm()
   m68k: convert to setup_initial_init_mm()
   nds32: convert to setup_initial_init_mm()
   nios2: convert to setup_initial_init_mm()
   openrisc: convert to setup_initial_init_mm()
   powerpc: convert to setup_initial_init_mm()
   riscv: convert to setup_initial_init_mm()
   s390: convert to setup_initial_init_mm()
   sh: convert to setup_initial_init_mm()
   x86: convert to setup_initial_init_mm()
I might be missing something, but AFAIU the init_mm.start_code and other
fields are not used really early so the new setup_initial_init_mm()
function can be called in the generic code outside setup_arch(), e.g in
mm_init().
Hi Mike, each architecture has their own value, not the same, eg m68K and
h8300, also the name of the text/code/brk is different in some arch, so I keep
unchanged.
What you could do is to define a __weak function that architectures can override and call that
function from mm_init() as suggested by Mike,

Something like:

void __weak setup_initial_init_mm(void)
{
init_mm.start_code = (unsigned long)_stext;
init_mm.end_code = (unsigned long)_etext;
init_mm.end_data = (unsigned long)_edata;
init_mm.brk = (unsigned long)_end;
}

Then only the few architecture that are different would override it.

I see a few archictectures are usigne PAGE_OFFSET to set .start_code, but it is likely that this is
equivalent to _stext.

Christophe
Kefeng Wang
2021-06-07 08:30:40 UTC
Permalink
Post by Christophe Leroy
Hi Kefeng,
Post by Kefeng Wang
Post by Mike Rapoport
Hello Kefeng,
Add setup_initial_init_mm() helper, then use it
to cleanup the text, data and brk setup code.
- change argument from "char *" to "void *" setup_initial_init_mm()
   suggested by Geert Uytterhoeven
- use NULL instead of (void *)0 on h8300 and m68k
- collect ACKs
   mm: add setup_initial_init_mm() helper
   arc: convert to setup_initial_init_mm()
   arm: convert to setup_initial_init_mm()
   arm64: convert to setup_initial_init_mm()
   csky: convert to setup_initial_init_mm()
   h8300: convert to setup_initial_init_mm()
   m68k: convert to setup_initial_init_mm()
   nds32: convert to setup_initial_init_mm()
   nios2: convert to setup_initial_init_mm()
   openrisc: convert to setup_initial_init_mm()
   powerpc: convert to setup_initial_init_mm()
   riscv: convert to setup_initial_init_mm()
   s390: convert to setup_initial_init_mm()
   sh: convert to setup_initial_init_mm()
   x86: convert to setup_initial_init_mm()
I might be missing something, but AFAIU the init_mm.start_code and other
fields are not used really early so the new setup_initial_init_mm()
function can be called in the generic code outside setup_arch(), e.g in
mm_init().
Hi Mike, each architecture has their own value, not the same, eg m68K and
h8300, also the name of the text/code/brk is different in some arch, so I keep
unchanged.
What you could do is to define a __weak function that architectures
can override and call that function from mm_init() as suggested by Mike,
void __weak setup_initial_init_mm(void)
{
    init_mm.start_code = (unsigned long)_stext;
    init_mm.end_code = (unsigned long)_etext;
    init_mm.end_data = (unsigned long)_edata;
    init_mm.brk = (unsigned long)_end;
}
Then only the few architecture that are different would override it.
I see a few archictectures are usigne PAGE_OFFSET to set .start_code,
but it is likely that this is equivalent to _stext.
Yes,  the __weak function is option, but the change is only covered 14
archs, there are 7 other archs(alpha  hexagon  ia64

microblaze  mips sparc  um xtensa)without related setup code. Also like
x86, it has own brk , maybe there are some

other different in some arch, so I think let's keep unchanged for now, 
thanks.
Post by Christophe Leroy
Christophe
.
Russell King (Oracle)
2021-06-07 09:31:21 UTC
Permalink
Post by Christophe Leroy
Hi Kefeng,
What you could do is to define a __weak function that architectures can
override and call that function from mm_init() as suggested by Mike,
The problem with weak functions is that they bloat the kernel. Each
time a weak function is overriden, it becomes dead unreachable code
within the kernel image.

At some point we're probabily going to have to enable -ffunction-sections
to (hopefully) allow the dead code to be discarded.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Mike Rapoport
2021-06-06 21:31:09 UTC
Permalink
Hello Kefeng,
Post by Kefeng Wang
Add setup_initial_init_mm() helper to setup kernel text,
data and brk.
---
include/linux/mm_types.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 5aacc1c10a45..e1d2429089a4 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -572,6 +572,14 @@ struct mm_struct {
};
extern struct mm_struct init_mm;
+static inline void setup_initial_init_mm(void *start_code, void *end_code,
+ void *end_data, void *brk)
I think it's not that performance sensitive to make it inline. It can be
placed in mm/init-mm.c with a forward declaration in mm.h
Post by Kefeng Wang
+{
+ init_mm.start_code = (unsigned long)start_code;
+ init_mm.end_code = (unsigned long)end_code;
+ init_mm.end_data = (unsigned long)end_data;
+ init_mm.brk = (unsigned long)brk;
+}
/* Pointer magic because the dynamic array size confuses some compilers. */
static inline void mm_init_cpumask(struct mm_struct *mm)
--
2.26.2
_______________________________________________
linux-riscv mailing list
http://lists.infradead.org/mailman/listinfo/linux-riscv
--
Sincerely yours,
Mike.
Kefeng Wang
2021-06-07 01:50:31 UTC
Permalink
Post by Mike Rapoport
Hello Kefeng,
Post by Kefeng Wang
Add setup_initial_init_mm() helper to setup kernel text,
data and brk.
---
include/linux/mm_types.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 5aacc1c10a45..e1d2429089a4 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -572,6 +572,14 @@ struct mm_struct {
};
extern struct mm_struct init_mm;
+static inline void setup_initial_init_mm(void *start_code, void *end_code,
+ void *end_data, void *brk)
I think it's not that performance sensitive to make it inline. It can be
placed in mm/init-mm.c with a forward declaration in mm.h
Ok, I will send a update one with this change.
Post by Mike Rapoport
Post by Kefeng Wang
+{
+ init_mm.start_code = (unsigned long)start_code;
+ init_mm.end_code = (unsigned long)end_code;
+ init_mm.end_data = (unsigned long)end_data;
+ init_mm.brk = (unsigned long)brk;
+}
/* Pointer magic because the dynamic array size confuses some compilers. */
static inline void mm_init_cpumask(struct mm_struct *mm)
--
2.26.2
_______________________________________________
linux-riscv mailing list
http://lists.infradead.org/mailman/listinfo/linux-riscv
Kefeng Wang
2021-06-07 02:36:11 UTC
Permalink
Add setup_initial_init_mm() helper to setup kernel text,
data and brk.

Cc: linux-snps-***@lists.infradead.org
Cc: linux-arm-***@lists.infradead.org
Cc: linux-***@vger.kernel.org
Cc: uclinux-h8-***@lists.sourceforge.jp
Cc: linux-***@lists.linux-m68k.org
Cc: ***@lists.librecores.org
Cc: linuxppc-***@lists.ozlabs.org
Cc: linux-***@lists.infradead.org
Cc: linux-***@vger.kernel.org
Cc: linux-***@vger.kernel.org
Cc: ***@kernel.org
Signed-off-by: Kefeng Wang <***@huawei.com>
---
v3: declaration in mm.h, implemention in init-mm.c
include/linux/mm.h | 3 +++
mm/init-mm.c | 9 +++++++++
2 files changed, 12 insertions(+)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index c274f75efcf9..02aa057540b7 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -244,6 +244,9 @@ int __add_to_page_cache_locked(struct page *page, struct address_space *mapping,

#define lru_to_page(head) (list_entry((head)->prev, struct page, lru))

+void setup_initial_init_mm(void *start_code, void *end_code,
+ void *end_data, void *brk);
+
/*
* Linux kernel virtual memory manager primitives.
* The idea being to have a "virtual" mm in the same way
diff --git a/mm/init-mm.c b/mm/init-mm.c
index 153162669f80..b4a6f38fb51d 100644
--- a/mm/init-mm.c
+++ b/mm/init-mm.c
@@ -40,3 +40,12 @@ struct mm_struct init_mm = {
.cpu_bitmap = CPU_BITS_NONE,
INIT_MM_CONTEXT(init_mm)
};
+
+void setup_initial_init_mm(void *start_code, void *end_code,
+ void *end_data, void *brk)
+{
+ init_mm.start_code = (unsigned long)start_code;
+ init_mm.end_code = (unsigned long)end_code;
+ init_mm.end_data = (unsigned long)end_data;
+ init_mm.brk = (unsigned long)brk;
+}
--
2.26.2
Loading...