From: Simon Guo <wei.guo.simon@gmail.com>
To: Cyril Bur <cyrilbur@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org,
Michael Ellerman <mpe@ellerman.id.au>,
Suraj Jitindar Singh <sjitindarsingh@gmail.com>,
Anshuman Khandual <khandual@linux.vnet.ibm.com>,
Rashmica Gupta <rashmicy@gmail.com>
Subject: Re: [PATCH v14 00/15] selftests/powerpc: Add ptrace tests for ppc registers
Date: Tue, 13 Sep 2016 01:01:05 +0800 [thread overview]
Message-ID: <20160912170105.GA17588@simonLocalRHEL7.x64> (raw)
In-Reply-To: <1473745750.15800.1.camel@gmail.com>
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 <torvalds@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
next prev parent reply other threads:[~2016-09-13 11:58 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-12 7:33 [PATCH v14 00/15] selftests/powerpc: Add ptrace tests for ppc registers wei.guo.simon
2016-09-12 7:33 ` [PATCH v14 01/15] selftests/powerpc: Add more SPR numbers, TM & VMX instructions to 'reg.h' wei.guo.simon
2016-09-14 4:48 ` Cyril Bur
2016-09-12 7:33 ` [PATCH v14 02/15] selftests/powerpc: Use the new SPRN_DSCR_PRIV definiton wei.guo.simon
2016-09-12 7:33 ` [PATCH v14 03/15] selftests/powerpc: Add ptrace tests for EBB wei.guo.simon
2016-09-12 7:33 ` [PATCH v14 04/15] selftests/powerpc: Add ptrace tests for GPR/FPR registers wei.guo.simon
2016-09-14 4:50 ` Cyril Bur
2016-09-12 7:33 ` [PATCH v14 05/15] selftests/powerpc: Add ptrace tests for GPR/FPR registers in TM wei.guo.simon
2016-09-14 4:51 ` Cyril Bur
2016-09-12 7:33 ` [PATCH v14 06/15] selftests/powerpc: Add ptrace tests for GPR/FPR registers in suspended TM wei.guo.simon
2016-09-12 7:33 ` [PATCH v14 07/15] selftests/powerpc: Add ptrace tests for TAR, PPR, DSCR registers wei.guo.simon
2016-09-14 4:53 ` Cyril Bur
2016-09-12 7:33 ` [PATCH v14 08/15] selftests/powerpc: Add ptrace tests for TAR, PPR, DSCR in TM wei.guo.simon
2016-09-12 7:33 ` [PATCH v14 09/15] selftests/powerpc: Add ptrace tests for TAR, PPR, DSCR in suspended TM wei.guo.simon
2016-09-12 7:33 ` [PATCH v14 10/15] selftests/powerpc: Add ptrace tests for VSX, VMX registers wei.guo.simon
2016-09-12 7:33 ` [PATCH v14 11/15] selftests/powerpc: Add ptrace tests for VSX, VMX registers in TM wei.guo.simon
2016-09-12 7:33 ` [PATCH v14 12/15] selftests/powerpc: Add ptrace tests for VSX, VMX registers in suspended TM wei.guo.simon
2016-09-12 7:33 ` [PATCH v14 13/15] selftests/powerpc: Add ptrace tests for TM SPR registers wei.guo.simon
2016-09-14 5:04 ` Cyril Bur
2016-09-19 2:33 ` Simon Guo
2016-09-30 2:17 ` Simon Guo
2016-09-12 7:33 ` [PATCH v14 14/15] selftests/powerpc: Add .gitignore file for ptrace executables wei.guo.simon
2016-09-12 7:33 ` [PATCH v14 15/15] selftests/powerpc: Fix a build issue wei.guo.simon
2016-09-14 4:55 ` Cyril Bur
2016-09-13 5:49 ` [PATCH v14 00/15] selftests/powerpc: Add ptrace tests for ppc registers Cyril Bur
2016-09-12 17:01 ` Simon Guo [this message]
2016-09-14 4:09 ` Cyril Bur
2016-09-14 7:06 ` Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160912170105.GA17588@simonLocalRHEL7.x64 \
--to=wei.guo.simon@gmail.com \
--cc=cyrilbur@gmail.com \
--cc=khandual@linux.vnet.ibm.com \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=rashmicy@gmail.com \
--cc=sjitindarsingh@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).