On 03/03/2015 07:49 AM, Dave Jones wrote: > On Mon, Mar 02, 2015 at 09:48:08PM -0700, Shuah Khan wrote: > > kselftest_install.sh tool adds support for installing selftests > > at user specified location/kselftest. By default this tool > > will install selftests in the selftests/kselftest directory. > > For example, kselftest_install /tmp will install tests under > > /tmp/kselftest > > How is this an improvement over having each test install method isolated > to its own Makefile ? Dave/Michael, Makefile approach requires changes to all the existing test Makefiles. After looking at the churn to individual Makefiles, I have the following concerns. I am concerned about maintenance and potential for mistakes in install logic in individual Makefiles when new tests get added. I keep seeing run_tests target breaking when new tests get added and also when existing tests get modified. That said, I looked at Michael's patches and Michael's work does address several of my concerns. Hence, the following plan: I will take the following patches from Michael after requested changes are made: - [PATCH 1/9] selftests: Introduce minimal shared logic for running tests This improves current run_tests logic. Will need changes to account for duplicate cpu and memory hot-plug scripts. Both are named on-off-test.sh - won't make a difference for this patch, but will for install logic Note: I am seeing failures when I run sudo make kselftest after applying this patch /bin/sh: 1: ./run_netsocktests: Permission denied selftests: run_netsocktests [FAIL] /bin/sh: 1: ./run_afpackettests: Permission denied selftests: run_afpackettests [FAIL] /bin/sh: 1: ./run_numerictests: Permission denied selftests: run_numerictests [FAIL] /bin/sh: 1: ./run_stringtests: Permission denied selftests: run_stringtests [FAIL] /bin/sh: 1: ./run_vmtests: Permission denied Please make sure make kselftest doesn't regress when run as root as well as user. In addition, the following don't regress: make -C tools/testing/selftests TARGETS=test1 run_tests make -C tools/testing/selftests TARGETS="test1 test2" run_tests make -C tools/testing/selftests run_hotplug Please see Documentation/kselftest.txt - don't want to regress the current use-cases. - [PATCH 2/9] selftests: Add install target Looks like lib.mk logic is addressing some of my concerns about the individual Makefile install logic. I would like to see 1. the all script name changed to run_kselftest.sh - [PATCH 3/9] selftests: Add install support for the powerpc tests This is good as is. - [PATCH 5/9] kbuild: Don't pass -rR to selftest makefiles Drop kselftest_install from this patch. There is no need. More on this below. - PATCH 6/9] selftests: Set CC using CROSS_COMPILE once in lib.mk - [PATCH 7/9] selftests/timers: Use implicit rules Please check John Stultz's timers test work to make sure there is no conflict. Please see: [PATCH 01/19] selftests/timers: Cleanup Makefile to make it easier to add future tests https://lkml.org/lkml/2015/2/5/56 - [PATCH 8/9] selftests/mqueue: Use implicit rules This is good as is. - [PATCH 9/9] selftests/mount: Use implicit rules This is good as is. Drop these patches: - [PATCH 4/9] kbuild: add a new kselftest_install make target to install selftests I am not seeing any value in adding install target to the main Makefile. Invoking test run makes sense as it allows user to run them from the git without install step which makes sense. Being able to invoke install from main Makefile really doesn't add any value. We can isolate install logic under selftests and also avoid adding one more target to the main Makefile. I still want a wrapper script for install, so: - I will change kselftest_install.sh to leverage the above work. It can just invoke make install at the selftests directory after figuring base directory logic etc. This will be based on the above patches. - Keep gen_kselftest_tar.sh as is - no changes need. - I can drop run_kselftest tool completely, since emit logic in Michael's work takes care of it. Comments and questions? I am cc'ing Michal Marek to keep him in the loop. Michael! Please let me know if you want to see responses to your individual patches, in addition to this email. thanks, -- Shuah -- Shuah Khan Sr. Linux Kernel Developer Open Source Innovation Group Samsung Research America (Silicon Valley) shuahkh@xxxxxxxxxxxxxxx | (970) 217-8978 -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
