All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] kunit: add support for kunit_suites that reference init code
@ 2022-03-11  7:28 Brendan Higgins
  2022-04-04 22:37 ` Shuah Khan
  0 siblings, 1 reply; 6+ messages in thread
From: Brendan Higgins @ 2022-03-11  7:28 UTC (permalink / raw)
  To: shuah, davidgow, dlatypov, martin.fernandez, daniel.gutson
  Cc: linux-kselftest, kunit-dev, linux-kernel, keescook, jk, Brendan Higgins

Add support for a new kind of kunit_suite registration macro called
kunit_test_init_suite(); this new registration macro allows the
registration of kunit_suites that reference functions marked __init and
data marked __initdata.

Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Tested-by: Martin Fernandez <martin.fernandez@eclypsium.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: David Gow <davidgow@google.com>
---

This is a follow-up to the RFC here[1].

This patch is in response to a KUnit user issue[2] in which the user was
attempting to test some init functions; although this is a functional
solution as long as KUnit tests only run during the init phase, we will
need to do more work if we ever allow tests to run after the init phase
is over; it is for this reason that this patch adds a new registration
macro rather than simply modifying the existing macros.

Changes since last version:
 - I added more to the kunit_test_init_suites() kernel-doc comment
   detailing "how" the modpost warnings are suppressed in addition to
   the existing information regarding "why" it is OK for the modpost
   warnings to be suppressed.

[1] https://lore.kernel.org/linux-kselftest/20220310210210.2124637-1-brendanhiggins@google.com/
[2] https://groups.google.com/g/kunit-dev/c/XDjieRHEneg/m/D0rFCwVABgAJ

---
 include/kunit/test.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index b26400731c02..7f303a06bc97 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -379,6 +379,32 @@ static inline int kunit_run_all_tests(void)
 
 #define kunit_test_suite(suite)	kunit_test_suites(&suite)
 
+/**
+ * kunit_test_init_suites() - used to register one or more &struct kunit_suite
+ *			      containing init functions or init data.
+ *
+ * @__suites: a statically allocated list of &struct kunit_suite.
+ *
+ * This functions identically as &kunit_test_suites() except that it suppresses
+ * modpost warnings for referencing functions marked __init or data marked
+ * __initdata; this is OK because currently KUnit only runs tests upon boot
+ * during the init phase or upon loading a module during the init phase.
+ *
+ * NOTE TO KUNIT DEVS: If we ever allow KUnit tests to be run after boot, these
+ * tests must be excluded.
+ *
+ * The only thing this macro does that's different from kunit_test_suites is
+ * that it suffixes the array and suite declarations it makes with _probe;
+ * modpost suppresses warnings about referencing init data for symbols named in
+ * this manner.
+ */
+#define kunit_test_init_suites(__suites...)				\
+	__kunit_test_suites(CONCATENATE(__UNIQUE_ID(array), _probe),	\
+			    CONCATENATE(__UNIQUE_ID(suites), _probe),	\
+			    ##__suites)
+
+#define kunit_test_init_suite(suite)	kunit_test_init_suites(&suite)
+
 #define kunit_suite_for_each_test_case(suite, test_case)		\
 	for (test_case = suite->test_cases; test_case->run_case; test_case++)
 

base-commit: 330f4c53d3c2d8b11d86ec03a964b86dc81452f5
-- 
2.35.1.723.g4982287a31-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] kunit: add support for kunit_suites that reference init code
  2022-03-11  7:28 [PATCH v1] kunit: add support for kunit_suites that reference init code Brendan Higgins
@ 2022-04-04 22:37 ` Shuah Khan
  2022-04-04 22:48   ` Brendan Higgins
  0 siblings, 1 reply; 6+ messages in thread
From: Shuah Khan @ 2022-04-04 22:37 UTC (permalink / raw)
  To: Brendan Higgins, shuah, davidgow, dlatypov, martin.fernandez,
	daniel.gutson
  Cc: linux-kselftest, kunit-dev, linux-kernel, keescook, jk, Shuah Khan

Hi Brendan,

On 3/11/22 12:28 AM, Brendan Higgins wrote:
> Add support for a new kind of kunit_suite registration macro called
> kunit_test_init_suite(); this new registration macro allows the
> registration of kunit_suites that reference functions marked __init and
> data marked __initdata.
> 
> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> Tested-by: Martin Fernandez <martin.fernandez@eclypsium.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: David Gow <davidgow@google.com>
> ---
> 

I almost applied it ...

> This is a follow-up to the RFC here[1].
> 
> This patch is in response to a KUnit user issue[2] in which the user was
> attempting to test some init functions; although this is a functional
> solution as long as KUnit tests only run during the init phase, we will
> need to do more work if we ever allow tests to run after the init phase
> is over; it is for this reason that this patch adds a new registration
> macro rather than simply modifying the existing macros.
> 
> Changes since last version:
>   - I added more to the kunit_test_init_suites() kernel-doc comment
>     detailing "how" the modpost warnings are suppressed in addition to
>     the existing information regarding "why" it is OK for the modpost
>     warnings to be suppressed.
> 
> [1] https://lore.kernel.org/linux-kselftest/20220310210210.2124637-1-brendanhiggins@google.com/
> [2] https://groups.google.com/g/kunit-dev/c/XDjieRHEneg/m/D0rFCwVABgAJ
> 
> ---
>   include/kunit/test.h | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
> 
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index b26400731c02..7f303a06bc97 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -379,6 +379,32 @@ static inline int kunit_run_all_tests(void)
>   
>   #define kunit_test_suite(suite)	kunit_test_suites(&suite)
>   
> +/**
> + * kunit_test_init_suites() - used to register one or more &struct kunit_suite
> + *			      containing init functions or init data.
> + *
> + * @__suites: a statically allocated list of &struct kunit_suite.
> + *
> + * This functions identically as &kunit_test_suites() except that it suppresses
> + * modpost warnings for referencing functions marked __init or data marked
> + * __initdata; this is OK because currently KUnit only runs tests upon boot
> + * during the init phase or upon loading a module during the init phase.
> + *
> + * NOTE TO KUNIT DEVS: If we ever allow KUnit tests to be run after boot, these
> + * tests must be excluded.
> + *
> + * The only thing this macro does that's different from kunit_test_suites is
> + * that it suffixes the array and suite declarations it makes with _probe;
> + * modpost suppresses warnings about referencing init data for symbols named in
> + * this manner.
> + */
> +#define kunit_test_init_suites(__suites...)				\
> +	__kunit_test_suites(CONCATENATE(__UNIQUE_ID(array), _probe),	\
> +			    CONCATENATE(__UNIQUE_ID(suites), _probe),	\
> +			    ##__suites)
> +
> +#define kunit_test_init_suite(suite)	kunit_test_init_suites(&suite)
> +
>   #define kunit_suite_for_each_test_case(suite, test_case)		\
>   	for (test_case = suite->test_cases; test_case->run_case; test_case++)
>   
> 

The naming of the function and macro are rather confusing and can become
error prone. Let's find better naming scheme.

> base-commit: 330f4c53d3c2d8b11d86ec03a964b86dc81452f5
> 

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] kunit: add support for kunit_suites that reference init code
  2022-04-04 22:37 ` Shuah Khan
@ 2022-04-04 22:48   ` Brendan Higgins
  2022-04-07 21:34     ` Martin Fernandez
  0 siblings, 1 reply; 6+ messages in thread
From: Brendan Higgins @ 2022-04-04 22:48 UTC (permalink / raw)
  To: Shuah Khan
  Cc: shuah, davidgow, dlatypov, martin.fernandez, daniel.gutson,
	linux-kselftest, kunit-dev, linux-kernel, keescook, jk

On Mon, Apr 4, 2022 at 6:37 PM Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> Hi Brendan,
>
> On 3/11/22 12:28 AM, Brendan Higgins wrote:
> > Add support for a new kind of kunit_suite registration macro called
> > kunit_test_init_suite(); this new registration macro allows the
> > registration of kunit_suites that reference functions marked __init and
> > data marked __initdata.
> >
> > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> > Tested-by: Martin Fernandez <martin.fernandez@eclypsium.com>
> > Reviewed-by: Kees Cook <keescook@chromium.org>
> > Reviewed-by: David Gow <davidgow@google.com>
> > ---
> >
>
> I almost applied it ...
>
> > This is a follow-up to the RFC here[1].
> >
> > This patch is in response to a KUnit user issue[2] in which the user was
> > attempting to test some init functions; although this is a functional
> > solution as long as KUnit tests only run during the init phase, we will
> > need to do more work if we ever allow tests to run after the init phase
> > is over; it is for this reason that this patch adds a new registration
> > macro rather than simply modifying the existing macros.
> >
> > Changes since last version:
> >   - I added more to the kunit_test_init_suites() kernel-doc comment
> >     detailing "how" the modpost warnings are suppressed in addition to
> >     the existing information regarding "why" it is OK for the modpost
> >     warnings to be suppressed.
> >
> > [1] https://lore.kernel.org/linux-kselftest/20220310210210.2124637-1-brendanhiggins@google.com/
> > [2] https://groups.google.com/g/kunit-dev/c/XDjieRHEneg/m/D0rFCwVABgAJ
> >
> > ---
> >   include/kunit/test.h | 26 ++++++++++++++++++++++++++
> >   1 file changed, 26 insertions(+)
> >
> > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > index b26400731c02..7f303a06bc97 100644
> > --- a/include/kunit/test.h
> > +++ b/include/kunit/test.h
> > @@ -379,6 +379,32 @@ static inline int kunit_run_all_tests(void)
> >
> >   #define kunit_test_suite(suite)     kunit_test_suites(&suite)
> >
> > +/**
> > + * kunit_test_init_suites() - used to register one or more &struct kunit_suite
> > + *                         containing init functions or init data.
> > + *
> > + * @__suites: a statically allocated list of &struct kunit_suite.
> > + *
> > + * This functions identically as &kunit_test_suites() except that it suppresses
> > + * modpost warnings for referencing functions marked __init or data marked
> > + * __initdata; this is OK because currently KUnit only runs tests upon boot
> > + * during the init phase or upon loading a module during the init phase.
> > + *
> > + * NOTE TO KUNIT DEVS: If we ever allow KUnit tests to be run after boot, these
> > + * tests must be excluded.
> > + *
> > + * The only thing this macro does that's different from kunit_test_suites is
> > + * that it suffixes the array and suite declarations it makes with _probe;
> > + * modpost suppresses warnings about referencing init data for symbols named in
> > + * this manner.
> > + */
> > +#define kunit_test_init_suites(__suites...)                          \
> > +     __kunit_test_suites(CONCATENATE(__UNIQUE_ID(array), _probe),    \
> > +                         CONCATENATE(__UNIQUE_ID(suites), _probe),   \
> > +                         ##__suites)
> > +
> > +#define kunit_test_init_suite(suite) kunit_test_init_suites(&suite)
> > +
> >   #define kunit_suite_for_each_test_case(suite, test_case)            \
> >       for (test_case = suite->test_cases; test_case->run_case; test_case++)
> >
> >
>
> The naming of the function and macro are rather confusing and can become
> error prone. Let's find better naming scheme.

Yeah, I wasn't sure about the name. I didn't have any better ideas
initially though. Any suggestions?

> > base-commit: 330f4c53d3c2d8b11d86ec03a964b86dc81452f5
> >
>
> thanks,
> -- Shuah

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] kunit: add support for kunit_suites that reference init code
  2022-04-04 22:48   ` Brendan Higgins
@ 2022-04-07 21:34     ` Martin Fernandez
  2022-04-08 17:34       ` Brendan Higgins
  0 siblings, 1 reply; 6+ messages in thread
From: Martin Fernandez @ 2022-04-07 21:34 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: Shuah Khan, shuah, davidgow, dlatypov, daniel.gutson,
	linux-kselftest, kunit-dev, linux-kernel, keescook, jk

On 4/4/22, Brendan Higgins <brendanhiggins@google.com> wrote:
> On Mon, Apr 4, 2022 at 6:37 PM Shuah Khan <skhan@linuxfoundation.org>
> wrote:
>>
>> Hi Brendan,
>>
>> On 3/11/22 12:28 AM, Brendan Higgins wrote:
>> > Add support for a new kind of kunit_suite registration macro called
>> > kunit_test_init_suite(); this new registration macro allows the
>> > registration of kunit_suites that reference functions marked __init and
>> > data marked __initdata.
>> >
>> > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>> > Tested-by: Martin Fernandez <martin.fernandez@eclypsium.com>
>> > Reviewed-by: Kees Cook <keescook@chromium.org>
>> > Reviewed-by: David Gow <davidgow@google.com>
>> > ---
>> >
>>
>> I almost applied it ...
>>
>> > This is a follow-up to the RFC here[1].
>> >
>> > This patch is in response to a KUnit user issue[2] in which the user
>> > was
>> > attempting to test some init functions; although this is a functional
>> > solution as long as KUnit tests only run during the init phase, we will
>> > need to do more work if we ever allow tests to run after the init phase
>> > is over; it is for this reason that this patch adds a new registration
>> > macro rather than simply modifying the existing macros.
>> >
>> > Changes since last version:
>> >   - I added more to the kunit_test_init_suites() kernel-doc comment
>> >     detailing "how" the modpost warnings are suppressed in addition to
>> >     the existing information regarding "why" it is OK for the modpost
>> >     warnings to be suppressed.
>> >
>> > [1]
>> > https://lore.kernel.org/linux-kselftest/20220310210210.2124637-1-brendanhiggins@google.com/
>> > [2] https://groups.google.com/g/kunit-dev/c/XDjieRHEneg/m/D0rFCwVABgAJ
>> >
>> > ---
>> >   include/kunit/test.h | 26 ++++++++++++++++++++++++++
>> >   1 file changed, 26 insertions(+)
>> >
>> > diff --git a/include/kunit/test.h b/include/kunit/test.h
>> > index b26400731c02..7f303a06bc97 100644
>> > --- a/include/kunit/test.h
>> > +++ b/include/kunit/test.h
>> > @@ -379,6 +379,32 @@ static inline int kunit_run_all_tests(void)
>> >
>> >   #define kunit_test_suite(suite)     kunit_test_suites(&suite)
>> >
>> > +/**
>> > + * kunit_test_init_suites() - used to register one or more &struct
>> > kunit_suite
>> > + *                         containing init functions or init data.
>> > + *
>> > + * @__suites: a statically allocated list of &struct kunit_suite.
>> > + *
>> > + * This functions identically as &kunit_test_suites() except that it
>> > suppresses
>> > + * modpost warnings for referencing functions marked __init or data
>> > marked
>> > + * __initdata; this is OK because currently KUnit only runs tests upon
>> > boot
>> > + * during the init phase or upon loading a module during the init
>> > phase.
>> > + *
>> > + * NOTE TO KUNIT DEVS: If we ever allow KUnit tests to be run after
>> > boot, these
>> > + * tests must be excluded.
>> > + *
>> > + * The only thing this macro does that's different from
>> > kunit_test_suites is
>> > + * that it suffixes the array and suite declarations it makes with
>> > _probe;
>> > + * modpost suppresses warnings about referencing init data for symbols
>> > named in
>> > + * this manner.
>> > + */
>> > +#define kunit_test_init_suites(__suites...)                          \
>> > +     __kunit_test_suites(CONCATENATE(__UNIQUE_ID(array), _probe),    \
>> > +                         CONCATENATE(__UNIQUE_ID(suites), _probe),   \
>> > +                         ##__suites)
>> > +
>> > +#define kunit_test_init_suite(suite) kunit_test_init_suites(&suite)
>> > +
>> >   #define kunit_suite_for_each_test_case(suite, test_case)            \
>> >       for (test_case = suite->test_cases; test_case->run_case;
>> > test_case++)
>> >
>> >
>>
>> The naming of the function and macro are rather confusing and can become
>> error prone. Let's find better naming scheme.
>
> Yeah, I wasn't sure about the name. I didn't have any better ideas
> initially though. Any suggestions?
>

What about kunit_test_init_section_suite?

>> > base-commit: 330f4c53d3c2d8b11d86ec03a964b86dc81452f5
>> >
>>
>> thanks,
>> -- Shuah
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] kunit: add support for kunit_suites that reference init code
  2022-04-07 21:34     ` Martin Fernandez
@ 2022-04-08 17:34       ` Brendan Higgins
  2022-04-08 18:50         ` Shuah Khan
  0 siblings, 1 reply; 6+ messages in thread
From: Brendan Higgins @ 2022-04-08 17:34 UTC (permalink / raw)
  To: Martin Fernandez
  Cc: Shuah Khan, shuah, davidgow, dlatypov, daniel.gutson,
	linux-kselftest, kunit-dev, linux-kernel, keescook, jk

On Thu, Apr 7, 2022 at 5:34 PM Martin Fernandez
<martin.fernandez@eclypsium.com> wrote:
>
> On 4/4/22, Brendan Higgins <brendanhiggins@google.com> wrote:
> > On Mon, Apr 4, 2022 at 6:37 PM Shuah Khan <skhan@linuxfoundation.org>
> > wrote:
> >>
> >> Hi Brendan,
> >>
> >> On 3/11/22 12:28 AM, Brendan Higgins wrote:
> >> > Add support for a new kind of kunit_suite registration macro called
> >> > kunit_test_init_suite(); this new registration macro allows the
> >> > registration of kunit_suites that reference functions marked __init and
> >> > data marked __initdata.
> >> >
> >> > Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
> >> > Tested-by: Martin Fernandez <martin.fernandez@eclypsium.com>
> >> > Reviewed-by: Kees Cook <keescook@chromium.org>
> >> > Reviewed-by: David Gow <davidgow@google.com>
> >> > ---
> >> >
> >>
> >> I almost applied it ...
> >>
> >> > This is a follow-up to the RFC here[1].
> >> >
> >> > This patch is in response to a KUnit user issue[2] in which the user
> >> > was
> >> > attempting to test some init functions; although this is a functional
> >> > solution as long as KUnit tests only run during the init phase, we will
> >> > need to do more work if we ever allow tests to run after the init phase
> >> > is over; it is for this reason that this patch adds a new registration
> >> > macro rather than simply modifying the existing macros.
> >> >
> >> > Changes since last version:
> >> >   - I added more to the kunit_test_init_suites() kernel-doc comment
> >> >     detailing "how" the modpost warnings are suppressed in addition to
> >> >     the existing information regarding "why" it is OK for the modpost
> >> >     warnings to be suppressed.
> >> >
> >> > [1]
> >> > https://lore.kernel.org/linux-kselftest/20220310210210.2124637-1-brendanhiggins@google.com/
> >> > [2] https://groups.google.com/g/kunit-dev/c/XDjieRHEneg/m/D0rFCwVABgAJ
> >> >
> >> > ---
> >> >   include/kunit/test.h | 26 ++++++++++++++++++++++++++
> >> >   1 file changed, 26 insertions(+)
> >> >
> >> > diff --git a/include/kunit/test.h b/include/kunit/test.h
> >> > index b26400731c02..7f303a06bc97 100644
> >> > --- a/include/kunit/test.h
> >> > +++ b/include/kunit/test.h
> >> > @@ -379,6 +379,32 @@ static inline int kunit_run_all_tests(void)
> >> >
> >> >   #define kunit_test_suite(suite)     kunit_test_suites(&suite)
> >> >
> >> > +/**
> >> > + * kunit_test_init_suites() - used to register one or more &struct
> >> > kunit_suite
> >> > + *                         containing init functions or init data.
> >> > + *
> >> > + * @__suites: a statically allocated list of &struct kunit_suite.
> >> > + *
> >> > + * This functions identically as &kunit_test_suites() except that it
> >> > suppresses
> >> > + * modpost warnings for referencing functions marked __init or data
> >> > marked
> >> > + * __initdata; this is OK because currently KUnit only runs tests upon
> >> > boot
> >> > + * during the init phase or upon loading a module during the init
> >> > phase.
> >> > + *
> >> > + * NOTE TO KUNIT DEVS: If we ever allow KUnit tests to be run after
> >> > boot, these
> >> > + * tests must be excluded.
> >> > + *
> >> > + * The only thing this macro does that's different from
> >> > kunit_test_suites is
> >> > + * that it suffixes the array and suite declarations it makes with
> >> > _probe;
> >> > + * modpost suppresses warnings about referencing init data for symbols
> >> > named in
> >> > + * this manner.
> >> > + */
> >> > +#define kunit_test_init_suites(__suites...)                          \
> >> > +     __kunit_test_suites(CONCATENATE(__UNIQUE_ID(array), _probe),    \
> >> > +                         CONCATENATE(__UNIQUE_ID(suites), _probe),   \
> >> > +                         ##__suites)
> >> > +
> >> > +#define kunit_test_init_suite(suite) kunit_test_init_suites(&suite)
> >> > +
> >> >   #define kunit_suite_for_each_test_case(suite, test_case)            \
> >> >       for (test_case = suite->test_cases; test_case->run_case;
> >> > test_case++)
> >> >
> >> >
> >>
> >> The naming of the function and macro are rather confusing and can become
> >> error prone. Let's find better naming scheme.
> >
> > Yeah, I wasn't sure about the name. I didn't have any better ideas
> > initially though. Any suggestions?
> >
>
> What about kunit_test_init_section_suite?

Sounds fine to me. Shuah, does that sound OK to you?

> >> > base-commit: 330f4c53d3c2d8b11d86ec03a964b86dc81452f5
> >> >
> >>
> >> thanks,
> >> -- Shuah
> >

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] kunit: add support for kunit_suites that reference init code
  2022-04-08 17:34       ` Brendan Higgins
@ 2022-04-08 18:50         ` Shuah Khan
  0 siblings, 0 replies; 6+ messages in thread
From: Shuah Khan @ 2022-04-08 18:50 UTC (permalink / raw)
  To: Brendan Higgins, Martin Fernandez
  Cc: shuah, davidgow, dlatypov, daniel.gutson, linux-kselftest,
	kunit-dev, linux-kernel, keescook, jk, Shuah Khan

On 4/8/22 11:34 AM, Brendan Higgins wrote:
> On Thu, Apr 7, 2022 at 5:34 PM Martin Fernandez
> <martin.fernandez@eclypsium.com> wrote:
>>
>> On 4/4/22, Brendan Higgins <brendanhiggins@google.com> wrote:
>>> On Mon, Apr 4, 2022 at 6:37 PM Shuah Khan <skhan@linuxfoundation.org>
>>> wrote:
>>>>
>>>> Hi Brendan,
>>>>
>>>> On 3/11/22 12:28 AM, Brendan Higgins wrote:
>>>>> Add support for a new kind of kunit_suite registration macro called
>>>>> kunit_test_init_suite(); this new registration macro allows the
>>>>> registration of kunit_suites that reference functions marked __init and
>>>>> data marked __initdata.
>>>>>
>>>>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>>>>> Tested-by: Martin Fernandez <martin.fernandez@eclypsium.com>
>>>>> Reviewed-by: Kees Cook <keescook@chromium.org>
>>>>> Reviewed-by: David Gow <davidgow@google.com>
>>>>> ---
>>>>>
>>>>
>>>> I almost applied it ...
>>>>
>>>>> This is a follow-up to the RFC here[1].
>>>>>
>>>>> This patch is in response to a KUnit user issue[2] in which the user
>>>>> was
>>>>> attempting to test some init functions; although this is a functional
>>>>> solution as long as KUnit tests only run during the init phase, we will
>>>>> need to do more work if we ever allow tests to run after the init phase
>>>>> is over; it is for this reason that this patch adds a new registration
>>>>> macro rather than simply modifying the existing macros.
>>>>>
>>>>> Changes since last version:
>>>>>    - I added more to the kunit_test_init_suites() kernel-doc comment
>>>>>      detailing "how" the modpost warnings are suppressed in addition to
>>>>>      the existing information regarding "why" it is OK for the modpost
>>>>>      warnings to be suppressed.
>>>>>
>>>>> [1]
>>>>> https://lore.kernel.org/linux-kselftest/20220310210210.2124637-1-brendanhiggins@google.com/
>>>>> [2] https://groups.google.com/g/kunit-dev/c/XDjieRHEneg/m/D0rFCwVABgAJ
>>>>>
>>>>> ---
>>>>>    include/kunit/test.h | 26 ++++++++++++++++++++++++++
>>>>>    1 file changed, 26 insertions(+)
>>>>>
>>>>> diff --git a/include/kunit/test.h b/include/kunit/test.h
>>>>> index b26400731c02..7f303a06bc97 100644
>>>>> --- a/include/kunit/test.h
>>>>> +++ b/include/kunit/test.h
>>>>> @@ -379,6 +379,32 @@ static inline int kunit_run_all_tests(void)
>>>>>
>>>>>    #define kunit_test_suite(suite)     kunit_test_suites(&suite)
>>>>>
>>>>> +/**
>>>>> + * kunit_test_init_suites() - used to register one or more &struct
>>>>> kunit_suite
>>>>> + *                         containing init functions or init data.
>>>>> + *
>>>>> + * @__suites: a statically allocated list of &struct kunit_suite.
>>>>> + *
>>>>> + * This functions identically as &kunit_test_suites() except that it
>>>>> suppresses
>>>>> + * modpost warnings for referencing functions marked __init or data
>>>>> marked
>>>>> + * __initdata; this is OK because currently KUnit only runs tests upon
>>>>> boot
>>>>> + * during the init phase or upon loading a module during the init
>>>>> phase.
>>>>> + *
>>>>> + * NOTE TO KUNIT DEVS: If we ever allow KUnit tests to be run after
>>>>> boot, these
>>>>> + * tests must be excluded.
>>>>> + *
>>>>> + * The only thing this macro does that's different from
>>>>> kunit_test_suites is
>>>>> + * that it suffixes the array and suite declarations it makes with
>>>>> _probe;
>>>>> + * modpost suppresses warnings about referencing init data for symbols
>>>>> named in
>>>>> + * this manner.
>>>>> + */
>>>>> +#define kunit_test_init_suites(__suites...)                          \
>>>>> +     __kunit_test_suites(CONCATENATE(__UNIQUE_ID(array), _probe),    \
>>>>> +                         CONCATENATE(__UNIQUE_ID(suites), _probe),   \
>>>>> +                         ##__suites)
>>>>> +
>>>>> +#define kunit_test_init_suite(suite) kunit_test_init_suites(&suite)
>>>>> +
>>>>>    #define kunit_suite_for_each_test_case(suite, test_case)            \
>>>>>        for (test_case = suite->test_cases; test_case->run_case;
>>>>> test_case++)
>>>>>
>>>>>
>>>>
>>>> The naming of the function and macro are rather confusing and can become
>>>> error prone. Let's find better naming scheme.
>>>
>>> Yeah, I wasn't sure about the name. I didn't have any better ideas
>>> initially though. Any suggestions?
>>>
>>
>> What about kunit_test_init_section_suite?
> 
> Sounds fine to me. Shuah, does that sound OK to you?
> 

Sorry for the delay in responding.

As long as the two names are different enough to tell them apart.
The proposed name does that.

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-04-08 18:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-11  7:28 [PATCH v1] kunit: add support for kunit_suites that reference init code Brendan Higgins
2022-04-04 22:37 ` Shuah Khan
2022-04-04 22:48   ` Brendan Higgins
2022-04-07 21:34     ` Martin Fernandez
2022-04-08 17:34       ` Brendan Higgins
2022-04-08 18:50         ` Shuah Khan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.