Hi Shuah, On Thu, 30 Jun 2022, Shuah Khan wrote: > On 6/30/22 8:12 AM, Marcos Paulo de Souza wrote: > > Hi there, > > > > this is the v2 of the patchset. The v1 can be found at [1]. There is only > > one > > change in patch 1, which changed the target directory to build the test > > modules. > > All other changes happen in patch 2. > > > > Thanks for reviewing! > > > > Changes from v1: > > # test_modules/Makefile > > * Build the test modules targeting /lib/modules, instead of ksrc when > > building > > from the kernel source. > > > > # test_modules/test_klp_syscall.c > > * Added a parameter array to receive the pids that should transition to > > the > > new system call. (suggedted by Joe) > > * Create a new sysfs file /sys/kernel/test_klp_syscall/npids to show how > > many > > pids from the argument need to transition to the new state. (suggested > > by > > Joe) > > * Fix the PPC32 support by adding the syscall wrapper for archs that > > select it > > by default, without erroring out. PPC does not set SYSCALL_WRAPPER, so > > having it set in v1 was a mistake. (suggested by Joe) > > * The aarch64 syscall prefix was added too, since the livepatch support > > will come soon. > > > > # test_binaries/test_klp-call_getpid.c > > * Change %d/%u in printf (suggested byu Joe) > > * Change run -> stop variable name, and inverted the assignments > > (suggested by > > * Joe). > > > > # File test-syscall.sh > > * Fixed test-syscall.sh to call test_klp-call-getpid in test_binaries dir > > * Load test_klp_syscall passed the pids of the test_klp-call_getpid > > instances. > > Check the sysfs file from test_klp_syscall module to check that all > > pids > > transitioned correctly. (suggested by Joe) > > * Simplified the loop that calls test_klp-call_getpid. (suggested by Joe) > > * Removed the "success" comment from the script, as it's implicit that it > > succeed. Otherwise load_lp would error out. (suggested by Joe) > > > > * Changed the commit message of patch 2 to further detail what means > > "tricky" > > when livepatching syscalls. (suggested by Joe) > > > > [1]: 20220603143242.870-1-mpdesouza@xxxxxxxx > > > > Sorry Nack on this. Let's not add modules under selftests. Any usage of > module_init() > doesn't belong under selftests. as mentioned before, that ship has already sailed with tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c. Anyway... You wrote before that you did not have a problem with it. And you would not have a problem with Marcos' approach if modules can be compiled and if not, the tests would fail gracefully. What has changed? If you see a problem in the patch set regarding this, can we fix it? > Leave these under lib and use KSTM_MODULE_LOADERS to load these modules that > live under lib. I may misunderstand but KSTM_MODULE_LOADERS does not seem to provide the flexibility we need (yes, it could be hacked around, but I do not think that the result would be nice). See what we have in tools/testing/selftests/livepatch/functions.sh to make sure that a live patch module is properly loaded and unloaded. My main question is different though. As Marcos mentioned before, we would like to have our tests really flexible and a possibility to prepare and load different live patch modules based on a template is a part of it. What is your proposal regarding this? I can imagine having a template in lib/livepatch/ which would not be compilable and a script in tools/testing/selftests/livepatch/ would copy it many times, amend the copies (meaning parameters would be filled in with sed or the code would be changed), compile them and load them. But this sounds horrible to me, especially when compared to Marcos' approach in this patch set which is quite straightforward. Then there is an opportunity which Joe described. To run the latest livepatch kselftests on an older kernel. Having test modules in lib/ is kind of an obstacle there. Regards Miroslav
