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 <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
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
