linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Simon Guo <wei.guo.simon@gmail.com>
To: Cyril Bur <cyrilbur@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org, benh@kernel.crashing.org,
	mpe@ellerman.id.au, mikey@neuling.org,
	Laurent Dufour <ldufour@linux.vnet.ibm.com>
Subject: Re: [PATCH] powerpc: signals: Discard transaction state from signal frames
Date: Sat, 20 Aug 2016 18:03:08 +0800	[thread overview]
Message-ID: <20160820100307.GA6731@simonLocalRHEL7.x64> (raw)
In-Reply-To: <20160822073206.1342-1-cyrilbur@gmail.com>

Hi Cyril,
On Mon, Aug 22, 2016 at 05:32:06PM +1000, Cyril Bur wrote:
> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
> index b6aa378..31e4e15 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -1226,7 +1226,19 @@ long sys_rt_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
>  		(regs->gpr[1] + __SIGNAL_FRAMESIZE + 16);
>  	if (!access_ok(VERIFY_READ, rt_sf, sizeof(*rt_sf)))
>  		goto bad;
> +
>  #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +	/*
> +	 * If there is a transactional/suspended state then throw it away.
> +	 * The purpose of a sigreturn is to destroy all traces of the
> +	 * signal frame, this includes any transactional state created
> +	 * within in.
> +	 * The cause is not important as there will never be a
> +	 * recheckpoint so it's not user visible.
> +	 */
> +	if (MSR_TM_ACTIVE(mfmsr()))
> +		tm_reclaim_current(0);
> +
Maybe a little picky here:
Per my understanding, the TRANSACTIONAL state will be failed in system
call common entry. The only expected state to prevent here is SUSPEND 
state.
Should we use MSR_TM_SUSPENDED(mfmsr()) here and BUG_ON
MSR_TM_TRANSACTIONAL(mfmsr())?  -- If it is transactional state, something
 is wrong with kernel. 

Others looks good to me.

Thanks,
- Simon

  reply	other threads:[~2016-08-22 14:44 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-22  7:32 [PATCH] powerpc: signals: Discard transaction state from signal frames Cyril Bur
2016-08-20 10:03 ` Simon Guo [this message]
2016-08-22 23:28   ` Cyril Bur
2016-08-22  7:35 ` Cyril Bur
2016-08-22  9:47 ` Laurent Dufour
  -- strict thread matches above, loose matches on Subject: below --
2016-08-22  5:15 Cyril Bur
2016-08-22  7:07 ` Michael Neuling
2016-08-22  7:22   ` Cyril Bur
2016-08-22  8:15 ` kbuild test robot
2016-08-22 11:21 ` kbuild test robot
2016-08-23  0:41   ` Cyril Bur

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=20160820100307.GA6731@simonLocalRHEL7.x64 \
    --to=wei.guo.simon@gmail.com \
    --cc=benh@kernel.crashing.org \
    --cc=cyrilbur@gmail.com \
    --cc=ldufour@linux.vnet.ibm.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mikey@neuling.org \
    --cc=mpe@ellerman.id.au \
    /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).