| Message ID | 20200127141712.96738-1-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc/64: system call implement the bulk of the logic in C fix | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch powerpc/merge (23f1be8476528cfab2a015de5a4c5cecf69536d0) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch powerpc/next (21613cfad181c882b1effd227dcfbddc61dc80f7) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch linus/master (d5226fa6dbae0569ee43ecfc08bdcd6770fc4755) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch powerpc/fixes (5d2e5dd5849b4ef5e8ec35e812cdb732c13cd27e) |
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch linux-next (702ccea170f07783bd002055a353a0866c062267) |
| snowpatch_ozlabs/apply_patch | fail | Failed to apply to any branch |
On Tue, Jan 28, 2020 at 12:17:12AM +1000, Nicholas Piggin wrote: > This incremental patch fixes several soft-mask debug and unsafe > smp_processor_id messages due to tracing and false positives in > "unreconciled" code. > > It also fixes a bug with syscall tracing functions that set registers > (e.g., PTRACE_SETREG) not setting GPRs properly. > > There was a bug reported with the TM selftests, I haven't been able > to reproduce that one. > > I can squash this into the main patch and resend the series if it > helps but the incremental helps to see the bug fixes. There are some whitespace differences between this and the series I have applied locally. What does it apply to? Is there some revision of the patchset I missed? Thanks Michal > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > arch/powerpc/include/asm/cputime.h | 39 +++++++++++++++++------------- > arch/powerpc/kernel/syscall_64.c | 26 ++++++++++++++------ > 2 files changed, 41 insertions(+), 24 deletions(-) > > diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h > index c43614cffaac..6639a6847cc0 100644 > --- a/arch/powerpc/include/asm/cputime.h > +++ b/arch/powerpc/include/asm/cputime.h > @@ -44,6 +44,28 @@ static inline unsigned long cputime_to_usecs(const cputime_t ct) > #ifdef CONFIG_PPC64 > #define get_accounting(tsk) (&get_paca()->accounting) > static inline void arch_vtime_task_switch(struct task_struct *tsk) { } > + > +/* > + * account_cpu_user_entry/exit runs "unreconciled", so can't trace, > + * can't use use get_paca() > + */ > +static notrace inline void account_cpu_user_entry(void) > +{ > + unsigned long tb = mftb(); > + struct cpu_accounting_data *acct = &local_paca->accounting; > + > + acct->utime += (tb - acct->starttime_user); > + acct->starttime = tb; > +} > +static notrace inline void account_cpu_user_exit(void) > +{ > + unsigned long tb = mftb(); > + struct cpu_accounting_data *acct = &local_paca->accounting; > + > + acct->stime += (tb - acct->starttime); > + acct->starttime_user = tb; > +} > + > #else > #define get_accounting(tsk) (&task_thread_info(tsk)->accounting) > /* > @@ -60,23 +82,6 @@ static inline void arch_vtime_task_switch(struct task_struct *prev) > } > #endif > > -static inline void account_cpu_user_entry(void) > -{ > - unsigned long tb = mftb(); > - struct cpu_accounting_data *acct = get_accounting(current); > - > - acct->utime += (tb - acct->starttime_user); > - acct->starttime = tb; > -} > -static inline void account_cpu_user_exit(void) > -{ > - unsigned long tb = mftb(); > - struct cpu_accounting_data *acct = get_accounting(current); > - > - acct->stime += (tb - acct->starttime); > - acct->starttime_user = tb; > -} > - > #endif /* __KERNEL__ */ > #else /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */ > static inline void account_cpu_user_entry(void) > diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c > index 529393a1ff1e..cfe458adde07 100644 > --- a/arch/powerpc/kernel/syscall_64.c > +++ b/arch/powerpc/kernel/syscall_64.c > @@ -19,7 +19,8 @@ extern void __noreturn tabort_syscall(void); > > typedef long (*syscall_fn)(long, long, long, long, long, long); > > -long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8, > +/* Has to run notrace because it is entered "unreconciled" */ > +notrace long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8, > unsigned long r0, struct pt_regs *regs) > { > unsigned long ti_flags; > @@ -36,7 +37,7 @@ long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8, > #ifdef CONFIG_PPC_SPLPAR > if (IS_ENABLED(CONFIG_VIRT_CPU_ACCOUNTING_NATIVE) && > firmware_has_feature(FW_FEATURE_SPLPAR)) { > - struct lppaca *lp = get_lppaca(); > + struct lppaca *lp = local_paca->lppaca_ptr; > > if (unlikely(local_paca->dtl_ridx != be64_to_cpu(lp->dtl_idx))) > accumulate_stolen_time(); > @@ -71,13 +72,22 @@ long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8, > * We use the return value of do_syscall_trace_enter() as the > * syscall number. If the syscall was rejected for any reason > * do_syscall_trace_enter() returns an invalid syscall number > - * and the test below against NR_syscalls will fail. > + * and the test against NR_syscalls will fail and the return > + * value to be used is in regs->gpr[3]. > */ > r0 = do_syscall_trace_enter(regs); > - } > - > - if (unlikely(r0 >= NR_syscalls)) > + if (unlikely(r0 >= NR_syscalls)) > + return regs->gpr[3]; > + r3 = regs->gpr[3]; > + r4 = regs->gpr[4]; > + r5 = regs->gpr[5]; > + r6 = regs->gpr[6]; > + r7 = regs->gpr[7]; > + r8 = regs->gpr[8]; > + > + } else if (unlikely(r0 >= NR_syscalls)) { > return -ENOSYS; > + } > > /* May be faster to do array_index_nospec? */ > barrier_nospec(); > @@ -139,8 +149,10 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3, > regs->gpr[3] = r3; > } > > - if (unlikely(ti_flags & _TIF_SYSCALL_DOTRACE)) > + if (unlikely(ti_flags & _TIF_SYSCALL_DOTRACE)) { > do_syscall_trace_leave(regs); > + ret |= _TIF_RESTOREALL; > + } > > again: > local_irq_disable(); > -- > 2.23.0 >
Michal Suchánek's on January 28, 2020 4:08 am: > On Tue, Jan 28, 2020 at 12:17:12AM +1000, Nicholas Piggin wrote: >> This incremental patch fixes several soft-mask debug and unsafe >> smp_processor_id messages due to tracing and false positives in >> "unreconciled" code. >> >> It also fixes a bug with syscall tracing functions that set registers >> (e.g., PTRACE_SETREG) not setting GPRs properly. >> >> There was a bug reported with the TM selftests, I haven't been able >> to reproduce that one. >> >> I can squash this into the main patch and resend the series if it >> helps but the incremental helps to see the bug fixes. > > There are some whitespace differences between this and the series I have > applied locally. What does it apply to? > > Is there some revision of the patchset I missed? No I may have just missed some of your whitespace cleanups, or maybe I got some that Michael made which you don't have in his next-test branch. Thanks, Nick
On Tue, Jan 28, 2020 at 10:41:02AM +1000, Nicholas Piggin wrote: > Michal Suchánek's on January 28, 2020 4:08 am: > > On Tue, Jan 28, 2020 at 12:17:12AM +1000, Nicholas Piggin wrote: > >> This incremental patch fixes several soft-mask debug and unsafe > >> smp_processor_id messages due to tracing and false positives in > >> "unreconciled" code. > >> > >> It also fixes a bug with syscall tracing functions that set registers > >> (e.g., PTRACE_SETREG) not setting GPRs properly. > >> > >> There was a bug reported with the TM selftests, I haven't been able > >> to reproduce that one. > >> > >> I can squash this into the main patch and resend the series if it > >> helps but the incremental helps to see the bug fixes. > > > > There are some whitespace differences between this and the series I have > > applied locally. What does it apply to? > > > > Is there some revision of the patchset I missed? > > No I may have just missed some of your whitespace cleanups, or maybe I got > some that Michael made which you don't have in his next-test branch. Looks like the latter. I will pick patches from next-test. Thanks Michal
diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h index c43614cffaac..6639a6847cc0 100644 --- a/arch/powerpc/include/asm/cputime.h +++ b/arch/powerpc/include/asm/cputime.h @@ -44,6 +44,28 @@ static inline unsigned long cputime_to_usecs(const cputime_t ct) #ifdef CONFIG_PPC64 #define get_accounting(tsk) (&get_paca()->accounting) static inline void arch_vtime_task_switch(struct task_struct *tsk) { } + +/* + * account_cpu_user_entry/exit runs "unreconciled", so can't trace, + * can't use use get_paca() + */ +static notrace inline void account_cpu_user_entry(void) +{ + unsigned long tb = mftb(); + struct cpu_accounting_data *acct = &local_paca->accounting; + + acct->utime += (tb - acct->starttime_user); + acct->starttime = tb; +} +static notrace inline void account_cpu_user_exit(void) +{ + unsigned long tb = mftb(); + struct cpu_accounting_data *acct = &local_paca->accounting; + + acct->stime += (tb - acct->starttime); + acct->starttime_user = tb; +} + #else #define get_accounting(tsk) (&task_thread_info(tsk)->accounting) /* @@ -60,23 +82,6 @@ static inline void arch_vtime_task_switch(struct task_struct *prev) } #endif -static inline void account_cpu_user_entry(void) -{ - unsigned long tb = mftb(); - struct cpu_accounting_data *acct = get_accounting(current); - - acct->utime += (tb - acct->starttime_user); - acct->starttime = tb; -} -static inline void account_cpu_user_exit(void) -{ - unsigned long tb = mftb(); - struct cpu_accounting_data *acct = get_accounting(current); - - acct->stime += (tb - acct->starttime); - acct->starttime_user = tb; -} - #endif /* __KERNEL__ */ #else /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */ static inline void account_cpu_user_entry(void) diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c index 529393a1ff1e..cfe458adde07 100644 --- a/arch/powerpc/kernel/syscall_64.c +++ b/arch/powerpc/kernel/syscall_64.c @@ -19,7 +19,8 @@ extern void __noreturn tabort_syscall(void); typedef long (*syscall_fn)(long, long, long, long, long, long); -long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8, +/* Has to run notrace because it is entered "unreconciled" */ +notrace long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8, unsigned long r0, struct pt_regs *regs) { unsigned long ti_flags; @@ -36,7 +37,7 @@ long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8, #ifdef CONFIG_PPC_SPLPAR if (IS_ENABLED(CONFIG_VIRT_CPU_ACCOUNTING_NATIVE) && firmware_has_feature(FW_FEATURE_SPLPAR)) { - struct lppaca *lp = get_lppaca(); + struct lppaca *lp = local_paca->lppaca_ptr; if (unlikely(local_paca->dtl_ridx != be64_to_cpu(lp->dtl_idx))) accumulate_stolen_time(); @@ -71,13 +72,22 @@ long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8, * We use the return value of do_syscall_trace_enter() as the * syscall number. If the syscall was rejected for any reason * do_syscall_trace_enter() returns an invalid syscall number - * and the test below against NR_syscalls will fail. + * and the test against NR_syscalls will fail and the return + * value to be used is in regs->gpr[3]. */ r0 = do_syscall_trace_enter(regs); - } - - if (unlikely(r0 >= NR_syscalls)) + if (unlikely(r0 >= NR_syscalls)) + return regs->gpr[3]; + r3 = regs->gpr[3]; + r4 = regs->gpr[4]; + r5 = regs->gpr[5]; + r6 = regs->gpr[6]; + r7 = regs->gpr[7]; + r8 = regs->gpr[8]; + + } else if (unlikely(r0 >= NR_syscalls)) { return -ENOSYS; + } /* May be faster to do array_index_nospec? */ barrier_nospec(); @@ -139,8 +149,10 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3, regs->gpr[3] = r3; } - if (unlikely(ti_flags & _TIF_SYSCALL_DOTRACE)) + if (unlikely(ti_flags & _TIF_SYSCALL_DOTRACE)) { do_syscall_trace_leave(regs); + ret |= _TIF_RESTOREALL; + } again: local_irq_disable();
This incremental patch fixes several soft-mask debug and unsafe smp_processor_id messages due to tracing and false positives in "unreconciled" code. It also fixes a bug with syscall tracing functions that set registers (e.g., PTRACE_SETREG) not setting GPRs properly. There was a bug reported with the TM selftests, I haven't been able to reproduce that one. I can squash this into the main patch and resend the series if it helps but the incremental helps to see the bug fixes. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/include/asm/cputime.h | 39 +++++++++++++++++------------- arch/powerpc/kernel/syscall_64.c | 26 ++++++++++++++------ 2 files changed, 41 insertions(+), 24 deletions(-)