2022-03-11 21:03:41

by Daniel Latypov

[permalink] [raw]
Subject: Re: [RFC v1] kunit: add support for kunit_suites that reference init code

On Fri, Mar 11, 2022 at 4:14 AM Daniel Gutson
<[email protected]> wrote:
>
>
>
> El vie., 11 mar. 2022 4:02 a. m., David Gow <[email protected]> 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 <[email protected]>
>> > ---
>> >
>> > 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.


2022-03-25 19:37:05

by Daniel Gutson

[permalink] [raw]
Subject: Re: [RFC v1] kunit: add support for kunit_suites that reference init code

On Fri, Mar 11, 2022 at 2:56 PM Daniel Latypov <[email protected]> wrote:
>
> On Fri, Mar 11, 2022 at 4:14 AM Daniel Gutson
> <[email protected]> wrote:
> >
> >
> >
> > El vie., 11 mar. 2022 4:02 a. m., David Gow <[email protected]> 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 <[email protected]>
> >> > ---
> >> >
> >> > 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.

2022-03-25 19:37:23

by Daniel Latypov

[permalink] [raw]
Subject: Re: [RFC v1] kunit: add support for kunit_suites that reference init code

On Fri, Mar 25, 2022 at 8:26 AM Daniel Gutson
<[email protected]> wrote:
>
> On Fri, Mar 11, 2022 at 2:56 PM Daniel Latypov <[email protected]> wrote:
> >
> > On Fri, Mar 11, 2022 at 4:14 AM Daniel Gutson
> > <[email protected]> wrote:
> > >
> > >
> > >
> > > El vie., 11 mar. 2022 4:02 a. m., David Gow <[email protected]> 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 <[email protected]>
> > >> > ---
> > >> >
> > >> > 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/[email protected]/

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.

2022-03-28 22:18:26

by Brendan Higgins

[permalink] [raw]
Subject: Re: [RFC v1] kunit: add support for kunit_suites that reference init code

On Fri, Mar 25, 2022 at 11:59 AM Daniel Latypov <[email protected]> wrote:
>
> On Fri, Mar 25, 2022 at 8:26 AM Daniel Gutson
> <[email protected]> wrote:
> >
> > On Fri, Mar 11, 2022 at 2:56 PM Daniel Latypov <[email protected]> wrote:
> > >
> > > On Fri, Mar 11, 2022 at 4:14 AM Daniel Gutson
> > > <[email protected]> wrote:
> > > >
> > > >
> > > >
> > > > El vie., 11 mar. 2022 4:02 a. m., David Gow <[email protected]> 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 <[email protected]>
> > > >> > ---
> > > >> >
> > > >> > 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/[email protected]/
>
> 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.