| Message ID | 20191216220555.245089-1-brendanhiggins@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | kunit: create a centralized executor to dispatch all KUnit tests | expand |
On Mon, Dec 16, 2019 at 02:05:49PM -0800, Brendan Higgins wrote: > ## TL;DR > > This patchset adds a centralized executor to dispatch tests rather than > relying on late_initcall to schedule each test suite separately along > with a couple of new features that depend on it. > > ## What am I trying to do? > > Conceptually, I am trying to provide a mechanism by which test suites > can be grouped together so that they can be reasoned about collectively. > The last two patches in this series add features which depend on this: > > RFC 5/6 Prints out a test plan right before KUnit tests are run[1]; this > is valuable because it makes it possible for a test harness to > detect whether the number of tests run matches the number of > tests expected to be run, ensuring that no tests silently > failed. > > RFC 6/6 Add a new kernel command-line option which allows the user to > specify that the kernel poweroff, halt, or reboot after > completing all KUnit tests; this is very handy for running KUnit > tests on UML or a VM so that the UML/VM process exits cleanly > immediately after running all tests without needing a special > initramfs. The approach seems sensible to me given that it separates from a semantics perspective kernel subsystem init work from *testing*, and so we are sure we'd run the *test* stuff *after* all subsystem init stuff. Dispatching, however is still immediate, and with a bit of work, this dispatcher could be configurable to run at an arbirary time after boot. If there are not immediate use cases for that though, then I suppose this is not a requirement for the dispatcher. But since there exists another modular test framework with its own dispatcher and it seems the goal is to merge the work long term, this might preempt the requirement to define how and when we can dispatch tests post boot. And, if we're going to do that, I can suggest that a data structure instead of just a function init call be used to describe tests to be placed into an ELF section. With my linker table work this would be easy, I define section ranges for code describing only executable routines, but it defines linker tables for when a component in the kernel would define a data structure, part of which can be a callback. Such data structure stuffed into an ELF section could allow dynamic configuration of the dipsatching, even post boot. I think this is a good stepping stone forward then, and to allow dynamic configuration of the dispatcher could mean eventual extensions to kunit's init stuff to stuff init calls into a data structure which can then allow configuration of the dispatching. One benefit that the linker table work *may* be able to help here with is that it allows an easy way to create kunit specific ordering, at linker time. There is also an example of addressing / generalizing dynamic / run time changes of ordering, by using the x86 IOMMU initialization as an example case. We don't have an easy way to do this today, but if kunit could benefit from such framework, it'd be another use case for the linker table work. That is, the ability to easilly allow dynamically modifying run time ordering of code through ELF sections. > In addition, by dispatching tests from a single location, we can > guarantee that all KUnit tests run after late_init is complete, which > was a concern during the initial KUnit patchset review (this has not > been a problem in practice, but resolving with certainty is nevertheless > desirable). Indeed, the concern is just a real semantics limitations. With the tests *always* running after all subsystem init stuff, we know we'd have a real full kernel ready. It does beg the question if this means kunit is happy to not be a tool to test pre basic setup stuff (terminology used in init.c, meaning prior to running all init levels). I suspect this is the case. > Other use cases for this exist, but the above features should provide an > idea of the value that this could provide. > > ## What work remains to be done? > > These patches were based on patches in our non-upstream branch[2], so we > have a pretty good idea that they are useable as presented; > nevertheless, some of the changes done in this patchset could > *definitely* use some review by subsystem experts (linker scripts, init, > etc), and will likely change a lot after getting feedback. > > The biggest thing that I know will require additional attention is > integrating this patchset with the KUnit module support patchset[3]. I > have not even attempted to build these patches on top of the module > support patches as I would like to get people's initial thoughts first > (especially Alan's :-) ). I think that making these patches work with > module support should be fairly straight forward, nevertheless. Modules just have their own sections too. That's all. So it'd be a matter of extending the linker script for modules too. But a module's init is different than the core kernel's for vmlinux. Luis
Sorry for the late reply. I am still catching up from being on vacation. On Mon, Jan 6, 2020 at 2:40 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Mon, Dec 16, 2019 at 02:05:49PM -0800, Brendan Higgins wrote: > > ## TL;DR > > > > This patchset adds a centralized executor to dispatch tests rather than > > relying on late_initcall to schedule each test suite separately along > > with a couple of new features that depend on it. > > > > ## What am I trying to do? > > > > Conceptually, I am trying to provide a mechanism by which test suites > > can be grouped together so that they can be reasoned about collectively. > > The last two patches in this series add features which depend on this: > > > > RFC 5/6 Prints out a test plan right before KUnit tests are run[1]; this > > is valuable because it makes it possible for a test harness to > > detect whether the number of tests run matches the number of > > tests expected to be run, ensuring that no tests silently > > failed. > > > > RFC 6/6 Add a new kernel command-line option which allows the user to > > specify that the kernel poweroff, halt, or reboot after > > completing all KUnit tests; this is very handy for running KUnit > > tests on UML or a VM so that the UML/VM process exits cleanly > > immediately after running all tests without needing a special > > initramfs. > > The approach seems sensible to me given that it separates from a > semantics perspective kernel subsystem init work from *testing*, and > so we are sure we'd run the *test* stuff *after* all subsystem init > stuff. Cool, I thought you would find this interesting. > Dispatching, however is still immediate, and with a bit of work, this > dispatcher could be configurable to run at an arbirary time after boot. > If there are not immediate use cases for that though, then I suppose > this is not a requirement for the dispatcher. But since there exists > another modular test framework with its own dispatcher and it seems the > goal is to merge the work long term, this might preempt the requirement > to define how and when we can dispatch tests post boot. > > And, if we're going to do that, I can suggest that a data structure > instead of just a function init call be used to describe tests to be > placed into an ELF section. With my linker table work this would be > easy, I define section ranges for code describing only executable > routines, but it defines linker tables for when a component in the > kernel would define a data structure, part of which can be a callback. > Such data structure stuffed into an ELF section could allow dynamic > configuration of the dipsatching, even post boot. The linker table work does sound interesting. Do you have a link? I was thinking about dynamic dispatching, actually. I thought it would be handy to be able to build all tests into a single kernel and then run different tests on different invocations. Also, for post boot dynamic dispatching, you should check out Alan's debugfs patches: https://lore.kernel.org/linux-kselftest/CAFd5g46657gZ36PaP8Pi999hPPgBU2Kz94nrMspS-AzGwdBF+g@mail.gmail.com/T/#m210cadbeee267e5c5a9253d83b7b7ca723d1f871 They look pretty handy! > I think this is a good stepping stone forward then, and to allow > dynamic configuration of the dispatcher could mean eventual extensions > to kunit's init stuff to stuff init calls into a data structure which > can then allow configuration of the dispatching. One benefit that the > linker table work *may* be able to help here with is that it allows > an easy way to create kunit specific ordering, at linker time. > There is also an example of addressing / generalizing dynamic / run time > changes of ordering, by using the x86 IOMMU initialization as an > example case. We don't have an easy way to do this today, but if kunit > could benefit from such framework, it'd be another use case for > the linker table work. That is, the ability to easilly allow > dynamically modifying run time ordering of code through ELF sections. > > > In addition, by dispatching tests from a single location, we can > > guarantee that all KUnit tests run after late_init is complete, which > > was a concern during the initial KUnit patchset review (this has not > > been a problem in practice, but resolving with certainty is nevertheless > > desirable). > > Indeed, the concern is just a real semantics limitations. With the tests > *always* running after all subsystem init stuff, we know we'd have a > real full kernel ready. Yep. > It does beg the question if this means kunit is happy to not be a tool > to test pre basic setup stuff (terminology used in init.c, meaning prior > to running all init levels). I suspect this is the case. Not sure. I still haven't seen any cases where this is necessary, so I am not super worried about it. Regardless, I don't think this patchset really changes anything in that regard, we are moving from late_init to after late_init, so it isn't that big of a change for most use cases. Please share if you can think of some things that need to be tested in early init. > > Other use cases for this exist, but the above features should provide an > > idea of the value that this could provide. > > > > ## What work remains to be done? > > > > These patches were based on patches in our non-upstream branch[2], so we > > have a pretty good idea that they are useable as presented; > > nevertheless, some of the changes done in this patchset could > > *definitely* use some review by subsystem experts (linker scripts, init, > > etc), and will likely change a lot after getting feedback. > > > > The biggest thing that I know will require additional attention is > > integrating this patchset with the KUnit module support patchset[3]. I > > have not even attempted to build these patches on top of the module > > support patches as I would like to get people's initial thoughts first > > (especially Alan's :-) ). I think that making these patches work with > > module support should be fairly straight forward, nevertheless. > > Modules just have their own sections too. That's all. So it'd be a > matter of extending the linker script for modules too. But a module's > init is different than the core kernel's for vmlinux. Truth. It seems as though Alan has already fixed this for me, however.
On 1/23/20 4:40 PM, Brendan Higgins wrote: > Sorry for the late reply. I am still catching up from being on vacation. > > On Mon, Jan 6, 2020 at 2:40 PM Luis Chamberlain <mcgrof@kernel.org> wrote: >> >> On Mon, Dec 16, 2019 at 02:05:49PM -0800, Brendan Higgins wrote: >>> ## TL;DR >>> >>> This patchset adds a centralized executor to dispatch tests rather than >>> relying on late_initcall to schedule each test suite separately along >>> with a couple of new features that depend on it. >>> >>> ## What am I trying to do? >>> >>> Conceptually, I am trying to provide a mechanism by which test suites >>> can be grouped together so that they can be reasoned about collectively. >>> The last two patches in this series add features which depend on this: >>> >>> RFC 5/6 Prints out a test plan right before KUnit tests are run[1]; this >>> is valuable because it makes it possible for a test harness to >>> detect whether the number of tests run matches the number of >>> tests expected to be run, ensuring that no tests silently >>> failed. >>> >>> RFC 6/6 Add a new kernel command-line option which allows the user to >>> specify that the kernel poweroff, halt, or reboot after >>> completing all KUnit tests; this is very handy for running KUnit >>> tests on UML or a VM so that the UML/VM process exits cleanly >>> immediately after running all tests without needing a special >>> initramfs. >> >> The approach seems sensible to me given that it separates from a >> semantics perspective kernel subsystem init work from *testing*, and >> so we are sure we'd run the *test* stuff *after* all subsystem init >> stuff. > > Cool, I thought you would find this interesting. > >> Dispatching, however is still immediate, and with a bit of work, this >> dispatcher could be configurable to run at an arbirary time after boot. >> If there are not immediate use cases for that though, then I suppose >> this is not a requirement for the dispatcher. But since there exists >> another modular test framework with its own dispatcher and it seems the >> goal is to merge the work long term, this might preempt the requirement >> to define how and when we can dispatch tests post boot. >> >> And, if we're going to do that, I can suggest that a data structure >> instead of just a function init call be used to describe tests to be >> placed into an ELF section. With my linker table work this would be >> easy, I define section ranges for code describing only executable >> routines, but it defines linker tables for when a component in the >> kernel would define a data structure, part of which can be a callback. >> Such data structure stuffed into an ELF section could allow dynamic >> configuration of the dipsatching, even post boot. > > The linker table work does sound interesting. Do you have a link? > > I was thinking about dynamic dispatching, actually. I thought it would > be handy to be able to build all tests into a single kernel and then > run different tests on different invocations. > > Also, for post boot dynamic dispatching, you should check out Alan's > debugfs patches: > > https://lore.kernel.org/linux-kselftest/CAFd5g46657gZ36PaP8Pi999hPPgBU2Kz94nrMspS-AzGwdBF+g@mail.gmail.com/T/#m210cadbeee267e5c5a9253d83b7b7ca723d1f871 > > They look pretty handy! > >> I think this is a good stepping stone forward then, and to allow >> dynamic configuration of the dispatcher could mean eventual extensions >> to kunit's init stuff to stuff init calls into a data structure which >> can then allow configuration of the dispatching. One benefit that the >> linker table work *may* be able to help here with is that it allows >> an easy way to create kunit specific ordering, at linker time. >> There is also an example of addressing / generalizing dynamic / run time >> changes of ordering, by using the x86 IOMMU initialization as an >> example case. We don't have an easy way to do this today, but if kunit >> could benefit from such framework, it'd be another use case for >> the linker table work. That is, the ability to easilly allow >> dynamically modifying run time ordering of code through ELF sections. >> >>> In addition, by dispatching tests from a single location, we can >>> guarantee that all KUnit tests run after late_init is complete, which >>> was a concern during the initial KUnit patchset review (this has not >>> been a problem in practice, but resolving with certainty is nevertheless >>> desirable). >> >> Indeed, the concern is just a real semantics limitations. With the tests >> *always* running after all subsystem init stuff, we know we'd have a >> real full kernel ready. > > Yep. > >> It does beg the question if this means kunit is happy to not be a tool >> to test pre basic setup stuff (terminology used in init.c, meaning prior >> to running all init levels). I suspect this is the case. > > Not sure. I still haven't seen any cases where this is necessary, so I > am not super worried about it. Regardless, I don't think this patchset > really changes anything in that regard, we are moving from late_init > to after late_init, so it isn't that big of a change for most use > cases. > > Please share if you can think of some things that need to be tested in > early init. I don't have a specific need for this right now. I had not thought about how the current kunit implementation forces all kunit tests to run at a specific initcall level before reading this email thread. I can see the value of being able to have some tests run at different initcall levels to verify what functionality is available and working at different points in the boot sequence. But more important than early initcall levels, I do not want the framework to prevent using or testing code and data that are marked as '__init'. So it is important to retain a way to invoke the tests while __init code and data are available, if there is also a change to generally invoke the tests later. -Frank > >>> Other use cases for this exist, but the above features should provide an >>> idea of the value that this could provide. >>> >>> ## What work remains to be done? >>> >>> These patches were based on patches in our non-upstream branch[2], so we >>> have a pretty good idea that they are useable as presented; >>> nevertheless, some of the changes done in this patchset could >>> *definitely* use some review by subsystem experts (linker scripts, init, >>> etc), and will likely change a lot after getting feedback. >>> >>> The biggest thing that I know will require additional attention is >>> integrating this patchset with the KUnit module support patchset[3]. I >>> have not even attempted to build these patches on top of the module >>> support patches as I would like to get people's initial thoughts first >>> (especially Alan's :-) ). I think that making these patches work with >>> module support should be fairly straight forward, nevertheless. >> >> Modules just have their own sections too. That's all. So it'd be a >> matter of extending the linker script for modules too. But a module's >> init is different than the core kernel's for vmlinux. > > Truth. It seems as though Alan has already fixed this for me, however. >
On Mon, Jan 27, 2020 at 9:40 AM Frank Rowand <frowand.list@gmail.com> wrote: > > On 1/23/20 4:40 PM, Brendan Higgins wrote: > > Sorry for the late reply. I am still catching up from being on vacation. > >> > On Mon, Jan 6, 2020 at 2:40 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > >> It does beg the question if this means kunit is happy to not be a tool > >> to test pre basic setup stuff (terminology used in init.c, meaning prior > >> to running all init levels). I suspect this is the case. > > > > Not sure. I still haven't seen any cases where this is necessary, so I > > am not super worried about it. Regardless, I don't think this patchset > > really changes anything in that regard, we are moving from late_init > > to after late_init, so it isn't that big of a change for most use > > cases. > > > > Please share if you can think of some things that need to be tested in > > early init. > > I don't have a specific need for this right now. I had not thought about > how the current kunit implementation forces all kunit tests to run at a > specific initcall level before reading this email thread. > > I can see the value of being able to have some tests run at different > initcall levels to verify what functionality is available and working > at different points in the boot sequence. Let's cross that bridge when we get there. It should be fairly easy to add that functionality. > But more important than early initcall levels, I do not want the > framework to prevent using or testing code and data that are marked > as '__init'. So it is important to retain a way to invoke the tests > while __init code and data are available, if there is also a change > to generally invoke the tests later. Definitely. For now that still works as long as you don't build KUnit as a module, but I think Alan's new patches which allow KUnit to be run at runtime via debugfs could cause some difficulty there. Again, we could add Kconfigs to control this, but the compiler nevertheless complains because it doesn't know what phase KUnit runs in. Is there any way to tell the compiler that it is okay for non __init code to call __init code? I would prefer not to have a duplicate version of all the KUnit libraries with all the symbols marked __init. Thoughts?
On 1/28/20 1:19 AM, Brendan Higgins wrote: > On Mon, Jan 27, 2020 at 9:40 AM Frank Rowand <frowand.list@gmail.com> wrote: >> >> On 1/23/20 4:40 PM, Brendan Higgins wrote: >>> Sorry for the late reply. I am still catching up from being on vacation. >>>>> On Mon, Jan 6, 2020 at 2:40 PM Luis Chamberlain <mcgrof@kernel.org> wrote: >>>> It does beg the question if this means kunit is happy to not be a tool >>>> to test pre basic setup stuff (terminology used in init.c, meaning prior >>>> to running all init levels). I suspect this is the case. >>> >>> Not sure. I still haven't seen any cases where this is necessary, so I >>> am not super worried about it. Regardless, I don't think this patchset >>> really changes anything in that regard, we are moving from late_init >>> to after late_init, so it isn't that big of a change for most use >>> cases. >>> >>> Please share if you can think of some things that need to be tested in >>> early init. >> >> I don't have a specific need for this right now. I had not thought about >> how the current kunit implementation forces all kunit tests to run at a >> specific initcall level before reading this email thread. >> >> I can see the value of being able to have some tests run at different >> initcall levels to verify what functionality is available and working >> at different points in the boot sequence. > > Let's cross that bridge when we get there. It should be fairly easy to > add that functionality. Yes. I just wanted to add the thought to the back of your mind so that it does not get precluded by future changes to the kunit architecture. > >> But more important than early initcall levels, I do not want the >> framework to prevent using or testing code and data that are marked >> as '__init'. So it is important to retain a way to invoke the tests >> while __init code and data are available, if there is also a change >> to generally invoke the tests later. > > Definitely. For now that still works as long as you don't build KUnit > as a module, but I think Alan's new patches which allow KUnit to be > run at runtime via debugfs could cause some difficulty there. Again, Yes, Alan's patches are part of what triggered me thinking about the issues I raised. > we could add Kconfigs to control this, but the compiler nevertheless > complains because it doesn't know what phase KUnit runs in. > > Is there any way to tell the compiler that it is okay for non __init > code to call __init code? I would prefer not to have a duplicate > version of all the KUnit libraries with all the symbols marked __init. I'm not sure. The build messages have always been useful and valid in my context, so I never thought to consider that possibility. > Thoughts? > . >
> -----Original Message----- > From: Frank Rowand on January 28, 2020 11:37 AM > > On 1/28/20 1:19 AM, Brendan Higgins wrote: > > On Mon, Jan 27, 2020 at 9:40 AM Frank Rowand <frowand.list@gmail.com> wrote: ... > > we could add Kconfigs to control this, but the compiler nevertheless > > complains because it doesn't know what phase KUnit runs in. > > > > Is there any way to tell the compiler that it is okay for non __init > > code to call __init code? I would prefer not to have a duplicate > > version of all the KUnit libraries with all the symbols marked __init. > > I'm not sure. The build messages have always been useful and valid in > my context, so I never thought to consider that possibility. > > > Thoughts? I'm not sure there's a restriction on non __init code calling __init code. In init/main.c arch_call_reset_init() is in __init, and it calls rest_init which is non __init, without any special handling. Is the compiler complaint mentioned above related to calling into __init code, or with some other issue? -- Tim
On Tue, Jan 28, 2020 at 11:35 AM <Tim.Bird@sony.com> wrote: > > > -----Original Message----- > > From: Frank Rowand on January 28, 2020 11:37 AM > > > > On 1/28/20 1:19 AM, Brendan Higgins wrote: > > > On Mon, Jan 27, 2020 at 9:40 AM Frank Rowand <frowand.list@gmail.com> wrote: > ... > > > we could add Kconfigs to control this, but the compiler nevertheless > > > complains because it doesn't know what phase KUnit runs in. > > > > > > Is there any way to tell the compiler that it is okay for non __init > > > code to call __init code? I would prefer not to have a duplicate > > > version of all the KUnit libraries with all the symbols marked __init. > > > > I'm not sure. The build messages have always been useful and valid in > > my context, so I never thought to consider that possibility. > > > > > Thoughts? > > I'm not sure there's a restriction on non __init code calling __init > code. In init/main.c arch_call_reset_init() is in __init, and it calls > rest_init which is non __init, without any special handling. > > Is the compiler complaint mentioned above related to calling > into __init code, or with some other issue? I distinctly remember having the compiler complain at me when I was messing around with the device tree unit tests because of KUnit calling code marked as __init. Maybe it's time to start converting those to KUnit to force the issue? Frank, does that work for you?
On 1/28/20 1:53 PM, Brendan Higgins wrote: > On Tue, Jan 28, 2020 at 11:35 AM <Tim.Bird@sony.com> wrote: >> >>> -----Original Message----- >>> From: Frank Rowand on January 28, 2020 11:37 AM >>> >>> On 1/28/20 1:19 AM, Brendan Higgins wrote: >>>> On Mon, Jan 27, 2020 at 9:40 AM Frank Rowand <frowand.list@gmail.com> wrote: >> ... >>>> we could add Kconfigs to control this, but the compiler nevertheless >>>> complains because it doesn't know what phase KUnit runs in. >>>> >>>> Is there any way to tell the compiler that it is okay for non __init >>>> code to call __init code? I would prefer not to have a duplicate >>>> version of all the KUnit libraries with all the symbols marked __init. >>> >>> I'm not sure. The build messages have always been useful and valid in >>> my context, so I never thought to consider that possibility. >>> >>>> Thoughts? >> >> I'm not sure there's a restriction on non __init code calling __init >> code. In init/main.c arch_call_reset_init() is in __init, and it calls >> rest_init which is non __init, without any special handling. >> >> Is the compiler complaint mentioned above related to calling >> into __init code, or with some other issue? > > I distinctly remember having the compiler complain at me when I was > messing around with the device tree unit tests because of KUnit > calling code marked as __init. Maybe it's time to start converting > those to KUnit to force the issue? Frank, does that work for you? I have agreed to try converting the devicetree unittest to KUnit. Now that KUnit is in 5.5, I think there is a solid foundation for me to proceed. -Frank
On Tue, 28 Jan 2020, Frank Rowand wrote: > On 1/28/20 1:19 AM, Brendan Higgins wrote: > > On Mon, Jan 27, 2020 at 9:40 AM Frank Rowand <frowand.list@gmail.com> wrote: > >> > >> On 1/23/20 4:40 PM, Brendan Higgins wrote: > >>> Sorry for the late reply. I am still catching up from being on vacation. > >>>>> On Mon, Jan 6, 2020 at 2:40 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > >>>> It does beg the question if this means kunit is happy to not be a tool > >>>> to test pre basic setup stuff (terminology used in init.c, meaning prior > >>>> to running all init levels). I suspect this is the case. > >>> > >>> Not sure. I still haven't seen any cases where this is necessary, so I > >>> am not super worried about it. Regardless, I don't think this patchset > >>> really changes anything in that regard, we are moving from late_init > >>> to after late_init, so it isn't that big of a change for most use > >>> cases. > >>> > >>> Please share if you can think of some things that need to be tested in > >>> early init. > >> > >> I don't have a specific need for this right now. I had not thought about > >> how the current kunit implementation forces all kunit tests to run at a > >> specific initcall level before reading this email thread. > >> > >> I can see the value of being able to have some tests run at different > >> initcall levels to verify what functionality is available and working > >> at different points in the boot sequence. > > > > Let's cross that bridge when we get there. It should be fairly easy to > > add that functionality. > > Yes. I just wanted to add the thought to the back of your mind so that > it does not get precluded by future changes to the kunit architecture. > > > > >> But more important than early initcall levels, I do not want the > >> framework to prevent using or testing code and data that are marked > >> as '__init'. So it is important to retain a way to invoke the tests > >> while __init code and data are available, if there is also a change > >> to generally invoke the tests later. > > > > Definitely. For now that still works as long as you don't build KUnit > > as a module, but I think Alan's new patches which allow KUnit to be > > run at runtime via debugfs could cause some difficulty there. Again, > > Yes, Alan's patches are part of what triggered me thinking about the > issues I raised. > > As Brendan says, any such tests probably shouldn't be buildable as modules, but I wonder if we need to add some sort of way to ensure execution from debugfs is not allowed for such cases? Even if a test suite is builtin, it can be executed via debugfs in the patches I sent out, allowing suites to be re-run. Sounds like we need a way to control that behaviour based on the desired test suite execution environment. Say, for example, the "struct kunit_suite" definitions associated with the tests was marked as __initdata; are there any handy macros to identify it as being in the __init section? If so, we could simply avoid adding a "run" file to the debugfs representation for such suites. Failing that, perhaps we need some sort of flags field in "struct kunit_suite" to specify execution environment constraints? Thanks! Alan
On Tue, Jan 28, 2020 at 8:24 PM Frank Rowand <frowand.list@gmail.com> wrote: > > On 1/28/20 1:53 PM, Brendan Higgins wrote: > > On Tue, Jan 28, 2020 at 11:35 AM <Tim.Bird@sony.com> wrote: > >> > >>> -----Original Message----- > >>> From: Frank Rowand on January 28, 2020 11:37 AM > >>> > >>> On 1/28/20 1:19 AM, Brendan Higgins wrote: > >>>> On Mon, Jan 27, 2020 at 9:40 AM Frank Rowand <frowand.list@gmail.com> wrote: > >> ... > >>>> we could add Kconfigs to control this, but the compiler nevertheless > >>>> complains because it doesn't know what phase KUnit runs in. > >>>> > >>>> Is there any way to tell the compiler that it is okay for non __init > >>>> code to call __init code? I would prefer not to have a duplicate > >>>> version of all the KUnit libraries with all the symbols marked __init. > >>> > >>> I'm not sure. The build messages have always been useful and valid in > >>> my context, so I never thought to consider that possibility. > >>> > >>>> Thoughts? > >> > >> I'm not sure there's a restriction on non __init code calling __init > >> code. In init/main.c arch_call_reset_init() is in __init, and it calls > >> rest_init which is non __init, without any special handling. > >> > >> Is the compiler complaint mentioned above related to calling > >> into __init code, or with some other issue? > > > > I distinctly remember having the compiler complain at me when I was > > messing around with the device tree unit tests because of KUnit > > calling code marked as __init. Maybe it's time to start converting > > those to KUnit to force the issue? Frank, does that work for you? > > I have agreed to try converting the devicetree unittest to KUnit. > > Now that KUnit is in 5.5, I think there is a solid foundation for > me to proceed. Awesome! Last time we talked (offline), it sounded like you had a clear idea of what you wanted to do; nevertheless, feel free to reuse anything from my attempt at it, if you find anything useful, or otherwise rope me in if you have any questions, comments, or complaints.
On Wed, Jan 29, 2020 at 5:07 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > On Tue, 28 Jan 2020, Frank Rowand wrote: > > > On 1/28/20 1:19 AM, Brendan Higgins wrote: > > > On Mon, Jan 27, 2020 at 9:40 AM Frank Rowand <frowand.list@gmail.com> wrote: > > >> > > >> On 1/23/20 4:40 PM, Brendan Higgins wrote: > > >>> Sorry for the late reply. I am still catching up from being on vacation. > > >>>>> On Mon, Jan 6, 2020 at 2:40 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > >>>> It does beg the question if this means kunit is happy to not be a tool > > >>>> to test pre basic setup stuff (terminology used in init.c, meaning prior > > >>>> to running all init levels). I suspect this is the case. > > >>> > > >>> Not sure. I still haven't seen any cases where this is necessary, so I > > >>> am not super worried about it. Regardless, I don't think this patchset > > >>> really changes anything in that regard, we are moving from late_init > > >>> to after late_init, so it isn't that big of a change for most use > > >>> cases. > > >>> > > >>> Please share if you can think of some things that need to be tested in > > >>> early init. > > >> > > >> I don't have a specific need for this right now. I had not thought about > > >> how the current kunit implementation forces all kunit tests to run at a > > >> specific initcall level before reading this email thread. > > >> > > >> I can see the value of being able to have some tests run at different > > >> initcall levels to verify what functionality is available and working > > >> at different points in the boot sequence. > > > > > > Let's cross that bridge when we get there. It should be fairly easy to > > > add that functionality. > > > > Yes. I just wanted to add the thought to the back of your mind so that > > it does not get precluded by future changes to the kunit architecture. > > > > > > > >> But more important than early initcall levels, I do not want the > > >> framework to prevent using or testing code and data that are marked > > >> as '__init'. So it is important to retain a way to invoke the tests > > >> while __init code and data are available, if there is also a change > > >> to generally invoke the tests later. > > > > > > Definitely. For now that still works as long as you don't build KUnit > > > as a module, but I think Alan's new patches which allow KUnit to be > > > run at runtime via debugfs could cause some difficulty there. Again, > > > > Yes, Alan's patches are part of what triggered me thinking about the > > issues I raised. > > > > > > As Brendan says, any such tests probably shouldn't be buildable > as modules, but I wonder if we need to add some sort of way > to ensure execution from debugfs is not allowed for such cases? > Even if a test suite is builtin, it can be executed via debugfs > in the patches I sent out, allowing suites to be re-run. Sounds > like we need a way to control that behaviour based on the > desired test suite execution environment. I think that's true. > Say, for example, the "struct kunit_suite" definitions associated > with the tests was marked as __initdata; are there any handy macros to > identify it as being in the __init section? If so, we could simply > avoid adding a "run" file to the debugfs representation for such > suites. Failing that, perhaps we need some sort of flags field > in "struct kunit_suite" to specify execution environment constraints? I think the former would be ideal, but the latter is acceptable as well, assuming neither results in complaints from the compiler (I guess we will find out for sure once we get a hold of the device tree KUnit test). Luis, you mentioned your linker table work might be applicable for dynamic post boot configuring of dispatching. Do you think this work could help solve this problem? For reference, Alan's debugfs code can be found here: https://lore.kernel.org/linux-kselftest/CAFd5g46657gZ36PaP8Pi999hPPgBU2Kz94nrMspS-AzGwdBF+g@mail.gmail.com/T/#m210cadbeee267e5c5a9253d83b7b7ca723d1f871
On Thu, Jan 23, 2020 at 02:40:31PM -0800, Brendan Higgins wrote: > On Mon, Jan 6, 2020 at 2:40 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > On Mon, Dec 16, 2019 at 02:05:49PM -0800, Brendan Higgins wrote: > > dispatcher could be configurable to run at an arbirary time after boot. > > If there are not immediate use cases for that though, then I suppose > > this is not a requirement for the dispatcher. But since there exists > > another modular test framework with its own dispatcher and it seems the > > goal is to merge the work long term, this might preempt the requirement > > to define how and when we can dispatch tests post boot. > > > > And, if we're going to do that, I can suggest that a data structure > > instead of just a function init call be used to describe tests to be > > placed into an ELF section. With my linker table work this would be > > easy, I define section ranges for code describing only executable > > routines, but it defines linker tables for when a component in the > > kernel would define a data structure, part of which can be a callback. > > Such data structure stuffed into an ELF section could allow dynamic > > configuration of the dipsatching, even post boot. > > The linker table work does sound interesting. Do you have a link? Sure https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20170620-linker-tables-v8 I had dropped this long ago mostly due to the fact that my use case (removal of dead code) was more long term, and not immediate so the use case wasn't there yet. I have been meaning to pick this work up again. > I was thinking about dynamic dispatching, actually. I thought it would > be handy to be able to build all tests into a single kernel and then > run different tests on different invocations. For built-in code it would be up to it to manage that. The linker table stuff would just allow a way for you to systematically aggregate data into an ELF section in a generic way. It does have a built in light weight sort mechanism, if you opt out of that and say wanted to do your own order it'd be up to how you program it in on the data structure and dispatching after that. > Also, for post boot dynamic dispatching, you should check out Alan's > debugfs patches: > > https://lore.kernel.org/linux-kselftest/CAFd5g46657gZ36PaP8Pi999hPPgBU2Kz94nrMspS-AzGwdBF+g@mail.gmail.com/T/#m210cadbeee267e5c5a9253d83b7b7ca723d1f871 > > They look pretty handy! Sure. > > I think this is a good stepping stone forward then, and to allow > > dynamic configuration of the dispatcher could mean eventual extensions > > to kunit's init stuff to stuff init calls into a data structure which > > can then allow configuration of the dispatching. One benefit that the > > linker table work *may* be able to help here with is that it allows > > an easy way to create kunit specific ordering, at linker time. > > There is also an example of addressing / generalizing dynamic / run time > > changes of ordering, by using the x86 IOMMU initialization as an > > example case. We don't have an easy way to do this today, but if kunit > > could benefit from such framework, it'd be another use case for > > the linker table work. That is, the ability to easilly allow > > dynamically modifying run time ordering of code through ELF sections. > > > > > In addition, by dispatching tests from a single location, we can > > > guarantee that all KUnit tests run after late_init is complete, which > > > was a concern during the initial KUnit patchset review (this has not > > > been a problem in practice, but resolving with certainty is nevertheless > > > desirable). > > > > Indeed, the concern is just a real semantics limitations. With the tests > > *always* running after all subsystem init stuff, we know we'd have a > > real full kernel ready. > > Yep. > > > It does beg the question if this means kunit is happy to not be a tool > > to test pre basic setup stuff (terminology used in init.c, meaning prior > > to running all init levels). I suspect this is the case. > > Not sure. I still haven't seen any cases where this is necessary, so I > am not super worried about it. Regardless, I don't think this patchset > really changes anything in that regard, we are moving from late_init > to after late_init, so it isn't that big of a change for most use > cases. > > Please share if you can think of some things that need to be tested in > early init. If and when we get to that point we can deal with it then. My instincts tell me that for early init code we should probably be specially crafted tests and have they should have their own hand crafted dispatchers. Luis
On Wed, Jan 29, 2020 at 01:28:19PM -0800, Brendan Higgins wrote: > On Wed, Jan 29, 2020 at 5:07 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > > > On Tue, 28 Jan 2020, Frank Rowand wrote: > > > > > On 1/28/20 1:19 AM, Brendan Higgins wrote: > > > > On Mon, Jan 27, 2020 at 9:40 AM Frank Rowand <frowand.list@gmail.com> wrote: > > > >> > > > >> On 1/23/20 4:40 PM, Brendan Higgins wrote: > > > >>> Sorry for the late reply. I am still catching up from being on vacation. > > > >>>>> On Mon, Jan 6, 2020 at 2:40 PM Luis Chamberlain <mcgrof@kernel.org> wrote: > > > >>>> It does beg the question if this means kunit is happy to not be a tool > > > >>>> to test pre basic setup stuff (terminology used in init.c, meaning prior > > > >>>> to running all init levels). I suspect this is the case. > > > >>> > > > >>> Not sure. I still haven't seen any cases where this is necessary, so I > > > >>> am not super worried about it. Regardless, I don't think this patchset > > > >>> really changes anything in that regard, we are moving from late_init > > > >>> to after late_init, so it isn't that big of a change for most use > > > >>> cases. > > > >>> > > > >>> Please share if you can think of some things that need to be tested in > > > >>> early init. > > > >> > > > >> I don't have a specific need for this right now. I had not thought about > > > >> how the current kunit implementation forces all kunit tests to run at a > > > >> specific initcall level before reading this email thread. > > > >> > > > >> I can see the value of being able to have some tests run at different > > > >> initcall levels to verify what functionality is available and working > > > >> at different points in the boot sequence. > > > > > > > > Let's cross that bridge when we get there. It should be fairly easy to > > > > add that functionality. > > > > > > Yes. I just wanted to add the thought to the back of your mind so that > > > it does not get precluded by future changes to the kunit architecture. > > > > > > > > > > >> But more important than early initcall levels, I do not want the > > > >> framework to prevent using or testing code and data that are marked > > > >> as '__init'. So it is important to retain a way to invoke the tests > > > >> while __init code and data are available, if there is also a change > > > >> to generally invoke the tests later. > > > > > > > > Definitely. For now that still works as long as you don't build KUnit > > > > as a module, but I think Alan's new patches which allow KUnit to be > > > > run at runtime via debugfs could cause some difficulty there. Again, > > > > > > Yes, Alan's patches are part of what triggered me thinking about the > > > issues I raised. > > > > > > > > > > As Brendan says, any such tests probably shouldn't be buildable > > as modules, but I wonder if we need to add some sort of way > > to ensure execution from debugfs is not allowed for such cases? The kernel's linker will ensure this doesn't happen by default, ie __init data called from non __init code gets a complaint at linker time today. *Iff* you are sure the code is proper, you *whitelist* it by adding the __ref tag to it. > > Even if a test suite is builtin, it can be executed via debugfs > > in the patches I sent out, allowing suites to be re-run. Sounds > > like we need a way to control that behaviour based on the > > desired test suite execution environment. > > I think that's true. > > > Say, for example, the "struct kunit_suite" definitions associated > > with the tests was marked as __initdata; are there any handy macros to > > identify it as being in the __init section? If so, we could simply > > avoid adding a "run" file to the debugfs representation for such > > suites. > > Failing that, perhaps we need some sort of flags field > > in "struct kunit_suite" to specify execution environment constraints? > > I think the former would be ideal, but the latter is acceptable as > well, assuming neither results in complaints from the compiler (I > guess we will find out for sure once we get a hold of the device tree > KUnit test). I'd split out tests in two different arrays, one with __init or __initdata one without. Likewise two dispatches, one for init and one for non-init data. > Luis, you mentioned your linker table work might be applicable for > dynamic post boot configuring of dispatching. Do you think this work > could help solve this problem? The Linux kernel table / section ranges code helps aggregate data into ELF sections in a generic way, that is, hacks we have been doing over years into a generic way. So it would be easier to read and implement. For instance see how in this commit the intent/goal of kprobe blacklists is a bit easier to read: https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/commit/?h=20170620-linker-tables-v8&id=b2662efa7c6a3c436961c07fa3082e8640f0e352 In particular DEFINE_LINKTABLE_INIT_DATA() use. I think Youd' want to use DEFINE_LINKTABLE_INIT_DATA() for code which you want to use to dispatch on init and and a DEFINE_LINKTABLE_DATA() for non-init code. If a dynamic dispatcher is used you'd opt out of the using for instance linktable_for_each() and instead use the data structure defined for however you want to disaptch your run time. Luis
## TL;DR This patchset adds a centralized executor to dispatch tests rather than relying on late_initcall to schedule each test suite separately along with a couple of new features that depend on it. ## What am I trying to do? Conceptually, I am trying to provide a mechanism by which test suites can be grouped together so that they can be reasoned about collectively. The last two patches in this series add features which depend on this: RFC 5/6 Prints out a test plan right before KUnit tests are run[1]; this is valuable because it makes it possible for a test harness to detect whether the number of tests run matches the number of tests expected to be run, ensuring that no tests silently failed. RFC 6/6 Add a new kernel command-line option which allows the user to specify that the kernel poweroff, halt, or reboot after completing all KUnit tests; this is very handy for running KUnit tests on UML or a VM so that the UML/VM process exits cleanly immediately after running all tests without needing a special initramfs. In addition, by dispatching tests from a single location, we can guarantee that all KUnit tests run after late_init is complete, which was a concern during the initial KUnit patchset review (this has not been a problem in practice, but resolving with certainty is nevertheless desirable). Other use cases for this exist, but the above features should provide an idea of the value that this could provide. ## What work remains to be done? These patches were based on patches in our non-upstream branch[2], so we have a pretty good idea that they are useable as presented; nevertheless, some of the changes done in this patchset could *definitely* use some review by subsystem experts (linker scripts, init, etc), and will likely change a lot after getting feedback. The biggest thing that I know will require additional attention is integrating this patchset with the KUnit module support patchset[3]. I have not even attempted to build these patches on top of the module support patches as I would like to get people's initial thoughts first (especially Alan's :-) ). I think that making these patches work with module support should be fairly straight forward, nevertheless. Brendan Higgins (5): vmlinux.lds.h: add linker section for KUnit test suites arch: um: add linker section for KUnit test suites kunit: test: create a single centralized executor for all tests init: main: add KUnit to kernel init kunit: test: add test plan to KUnit TAP format David Gow (1): kunit: Add 'kunit_shutdown' option arch/um/include/asm/common.lds.S | 4 + include/asm-generic/vmlinux.lds.h | 8 ++ include/kunit/test.h | 16 ++-- init/main.c | 4 + lib/kunit/Makefile | 3 +- lib/kunit/executor.c | 74 ++++++++++++++++++ lib/kunit/test.c | 11 --- tools/testing/kunit/kunit_kernel.py | 2 +- tools/testing/kunit/kunit_parser.py | 76 +++++++++++++++---- .../test_is_test_passed-all_passed.log | 1 + .../test_data/test_is_test_passed-crash.log | 1 + .../test_data/test_is_test_passed-failure.log | 1 + 12 files changed, 170 insertions(+), 31 deletions(-) create mode 100644 lib/kunit/executor.c [1]: https://github.com/isaacs/testanything.github.io/blob/tap14/tap-version-14-specification.md#the-plan [2]: https://kunit-review.googlesource.com/c/linux/+/1037 [3]: https://patchwork.kernel.org/project/linux-kselftest/list/?series=211727