[RFC,05/11] powerpc/tm: Function that updates the failure code

Message ID 1536781219-13938-6-git-send-email-leitao@debian.org
State New
Headers show
Series
  • New TM Model
Related show

Checks

Context Check Description
snowpatch_ozlabs/checkpatch warning Test checkpatch on branch next
snowpatch_ozlabs/apply_patch success next/apply_patch Successfully applied

Commit Message

Breno Leitao Sept. 12, 2018, 7:40 p.m.
Now the transaction reclaims happens very earlier in the trap handler, and
it is impossible to know precisely, at that early time, what should be set
as the failure cause for some specific cases, as, if the task will be
rescheduled, thus, the transaction abort case should be updated from
TM_CAUSE_MISC to TM_CAUSE_RESCHED, for example.

This patch creates a function that will update TEXASR special purpose
register in the task thread and set the failure code which will be
moved to the live register afterward.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 arch/powerpc/kernel/process.c | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Michael Neuling Sept. 17, 2018, 5:29 a.m. | #1
This series is not bisectable because this patch fails with:

arch/powerpc/kernel/process.c:993:13: error: ‘tm_fix_failure_cause’ defined but not used [-Werror=unused-function]
 static void tm_fix_failure_cause(struct task_struct *task, uint8_t cause)
             ^
cc1: all warnings being treated as errors
scripts/Makefile.build:305: recipe for target 'arch/powerpc/kernel/process.o' failed

Mikey

On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
> Now the transaction reclaims happens very earlier in the trap handler, and
> it is impossible to know precisely, at that early time, what should be set
> as the failure cause for some specific cases, as, if the task will be
> rescheduled, thus, the transaction abort case should be updated from
> TM_CAUSE_MISC to TM_CAUSE_RESCHED, for example.
> 
> This patch creates a function that will update TEXASR special purpose
> register in the task thread and set the failure code which will be
> moved to the live register afterward.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  arch/powerpc/kernel/process.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 54fddf03b97a..fe063c0142e3 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -85,6 +85,7 @@ extern unsigned long _get_SP(void);
>   * other paths that we should never reach with suspend disabled.
>   */
>  bool tm_suspend_disabled __ro_after_init = false;
> +static void tm_fix_failure_cause(struct task_struct *task, uint8_t cause);
>  
>  static void check_if_tm_restore_required(struct task_struct *tsk)
>  {
> @@ -988,6 +989,14 @@ void tm_recheckpoint(struct thread_struct *thread)
>  	local_irq_restore(flags);
>  }
>  
> +/* Change thread->tm.texasr failure code */
> +static void tm_fix_failure_cause(struct task_struct *task, uint8_t cause)
> +{
> +	/* Clear the cause first */
> +	task->thread.tm_texasr &= ~TEXASR_FC;
> +	task->thread.tm_texasr |= (unsigned long) cause << 56;
> +}
> +
>  static inline void tm_recheckpoint_new_task(struct task_struct *new)
>  {
>  	if (!cpu_has_feature(CPU_FTR_TM))
Michael Neuling Sept. 18, 2018, 1:29 a.m. | #2
On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
> Now the transaction reclaims happens very earlier in the trap handler, and
> it is impossible to know precisely, at that early time, what should be set
> as the failure cause for some specific cases, as, if the task will be
> rescheduled, thus, the transaction abort case should be updated from
> TM_CAUSE_MISC to TM_CAUSE_RESCHED, for example.

Please add comments to where this is used (in EXCEPTION_COMMON macro I think)
that say this might happen.

> 
> This patch creates a function that will update TEXASR special purpose
> register in the task thread and set the failure code which will be
> moved to the live register afterward.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  arch/powerpc/kernel/process.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 54fddf03b97a..fe063c0142e3 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -85,6 +85,7 @@ extern unsigned long _get_SP(void);
>   * other paths that we should never reach with suspend disabled.
>   */
>  bool tm_suspend_disabled __ro_after_init = false;
> +static void tm_fix_failure_cause(struct task_struct *task, uint8_t cause);
>  
>  static void check_if_tm_restore_required(struct task_struct *tsk)
>  {
> @@ -988,6 +989,14 @@ void tm_recheckpoint(struct thread_struct *thread)
>  	local_irq_restore(flags);
>  }
>  
> +/* Change thread->tm.texasr failure code */
> +static void tm_fix_failure_cause(struct task_struct *task, uint8_t cause)
> +{
> +	/* Clear the cause first */
> +	task->thread.tm_texasr &= ~TEXASR_FC;
> +	task->thread.tm_texasr |= (unsigned long) cause << 56;
> +}
> +
>  static inline void tm_recheckpoint_new_task(struct task_struct *new)
>  {
>  	if (!cpu_has_feature(CPU_FTR_TM))
Michael Neuling Sept. 18, 2018, 3:27 a.m. | #3
On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
> Now the transaction reclaims happens very earlier in the trap handler, and
> it is impossible to know precisely, at that early time, what should be set
> as the failure cause for some specific cases, as, if the task will be
> rescheduled, thus, the transaction abort case should be updated from
> TM_CAUSE_MISC to TM_CAUSE_RESCHED, for example.
> 
> This patch creates a function that will update TEXASR special purpose
> register in the task thread and set the failure code which will be
> moved to the live register afterward.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  arch/powerpc/kernel/process.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 54fddf03b97a..fe063c0142e3 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -85,6 +85,7 @@ extern unsigned long _get_SP(void);
>   * other paths that we should never reach with suspend disabled.
>   */
>  bool tm_suspend_disabled __ro_after_init = false;
> +static void tm_fix_failure_cause(struct task_struct *task, uint8_t cause);
>  
>  static void check_if_tm_restore_required(struct task_struct *tsk)
>  {
> @@ -988,6 +989,14 @@ void tm_recheckpoint(struct thread_struct *thread)
>  	local_irq_restore(flags);
>  }
>  
> +/* Change thread->tm.texasr failure code */
> +static void tm_fix_failure_cause(struct task_struct *task, uint8_t cause)

I would just call this tm_change_failure_cause() and drop the comment above.

> +{
> +	/* Clear the cause first */
> +	task->thread.tm_texasr &= ~TEXASR_FC;
> +	task->thread.tm_texasr |= (unsigned long) cause << 56;

56 == TEXASR_FC_LG;


> +}
> +
>  static inline void tm_recheckpoint_new_task(struct task_struct *new)
>  {
>  	if (!cpu_has_feature(CPU_FTR_TM))

Patch

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 54fddf03b97a..fe063c0142e3 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -85,6 +85,7 @@  extern unsigned long _get_SP(void);
  * other paths that we should never reach with suspend disabled.
  */
 bool tm_suspend_disabled __ro_after_init = false;
+static void tm_fix_failure_cause(struct task_struct *task, uint8_t cause);
 
 static void check_if_tm_restore_required(struct task_struct *tsk)
 {
@@ -988,6 +989,14 @@  void tm_recheckpoint(struct thread_struct *thread)
 	local_irq_restore(flags);
 }
 
+/* Change thread->tm.texasr failure code */
+static void tm_fix_failure_cause(struct task_struct *task, uint8_t cause)
+{
+	/* Clear the cause first */
+	task->thread.tm_texasr &= ~TEXASR_FC;
+	task->thread.tm_texasr |= (unsigned long) cause << 56;
+}
+
 static inline void tm_recheckpoint_new_task(struct task_struct *new)
 {
 	if (!cpu_has_feature(CPU_FTR_TM))