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:

quoted

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.

quoted

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

quoted

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

quoted

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 [off-list ref]
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