On Fri, Aug 02, 2019 at 06:02:49PM +0100, Cristian Marussi wrote:
Hi
this patchset aims to add the initial arch-specific arm64 support to kselftest starting with signals-related test-cases. A common internal test-case layout is proposed which then it is anyway wired-up to the toplevel kselftest Makefile, so that it should be possible at the end to run it on an arm64 target in the usual way with KSFT.
The tests look like a reasonable base overall and something that we can extend later as needed.
There are various minor things that need attention -- see my comments on the individual patches. Apart for some things that can be factored out, I don't think any of it involves redesign.
A few general comments:
* Please wrap all commit messages to <= 75 chars, and follow the other guidelines about commit messages in Documentation/process/submitting-patches.rst).
* Remember to run scripts/checkpatch.pl on your patches. Currently various issues are reported: they should mostly be trivial to fix. checkpatch does report some false positives, but most of the warnings I see look relevant.
* If you like, you can add an Author: line alongside the copyright notice in new files that you create. (You'll see this elsewhere in the kernel if you grep.)
One general stylistic issue (IMHO):
* Try to avoid inventing names for things that have no established name (for example "magic0" to mean "magic number 0").
The risk is that the reader wastes time grepping for the definition, when really the text should be read at face value. It's best to use all caps just for #define names, abbreviations, and other things that are customarily capitalised (like "CPU" etc.). Other words containing underscores may resemble variable / function names, and may cause confusion of there is no actual variable or function with that name.
I don't think it's worth heavily reworking the patches for this, but it's something to bear in mind.
[...]
Cheers ---Dave