On Fri, May 06, 2022 at 03:01:34PM +0800, David Gow wrote: > On Thu, May 5, 2022 at 1:57 PM Luis Chamberlain <mcgrof@xxxxxxxxxx> wrote: > > > > On Wed, May 04, 2022 at 02:12:30PM -0700, Luis Chamberlain wrote: > > > On Wed, May 04, 2022 at 02:19:59PM -0500, Daniel Latypov wrote: > > > > On Wed, May 4, 2022 at 1:46 PM Luis Chamberlain <mcgrof@xxxxxxxxxx> wrote: > > > > > OK so, we can just skip tainting considerations for selftests which > > > > > don't use modules for now. There may be selftests which do wonky > > > > > things in userspace but indeed I agree the userspace taint would > > > > > be better for those but I don't think it may be worth bother > > > > > worrying about those at this point in time. > > > > > > > > > > But my point in that sharing a taint between kunit / selftests modules > > > > > does make sense and is easily possible. The unfortunate aspect is just > > > > > > > > Yes, I 100% agree that we should share a taint for kernelspace testing > > > > from both kunit/kselftest. > > > > Someone running the system won't care what framework was used. > > > > > > OK do you mind doing the nasty work of manually adding the new > > > MODULE_TAINT() to the selftests as part of your effort? > > > > > > *Alternatively*, if we *moved* all sefltests modules to a new > > > lib/debug/selftests/ directory or something like that then t would > > > seem modpost *could* add the taint flag automagically for us without > > > having to edit or require it on new drivers. We have similar type of > > > taint for staging, see add_staging_flag(). > > > > > > I would *highly* prefer this approach, event though it is more work, > > > because I think this is a step we should take anyway. > > > > > > However, I just checked modules on lib/ and well, some of them are > > > already in their own directory, like lib/math/test_div64.c. So not > > > sure, maybe just move a few modules which are just in lib/*.c for now > > > and then just sprinkle the MODULE_TAINT() to the others? > > > > I *think* we could just pull this off with a much easier approach, > > simply looking for the substrings in the module name in modpost.c: > > > > * "_test." || "-test." > > * ^"test_" || ^"test-" > > > > An issue with this of course is a vendor $FOO with an out of tree > > test driver may end up with the taint. Perhaps we don't care. > > > > That means moving selftests to its own directory is not needed at this > > point in time. > > I can't say I'm thrilled with the idea of just doing name comparisons, > particularly since not all of them match this pattern, for example: > bpf_testmod.ko. (Though, frankly, more of them do than I'd've > guessed.) > > Maybe adding a taint call to the selftest helper module framework in > kselftest_module.h, though again, there are several tests which don't > use it. Right, I can't think of a generic way to peg this. I think long term we do stand to gain to move all selftests under a lib/debug/selftests/ or something like that, but for now what I suggested is the only thing I can come up with. > I _suspect_ we'd be able to hit most of them by tainting in frameworks > like the above, and patch the remaining modules manually. Works with me. > There's also > definitely a grey area with things like netdevsim, which are used a > lot as helper modules by selftests, but may have other uses as well. They can peg the module if they want the taint. > (The advantage of the KUnit tainting is that, due to KUnit's > centralised executor, we can be sure all KUnit tests will correctly > trigger the taint. But maybe it doesn't matter as much if one or two > selftests miss out.) That is what I was thinking. I'm convinced we *should* move selftests to a one directory. The amount of stuff in lib/ is getting out of hand. Luis