2020-02-28 01:21:41

by Brendan Higgins

[permalink] [raw]
Subject: [PATCH v3 1/7] vmlinux.lds.h: add linker section for KUnit test suites

Add a linker section where KUnit can put references to its test suites.
This patch is the first step in transitioning to dispatching all KUnit
tests from a centralized executor rather than having each as its own
separate late_initcall.

Co-developed-by: Iurii Zaikin <[email protected]>
Signed-off-by: Iurii Zaikin <[email protected]>
Signed-off-by: Brendan Higgins <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
---
include/asm-generic/vmlinux.lds.h | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index e00f41aa8ec4f..99a866f49cb3d 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -856,6 +856,13 @@
KEEP(*(.con_initcall.init)) \
__con_initcall_end = .;

+/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */
+#define KUNIT_TEST_SUITES \
+ . = ALIGN(8); \
+ __kunit_suites_start = .; \
+ KEEP(*(.kunit_test_suites)) \
+ __kunit_suites_end = .;
+
#ifdef CONFIG_BLK_DEV_INITRD
#define INIT_RAM_FS \
. = ALIGN(4); \
@@ -1024,6 +1031,7 @@
INIT_CALLS \
CON_INITCALL \
INIT_RAM_FS \
+ KUNIT_TEST_SUITES \
}

#define BSS_SECTION(sbss_align, bss_align, stop_align) \
--
2.25.1.481.gfbce0eb801-goog


2020-02-28 07:23:09

by Brendan Higgins

[permalink] [raw]
Subject: Re: [PATCH v3 1/7] vmlinux.lds.h: add linker section for KUnit test suites

On Thu, Feb 27, 2020 at 5:20 PM Brendan Higgins
<[email protected]> wrote:
>
> Add a linker section where KUnit can put references to its test suites.
> This patch is the first step in transitioning to dispatching all KUnit
> tests from a centralized executor rather than having each as its own
> separate late_initcall.
>
> Co-developed-by: Iurii Zaikin <[email protected]>
> Signed-off-by: Iurii Zaikin <[email protected]>
> Signed-off-by: Brendan Higgins <[email protected]>
> Reviewed-by: Stephen Boyd <[email protected]>
> ---
> include/asm-generic/vmlinux.lds.h | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index e00f41aa8ec4f..99a866f49cb3d 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -856,6 +856,13 @@
> KEEP(*(.con_initcall.init)) \
> __con_initcall_end = .;
>
> +/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */
> +#define KUNIT_TEST_SUITES \
> + . = ALIGN(8); \

After posting this, I saw I had gotten an email from 0day[1]. After
some investigation, I discovered that this 8 byte alignment works for
x86 64 bit fine, but only *sometimes* for 32 bit. 4 byte alignment
seems to work in all cases (so far). I am not sure why we went with
such a large alignment in hindsight. In any case, I should have a
fixed revision out pretty soon.

> + __kunit_suites_start = .; \
> + KEEP(*(.kunit_test_suites)) \
> + __kunit_suites_end = .;
> +
> #ifdef CONFIG_BLK_DEV_INITRD
> #define INIT_RAM_FS \
> . = ALIGN(4); \
> @@ -1024,6 +1031,7 @@
> INIT_CALLS \
> CON_INITCALL \
> INIT_RAM_FS \
> + KUNIT_TEST_SUITES \
> }
>
> #define BSS_SECTION(sbss_align, bss_align, stop_align) \
> --

[1] https://lists.01.org/hyperkitty/list/[email protected]/thread/4I4UW4OAT63ETMIEUJQTOF3BFTMO6ROD/

2020-02-28 17:54:03

by Iurii Zaikin

[permalink] [raw]
Subject: Re: [PATCH v3 1/7] vmlinux.lds.h: add linker section for KUnit test suites

We went with this alignment precisely because it's the largest that
any supported arch may possibly need. The problem as I understood it
was that the compiler, seeing a bunch of pointers decided to put them
at the memory-access efficient alignment rather than at the section
start. Remember that the section start used to be unaligned for some
reason. Note that the alignment that is a multiple of smaller
alignment is still aligned wrt the smaller alignment, so the compiler
shouldn't need to put the pointers elsewhere.
I wonder if there's a more robust way of forcing the compiler to put
the pointers right at the section start and insert no gaps between
them than playing with alignment.

On Thu, Feb 27, 2020 at 11:22 PM Brendan Higgins
<[email protected]> wrote:
>
> On Thu, Feb 27, 2020 at 5:20 PM Brendan Higgins
> <[email protected]> wrote:
> >
> > Add a linker section where KUnit can put references to its test suites.
> > This patch is the first step in transitioning to dispatching all KUnit
> > tests from a centralized executor rather than having each as its own
> > separate late_initcall.
> >
> > Co-developed-by: Iurii Zaikin <[email protected]>
> > Signed-off-by: Iurii Zaikin <[email protected]>
> > Signed-off-by: Brendan Higgins <[email protected]>
> > Reviewed-by: Stephen Boyd <[email protected]>
> > ---
> > include/asm-generic/vmlinux.lds.h | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > index e00f41aa8ec4f..99a866f49cb3d 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -856,6 +856,13 @@
> > KEEP(*(.con_initcall.init)) \
> > __con_initcall_end = .;
> >
> > +/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */
> > +#define KUNIT_TEST_SUITES \
> > + . = ALIGN(8); \
>
> After posting this, I saw I had gotten an email from 0day[1]. After
> some investigation, I discovered that this 8 byte alignment works for
> x86 64 bit fine, but only *sometimes* for 32 bit. 4 byte alignment
> seems to work in all cases (so far). I am not sure why we went with
> such a large alignment in hindsight. In any case, I should have a
> fixed revision out pretty soon.
>
> > + __kunit_suites_start = .; \
> > + KEEP(*(.kunit_test_suites)) \
> > + __kunit_suites_end = .;
> > +
> > #ifdef CONFIG_BLK_DEV_INITRD
> > #define INIT_RAM_FS \
> > . = ALIGN(4); \
> > @@ -1024,6 +1031,7 @@
> > INIT_CALLS \
> > CON_INITCALL \
> > INIT_RAM_FS \
> > + KUNIT_TEST_SUITES \
> > }
> >
> > #define BSS_SECTION(sbss_align, bss_align, stop_align) \
> > --
>
> [1] https://lists.01.org/hyperkitty/list/[email protected]/thread/4I4UW4OAT63ETMIEUJQTOF3BFTMO6ROD/