linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC v1] kunit: add support for kunit_suites that reference init code
@ 2022-03-10 21:02 Brendan Higgins
  2022-03-10 21:42 ` Martin Fernandez
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Brendan Higgins @ 2022-03-10 21:02 UTC (permalink / raw)
  To: shuah, davidgow, dlatypov, martin.fernandez, daniel.gutson
  Cc: linux-kselftest, kunit-dev, linux-kernel, keescook, 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>
---

This patch is in response to a KUnit user issue[1] 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.

[1] https://groups.google.com/g/kunit-dev/c/XDjieRHEneg/m/D0rFCwVABgAJ

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

diff --git a/include/kunit/test.h b/include/kunit/test.h
index b26400731c02..1878e585f6d3 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -379,6 +379,27 @@ 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.
+ */
+#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] 10+ messages in thread

* Re: [RFC v1] kunit: add support for kunit_suites that reference init code
  2022-03-10 21:02 [RFC v1] kunit: add support for kunit_suites that reference init code Brendan Higgins
@ 2022-03-10 21:42 ` Martin Fernandez
  2022-03-10 22:26 ` Kees Cook
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Martin Fernandez @ 2022-03-10 21:42 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: shuah, davidgow, dlatypov, daniel.gutson, linux-kselftest,
	kunit-dev, linux-kernel, keescook

On 3/10/22, Brendan Higgins <brendanhiggins@google.com> 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>
> ---
>
> This patch is in response to a KUnit user issue[1] 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.
>
> [1] https://groups.google.com/g/kunit-dev/c/XDjieRHEneg/m/D0rFCwVABgAJ
>
> ---
>  include/kunit/test.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index b26400731c02..1878e585f6d3 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -379,6 +379,27 @@ 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.
> + */
> +#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
>
>

Thanks for the feature :)

Tested-by: Martin Fernandez <martin.fernandez@eclypsium.com>

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

* Re: [RFC v1] kunit: add support for kunit_suites that reference init code
  2022-03-10 21:02 [RFC v1] kunit: add support for kunit_suites that reference init code Brendan Higgins
  2022-03-10 21:42 ` Martin Fernandez
@ 2022-03-10 22:26 ` Kees Cook
  2022-03-10 22:49 ` Brendan Higgins
  2022-03-11  7:02 ` David Gow
  3 siblings, 0 replies; 10+ messages in thread
From: Kees Cook @ 2022-03-10 22:26 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: shuah, davidgow, dlatypov, martin.fernandez, daniel.gutson,
	linux-kselftest, kunit-dev, linux-kernel

On Thu, Mar 10, 2022 at 01:02:10PM -0800, 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.

O_o is this due to the "_probe" name being used? I think that likely
deserves a comment in the code, so the "how" of the warning suppression
is clear.

> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>

Regardless:

Reviewed-by: Kees Cook <keescook@chromium.org>

> ---
> 
> This patch is in response to a KUnit user issue[1] 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.
> 
> [1] https://groups.google.com/g/kunit-dev/c/XDjieRHEneg/m/D0rFCwVABgAJ
> 
> ---
>  include/kunit/test.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index b26400731c02..1878e585f6d3 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -379,6 +379,27 @@ 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.
> + */
> +#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
> 

-- 
Kees Cook

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

* Re: [RFC v1] kunit: add support for kunit_suites that reference init code
  2022-03-10 21:02 [RFC v1] kunit: add support for kunit_suites that reference init code Brendan Higgins
  2022-03-10 21:42 ` Martin Fernandez
  2022-03-10 22:26 ` Kees Cook
@ 2022-03-10 22:49 ` Brendan Higgins
  2022-03-10 22:50   ` Brendan Higgins
  2022-03-11  7:02 ` David Gow
  3 siblings, 1 reply; 10+ messages in thread
From: Brendan Higgins @ 2022-03-10 22:49 UTC (permalink / raw)
  To: shuah, David Gow, Daniel Latypov, Martin Fernandez, Daniel Gutson
  Cc: linux-kselftest, kunit-dev, linux-kernel, keescook

+Jeremy Kerr - Just remembered that Jeremy is doing some work here and
might be somewhat interested.

On Thu, Mar 10, 2022 at 4:02 PM Brendan Higgins
<brendanhiggins@google.com> 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>
> ---
>
> This patch is in response to a KUnit user issue[1] 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.
>
> [1] https://groups.google.com/g/kunit-dev/c/XDjieRHEneg/m/D0rFCwVABgAJ
>
> ---
>  include/kunit/test.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index b26400731c02..1878e585f6d3 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -379,6 +379,27 @@ 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.
> + */
> +#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	[flat|nested] 10+ messages in thread

* Re: [RFC v1] kunit: add support for kunit_suites that reference init code
  2022-03-10 22:49 ` Brendan Higgins
@ 2022-03-10 22:50   ` Brendan Higgins
  0 siblings, 0 replies; 10+ messages in thread
From: Brendan Higgins @ 2022-03-10 22:50 UTC (permalink / raw)
  To: shuah, David Gow, Daniel Latypov, Martin Fernandez,
	Daniel Gutson, Jeremy Kerr
  Cc: linux-kselftest, kunit-dev, linux-kernel, keescook

Actually add Jeremy this time. Sorry for the spam.

On Thu, Mar 10, 2022 at 5:49 PM Brendan Higgins
<brendanhiggins@google.com> wrote:
>
> +Jeremy Kerr - Just remembered that Jeremy is doing some work here and
> might be somewhat interested.
>
> On Thu, Mar 10, 2022 at 4:02 PM Brendan Higgins
> <brendanhiggins@google.com> 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>
> > ---
> >
> > This patch is in response to a KUnit user issue[1] 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.
> >
> > [1] https://groups.google.com/g/kunit-dev/c/XDjieRHEneg/m/D0rFCwVABgAJ
> >
> > ---
> >  include/kunit/test.h | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > index b26400731c02..1878e585f6d3 100644
> > --- a/include/kunit/test.h
> > +++ b/include/kunit/test.h
> > @@ -379,6 +379,27 @@ 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.
> > + */
> > +#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	[flat|nested] 10+ messages in thread

* Re: [RFC v1] kunit: add support for kunit_suites that reference init code
  2022-03-10 21:02 [RFC v1] kunit: add support for kunit_suites that reference init code Brendan Higgins
                   ` (2 preceding siblings ...)
  2022-03-10 22:49 ` Brendan Higgins
@ 2022-03-11  7:02 ` David Gow
       [not found]   ` <CAFmMkTFa9xVt314WEGd0nNx+ovc=aGB_yN1LorP7WrBga9quxw@mail.gmail.com>
  3 siblings, 1 reply; 10+ messages in thread
From: David Gow @ 2022-03-11  7:02 UTC (permalink / raw)
  To: Brendan Higgins
  Cc: shuah, dlatypov, martin.fernandez, daniel.gutson,
	linux-kselftest, kunit-dev, linux-kernel, keescook

[-- Attachment #1: Type: text/plain, Size: 2809 bytes --]

On Thu, Mar 10, 2022 at 01:02:10PM -0800, 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>
> ---
> 
> This patch is in response to a KUnit user issue[1] 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.
> 
> [1] https://groups.google.com/g/kunit-dev/c/XDjieRHEneg/m/D0rFCwVABgAJ
> 
> ---

I'm a little concerned that this is just removing the warnings, but do
agree that this is safe enough for the moment. At least the information
about which tests need __init is preserved by the use of a different
macro.

I guess one day we'll need a second list of 'init' tests or something...

Anyway,

Reviewed-by: David Gow <davidgow@google.com>

-- David

>  include/kunit/test.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index b26400731c02..1878e585f6d3 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -379,6 +379,27 @@ 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.
> + */
> +#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
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]

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

* Re: [RFC v1] kunit: add support for kunit_suites that reference init code
       [not found]   ` <CAFmMkTFa9xVt314WEGd0nNx+ovc=aGB_yN1LorP7WrBga9quxw@mail.gmail.com>
@ 2022-03-11 17:56     ` Daniel Latypov
  2022-03-25 13:25       ` Daniel Gutson
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Latypov @ 2022-03-11 17:56 UTC (permalink / raw)
  To: Daniel Gutson
  Cc: David Gow, Brendan Higgins, shuah, Martin Fernandez,
	linux-kselftest, kunit-dev, linux-kernel, Kees Cook

On Fri, Mar 11, 2022 at 4:14 AM Daniel Gutson
<daniel.gutson@eclypsium.com> wrote:
>
>
>
> El vie., 11 mar. 2022 4:02 a. m., David Gow <davidgow@google.com> escribió:
>>
>> On Thu, Mar 10, 2022 at 01:02:10PM -0800, 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>
>> > ---
>> >
>> > This patch is in response to a KUnit user issue[1] 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.
>> >
>> > [1] https://groups.google.com/g/kunit-dev/c/XDjieRHEneg/m/D0rFCwVABgAJ
>> >
>> > ---
>>
>> I'm a little concerned that this is just removing the warnings, but do
>> agree that this is safe enough for the moment. At least the information
>> about which tests need __init is preserved by the use of a different
>> macro.
>>
>> I guess one day we'll need a second list of 'init' tests or something...
>
>
> Hi, could you please detail about this? Why a second list?
>

I assume this is referring to a future where we want to run tests
_after_ the init phase.
In that case, we'd need to be able to separately register tests that
run during and those that run after.
(Or we could have one list and just tag each suite as init/post-init.
If we ever had >2 "phases" where we run tests, this might be the more
scalable option)

Is it likely we'd have tests run after?
Not in the near future, I don't think. But it could be asked for.

For context, here's where built-in KUnit tests currently run:
https://elixir.bootlin.com/linux/v5.17-rc7/source/init/main.c#L1615
That'd probably become kunit_run_init_tests() and then we'd have
another kunit_run_post_init_tests() called later, or something.

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

* Re: [RFC v1] kunit: add support for kunit_suites that reference init code
  2022-03-11 17:56     ` Daniel Latypov
@ 2022-03-25 13:25       ` Daniel Gutson
  2022-03-25 15:59         ` Daniel Latypov
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Gutson @ 2022-03-25 13:25 UTC (permalink / raw)
  To: Daniel Latypov
  Cc: David Gow, Brendan Higgins, shuah, Martin Fernandez,
	linux-kselftest, kunit-dev, linux-kernel, Kees Cook,
	Richard Hughes

On Fri, Mar 11, 2022 at 2:56 PM Daniel Latypov <dlatypov@google.com> wrote:
>
> On Fri, Mar 11, 2022 at 4:14 AM Daniel Gutson
> <daniel.gutson@eclypsium.com> wrote:
> >
> >
> >
> > El vie., 11 mar. 2022 4:02 a. m., David Gow <davidgow@google.com> escribió:
> >>
> >> On Thu, Mar 10, 2022 at 01:02:10PM -0800, 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>
> >> > ---
> >> >
> >> > This patch is in response to a KUnit user issue[1] 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.
> >> >
> >> > [1] https://groups.google.com/g/kunit-dev/c/XDjieRHEneg/m/D0rFCwVABgAJ
> >> >
> >> > ---
> >>
> >> I'm a little concerned that this is just removing the warnings, but do
> >> agree that this is safe enough for the moment. At least the information
> >> about which tests need __init is preserved by the use of a different
> >> macro.
> >>
> >> I guess one day we'll need a second list of 'init' tests or something...
> >
> >
> > Hi, could you please detail about this? Why a second list?
> >
>
> I assume this is referring to a future where we want to run tests
> _after_ the init phase.
> In that case, we'd need to be able to separately register tests that
> run during and those that run after.
> (Or we could have one list and just tag each suite as init/post-init.
> If we ever had >2 "phases" where we run tests, this might be the more
> scalable option)
>
> Is it likely we'd have tests run after?
> Not in the near future, I don't think. But it could be asked for.
>
> For context, here's where built-in KUnit tests currently run:
> https://elixir.bootlin.com/linux/v5.17-rc7/source/init/main.c#L1615
> That'd probably become kunit_run_init_tests() and then we'd have
> another kunit_run_post_init_tests() called later, or something.

Hi folks, any update on this? I'm adding Richard Hughes since we
need this for fwupd/LVFS, so he can provide more context.

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

* Re: [RFC v1] kunit: add support for kunit_suites that reference init code
  2022-03-25 13:25       ` Daniel Gutson
@ 2022-03-25 15:59         ` Daniel Latypov
  2022-03-28 17:04           ` Brendan Higgins
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Latypov @ 2022-03-25 15:59 UTC (permalink / raw)
  To: Daniel Gutson
  Cc: David Gow, Brendan Higgins, shuah, Martin Fernandez,
	linux-kselftest, kunit-dev, linux-kernel, Kees Cook,
	Richard Hughes

On Fri, Mar 25, 2022 at 8:26 AM Daniel Gutson
<daniel.gutson@eclypsium.com> wrote:
>
> On Fri, Mar 11, 2022 at 2:56 PM Daniel Latypov <dlatypov@google.com> wrote:
> >
> > On Fri, Mar 11, 2022 at 4:14 AM Daniel Gutson
> > <daniel.gutson@eclypsium.com> wrote:
> > >
> > >
> > >
> > > El vie., 11 mar. 2022 4:02 a. m., David Gow <davidgow@google.com> escribió:
> > >>
> > >> On Thu, Mar 10, 2022 at 01:02:10PM -0800, 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>
> > >> > ---
> > >> >
> > >> > This patch is in response to a KUnit user issue[1] 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.
> > >> >
> > >> > [1] https://groups.google.com/g/kunit-dev/c/XDjieRHEneg/m/D0rFCwVABgAJ
> > >> >
> > >> > ---
> > >>
> > >> I'm a little concerned that this is just removing the warnings, but do
> > >> agree that this is safe enough for the moment. At least the information
> > >> about which tests need __init is preserved by the use of a different
> > >> macro.
> > >>
> > >> I guess one day we'll need a second list of 'init' tests or something...
> > >
> > >
> > > Hi, could you please detail about this? Why a second list?
> > >
> >
> > I assume this is referring to a future where we want to run tests
> > _after_ the init phase.
> > In that case, we'd need to be able to separately register tests that
> > run during and those that run after.
> > (Or we could have one list and just tag each suite as init/post-init.
> > If we ever had >2 "phases" where we run tests, this might be the more
> > scalable option)
> >
> > Is it likely we'd have tests run after?
> > Not in the near future, I don't think. But it could be asked for.
> >
> > For context, here's where built-in KUnit tests currently run:
> > https://elixir.bootlin.com/linux/v5.17-rc7/source/init/main.c#L1615
> > That'd probably become kunit_run_init_tests() and then we'd have
> > another kunit_run_post_init_tests() called later, or something.
>
> Hi folks, any update on this? I'm adding Richard Hughes since we
> need this for fwupd/LVFS, so he can provide more context.

v1 of the patch was posted here:
https://lore.kernel.org/linux-kselftest/20220311072859.2174624-1-brendanhiggins@google.com/

It has the requisite Reviewed-by's and no one has complained about it.
So we're now waiting for that to get picked up into Shuah's tree and
into Linus' for 5.18 (possibly) or 5.19.

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

* Re: [RFC v1] kunit: add support for kunit_suites that reference init code
  2022-03-25 15:59         ` Daniel Latypov
@ 2022-03-28 17:04           ` Brendan Higgins
  0 siblings, 0 replies; 10+ messages in thread
From: Brendan Higgins @ 2022-03-28 17:04 UTC (permalink / raw)
  To: Daniel Latypov
  Cc: Daniel Gutson, David Gow, shuah, Martin Fernandez,
	linux-kselftest, kunit-dev, linux-kernel, Kees Cook,
	Richard Hughes

On Fri, Mar 25, 2022 at 11:59 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> On Fri, Mar 25, 2022 at 8:26 AM Daniel Gutson
> <daniel.gutson@eclypsium.com> wrote:
> >
> > On Fri, Mar 11, 2022 at 2:56 PM Daniel Latypov <dlatypov@google.com> wrote:
> > >
> > > On Fri, Mar 11, 2022 at 4:14 AM Daniel Gutson
> > > <daniel.gutson@eclypsium.com> wrote:
> > > >
> > > >
> > > >
> > > > El vie., 11 mar. 2022 4:02 a. m., David Gow <davidgow@google.com> escribió:
> > > >>
> > > >> On Thu, Mar 10, 2022 at 01:02:10PM -0800, 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>
> > > >> > ---
> > > >> >
> > > >> > This patch is in response to a KUnit user issue[1] 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.
> > > >> >
> > > >> > [1] https://groups.google.com/g/kunit-dev/c/XDjieRHEneg/m/D0rFCwVABgAJ
> > > >> >
> > > >> > ---
> > > >>
> > > >> I'm a little concerned that this is just removing the warnings, but do
> > > >> agree that this is safe enough for the moment. At least the information
> > > >> about which tests need __init is preserved by the use of a different
> > > >> macro.
> > > >>
> > > >> I guess one day we'll need a second list of 'init' tests or something...
> > > >
> > > >
> > > > Hi, could you please detail about this? Why a second list?
> > > >
> > >
> > > I assume this is referring to a future where we want to run tests
> > > _after_ the init phase.
> > > In that case, we'd need to be able to separately register tests that
> > > run during and those that run after.
> > > (Or we could have one list and just tag each suite as init/post-init.
> > > If we ever had >2 "phases" where we run tests, this might be the more
> > > scalable option)
> > >
> > > Is it likely we'd have tests run after?
> > > Not in the near future, I don't think. But it could be asked for.
> > >
> > > For context, here's where built-in KUnit tests currently run:
> > > https://elixir.bootlin.com/linux/v5.17-rc7/source/init/main.c#L1615
> > > That'd probably become kunit_run_init_tests() and then we'd have
> > > another kunit_run_post_init_tests() called later, or something.
> >
> > Hi folks, any update on this? I'm adding Richard Hughes since we
> > need this for fwupd/LVFS, so he can provide more context.
>
> v1 of the patch was posted here:
> https://lore.kernel.org/linux-kselftest/20220311072859.2174624-1-brendanhiggins@google.com/
>
> It has the requisite Reviewed-by's and no one has complained about it.
> So we're now waiting for that to get picked up into Shuah's tree and
> into Linus' for 5.18 (possibly) or 5.19.

It'll probably be 5.19, but it should be applied to Shuah's kunit-next
branch end of this week begining of next and then you can use that as
a base if you wish.

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

end of thread, other threads:[~2022-03-28 17:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-10 21:02 [RFC v1] kunit: add support for kunit_suites that reference init code Brendan Higgins
2022-03-10 21:42 ` Martin Fernandez
2022-03-10 22:26 ` Kees Cook
2022-03-10 22:49 ` Brendan Higgins
2022-03-10 22:50   ` Brendan Higgins
2022-03-11  7:02 ` David Gow
     [not found]   ` <CAFmMkTFa9xVt314WEGd0nNx+ovc=aGB_yN1LorP7WrBga9quxw@mail.gmail.com>
2022-03-11 17:56     ` Daniel Latypov
2022-03-25 13:25       ` Daniel Gutson
2022-03-25 15:59         ` Daniel Latypov
2022-03-28 17:04           ` Brendan Higgins

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).