On Tue, 2016-09-13 at 01:01 +0800, Simon Guo wrote: > Hi Cyril, > On Tue, Sep 13, 2016 at 03:49:10PM +1000, Cyril Bur wrote: > > > > Thanks for putting the effort in to get these merged! I have a few > > remarks that apply to more than one patch which I'll say here. > > > > I'm not sure #defining the TM instructions as .long for the > > selftests > > is useful. Compilers these days know about the instructions > > 'tbegin.' > > 'tsuspend.' and the like, I would question anyone using a compiler > > old > > enough not to know about these... > I agree. But let me check with original author Anshuman firstly. >
Great! I'll send through my other emails that I actually wrote before this one, please ignore if I've repeated myself. > > > > > > There are a few assembly fpu register load functions that could be > > consolidated into those in math/ and even some in tm/ > Will rework that. > Thanks > > > > > > Doing while(ptr); to wait for another thread should be > > > > while(ptr) > > asm volatile("" : : : "memory"); > > > > Documentation/volatile-considered-harmful.txt for reasons why. > > Even knowing this I did it your way without thinking in a selftest > > I > > wrote doing similar things and it turns out that it didn't work > > [the > > way we both expect it would]. > You are right. > Thanks > > > > > > Having said all that, I'm aware that these are selftests and this > > series could be nicer but I won't lose any sleep if they were > > merged > > almost as is. Thanks for your work! > > > > Finally, they didn't compile for me, I did a git rebase --exec with > > my > > build scripts and: > > > > selftests/powerpc: Add ptrace tests for EBB > > [snip] > > *** No rule to make target 'ptrace.S', needed by 'ptrace-ebb'. > > (that appears fixed by subsequent patch) > > > > selftests/powerpc: Add ptrace tests for GPR/FPR registers > > Seems to have failed horribly and those problems continue... > > > > I applied these to powerpc-next at: > > commit c6935931c1894ff857616ff8549b61236a19148f > > Author: Linus Torvalds <torva...@linux-foundation.org> > > Date: Sun Sep 4 14:31:46 2016 -0700 > > > > Linux 4.8-rc5 > > > > Should I have based on something else? > I didn't reproduce the latter error and I also applied on c69359. > My build script is only one line: > make -C tools/testing/selftests TARGETS=powerpc 1>/dev/null > I do the same except without TARGETS=powerpc (I test with all the selftests). The difference I suspect is that I'm cross compiling and the #include <linux/elf.h> in ptrace.h is the problem. On my x86 machine this includes an x86 version which has different defines. > Did I miss anything with your build script? > Anyway I need to fix that. Its messy but I think the accepted solution for kselftests is to do: #include "../../../../../usr/include/linux/elf.h" which I believe will get the headers generated for the target by `make headers_install` and therefore should match that for which the kselftests are being compiled. > > Thanks for the sharing. Most are good comments and I will rework > that. > > BR, > - Simon