Re: [RFC PATCH 10/11] powerpc/tm: Set failure summary

[Michael Neuling <mikey@neuling.org>]

On Thu, 2018-09-27 at 17:52 -0300, Breno Leitao wrote:
> Hi Mikey,
> 
> On 09/18/2018 02:50 AM, Michael Neuling wrote:
> > On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
> > > Since the transaction will be doomed with treckpt., the TEXASR[FS]
> > > should be set, to reflect that the transaction is a failure. This patch
> > > ensures it before recheckpointing, and remove changes from other places
> > > that were calling recheckpoint.
> > 
> > TEXASR[FS] should be set by the reclaim. 
> 
> Do you mean that the CPU should set TEXASR[FS] when treclaim is called, or,
> that the tm_reclaim?
> 
> Looking at the ISA, I didn't see TEXASR[FS] being set automatically when a
> reclaim happens, 
treclaim in ISA talks about "TMRecordFailure(cause)" and that macro sets
TEXASR[FS]=1. 

So yes treclaim always sets TEXASR[FS]=1.

> although, I see it needs TEXASR[FS] to be set when executing a 
> trecheckpoint, otherwise it will cause a TM Bad Thing.

Yep

> That is why I am forcing TEXASR[FS]=1 to doom the transaction so we can
> recheckpoint it, but it seems I understood this wrongly.

You shouldn't need to.  We do a bug_on() just before the trecheckpoint to make
sure it's set, but you shouldn't need to set it (other than the signals case).

> > I don't know why you'd need to set this
> > explicitly in process.c. The only case is when the user supplies a bad
> > signal
> > context, but we should check that in the signals code, not process.c
> > 
> > Hence I think this patch is wrong.
> > 
> > Also, according to the architecture, TEXASR[FS] HAS TO BE SET on
> > trecheckpoint
> > otherwise you'll get a TM Bad Thing. You should say that rather than
> > suggesting
> > it's because the transaction is doomed. It's ilqlegal to not do it. That's
> > why we
> > have this check in arch/powerpc/kernel/tm.S.
> 
> When you say "HAS TO BE SET", do you mean that the hardware will set it and
> we shouldn't care about this flag? Thus, if I am hitting EMIT_BUG_ENTRY, it
> means my TEXASR was messed somehow?

I'm just saying what you said before about the tm bad thing.  We have to make
sure it's set before we do the trecheckpoint otherwise we end up in the TM bad
thing. The treclaim should handle this for us (but again the signals case need
to be checked).

Mikey