[RFC,08/11] powerpc/tm: Do not reclaim on ptrace

Message ID 1536781219-13938-9-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.
Make sure that we are not suspended on ptrace and that the registers were
already reclaimed.

Since the data was already reclaimed, there is nothing to be done here
except to restore the SPRs.

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

Comments

Michael Neuling Sept. 18, 2018, 5:36 a.m. | #1
On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
> Make sure that we are not suspended on ptrace and that the registers were
> already reclaimed.
> 
> Since the data was already reclaimed, there is nothing to be done here
> except to restore the SPRs.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  arch/powerpc/kernel/ptrace.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 9667666eb18e..cf6ee9154b11 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -136,12 +136,10 @@ static void flush_tmregs_to_thread(struct task_struct
> *tsk)
>  	if ((!cpu_has_feature(CPU_FTR_TM)) || (tsk != current))
>  		return;
>  
> -	if (MSR_TM_SUSPENDED(mfmsr())) {
> -		tm_reclaim_current(TM_CAUSE_SIGNAL);
> -	} else {
> -		tm_enable();
> -		tm_save_sprs(&(tsk->thread));
> -	}
> +	WARN_ON(MSR_TM_SUSPENDED(mfmsr()));
> +
> +	tm_enable();
> +	tm_save_sprs(&(tsk->thread));

Do we need to check if TM was enabled in the task before saving the TM SPRs?

What happens if TM was lazily off and hence we had someone else's TM SPRs in the
CPU currently?  Wouldn't this flush the wrong values to the task_struct?

I think we need to check the processes MSR before doing this.

Mikey

>  }
>  #else
>  static inline void flush_tmregs_to_thread(struct task_struct *tsk) { }

Patch

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 9667666eb18e..cf6ee9154b11 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -136,12 +136,10 @@  static void flush_tmregs_to_thread(struct task_struct *tsk)
 	if ((!cpu_has_feature(CPU_FTR_TM)) || (tsk != current))
 		return;
 
-	if (MSR_TM_SUSPENDED(mfmsr())) {
-		tm_reclaim_current(TM_CAUSE_SIGNAL);
-	} else {
-		tm_enable();
-		tm_save_sprs(&(tsk->thread));
-	}
+	WARN_ON(MSR_TM_SUSPENDED(mfmsr()));
+
+	tm_enable();
+	tm_save_sprs(&(tsk->thread));
 }
 #else
 static inline void flush_tmregs_to_thread(struct task_struct *tsk) { }