On Wed, Jan 29, 2020 at 01:28:19PM -0800, Brendan Higgins wrote:
On Wed, Jan 29, 2020 at 5:07 AM Alan Maguire email@example.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 firstname.lastname@example.org 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 email@example.com 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:
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.