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

On Thu, 2018-09-27 at 17:52 -0300, Breno Leitao wrote:

Hi Mikey,

On 09/18/2018 02:50 AM, Michael Neuling wrote:

quoted

On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:

quoted

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

quoted

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