On 10/19/2017 02:04 PM, Tulio Magno Quites Machado Filho wrote:

Florian Weimer [off-list ref] writes:

quoted

On 10/12/2017 12:17 PM, Michael Ellerman wrote:

quoted

+	pr_info("Enabling TM (Transactional Memory) with Suspend Disabled\n");
+	cur_cpu_spec->cpu_features |= CPU_FTR_TM;
+	cur_cpu_spec->cpu_user_features2 |= PPC_FEATURE2_HTM_NO_SUSPEND;
+	tm_suspend_disabled = true;

This doesn't look right because if suspend is not available, you need to
clear the original PPC_FEATURE2_HTM flag because the semantics are not
right, so that applications can use fallback code.  Otherwise,
applications may incorrectly select the HTM code and break if running on
a system which supports HTM, but without the suspend state.

Just clarifying: it won't break an application, but abort the transaction,
which can cause a performance hit.

And in this case, it is better not HTM at all.

quoted

The new flag should say that HTM is supported, but without the suspend
state, and it should be always set if PPC_FEATURE2_HTM is set.

If we change the semantics of this bit, old applications that don't suspend
transactions won't run on these processors, even if they're safe to run.

If we adopt the current semantics, only applications that enter in suspend
state will have to be modified in order to not get a performance regression.

In this light, the trade-off implied by the posted patch seems to be the 
right one.  But the other discussion of an ABI change still makes me a 
bit nervous.

Thanks,
Florian