Re: [PATCH v14 00/15] selftests/powerpc: Add ptrace tests for ppc registers

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.

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.

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.

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

Did I miss anything with your build script?
Anyway I need to fix that.

Thanks for the sharing. Most are good comments and I will rework that.

BR,
- Simon