Messages in this thread | | | Date | Wed, 2 Mar 2016 11:15:00 +1100 | From | Cyril Bur <> | Subject | Re: [PATCH V10 02/28] powerpc, process: Add the function flush_tmregs_to_thread |
| |
On Tue, 16 Feb 2016 14:29:32 +0530 Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote:
> This patch creates a function flush_tmregs_to_thread which > will then be used by subsequent patches in this series. The > function checks for self tracing ptrace interface attempts > while in the TM context and logs appropriate warning message. >
Hi Anshuman,
You'll have to bare with me, my ptrace knowledge is non existent so you might have to walk me though some aspects.
I have been playing with FPU/VMX and VSX saving so I thought I'd take a look.
> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com> > --- > arch/powerpc/include/asm/switch_to.h | 8 ++++++++ > arch/powerpc/kernel/process.c | 20 ++++++++++++++++++++ > 2 files changed, 28 insertions(+) > > diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h > index 5b268b6..7b297bf 100644 > --- a/arch/powerpc/include/asm/switch_to.h > +++ b/arch/powerpc/include/asm/switch_to.h > @@ -70,6 +70,14 @@ static inline void disable_kernel_spe(void) > } > #endif > > +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM > +extern void flush_tmregs_to_thread(struct task_struct *); > +#else > +static inline void flush_tmregs_to_thread(struct task_struct *t) > +{ > +} > +#endif > + > static inline void clear_task_ebb(struct task_struct *t) > { > #ifdef CONFIG_PPC_BOOK3S_64 > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c > index dccc87e..2c4fa7f 100644 > --- a/arch/powerpc/kernel/process.c > +++ b/arch/powerpc/kernel/process.c > @@ -918,6 +918,26 @@ static inline void restore_sprs(struct thread_struct *old_thread, > #endif > } >
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM > +void flush_tmregs_to_thread(struct task_struct *tsk) > +{ > + /* > + * Process self tracing is not yet supported through > + * ptrace interface. Ptrace generic code should have > + * prevented this from happening in the first place. > + * Warn once here with the message, if some how it > + * is attempted. > + */ > + WARN_ONCE(tsk == current, > + "Not expecting ptrace on self: TM regs may be incorrect\n"); > + > + /* > + * If task is not current, it should have been flushed > + * already to it's thread_struct during __switch_to(). > + */
I totally agree except this highlights something that I notice in subsequent patches, and existing code. All the *_{get,set}() functions call flush_*_to_thread() when, as per your comment (and my understanding of task switching) there really shouldn't be a need to do that. My only thought is that this could be a relic of uniprocessor days when it would have been necessary but Anton recently stripped that out. Are you able to shed some light here?
The reason I ask is that if the flush_*_to_thread() calls ARE actually important then I worry that this function is inadequate...
> +} > +#endif > + > struct task_struct *__switch_to(struct task_struct *prev, > struct task_struct *new) > {
| |