From: Shrikanth Hegde <sshegde@linux.ibm.com>
Add PowerPC arch support for PREEMPT_LAZY by defining LAZY bits.
Since PowerPC doesn't use generic exit to functions, check for
NEED_RESCHED_LAZY when exiting to user or to the kernel from
interrupt routines.
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
[ Changed TIF_NEED_RESCHED_LAZY to now be defined unconditionally. ]
Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/thread_info.h | 5 ++++-
arch/powerpc/kernel/interrupt.c | 5 +++--
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 8094a01974cc..593a1d60d443 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -270,6 +270,7 @@ config PPC
select HAVE_PERF_REGS
select HAVE_PERF_USER_STACK_DUMP
select HAVE_RETHOOK if KPROBES
+ select ARCH_HAS_PREEMPT_LAZY
select HAVE_REGS_AND_STACK_ACCESS_API
select HAVE_RELIABLE_STACKTRACE
select HAVE_RSEQ
diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index 6ebca2996f18..ae7793dae763 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -117,11 +117,14 @@ void arch_setup_new_exec(void);
#endif
#define TIF_POLLING_NRFLAG 19 /* true if poll_idle() is polling TIF_NEED_RESCHED */
#define TIF_32BIT 20 /* 32 bit binary */
+#define TIF_NEED_RESCHED_LAZY 21 /* Lazy rescheduling */
/* as above, but as bit values */
#define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE)
#define _TIF_SIGPENDING (1<<TIF_SIGPENDING)
#define _TIF_NEED_RESCHED (1<<TIF_NEED_RESCHED)
+#define _TIF_NEED_RESCHED_LAZY (1<<TIF_NEED_RESCHED_LAZY)
+
#define _TIF_NOTIFY_SIGNAL (1<<TIF_NOTIFY_SIGNAL)
#define _TIF_POLLING_NRFLAG (1<<TIF_POLLING_NRFLAG)
#define _TIF_32BIT (1<<TIF_32BIT)
@@ -144,7 +147,7 @@ void arch_setup_new_exec(void);
#define _TIF_USER_WORK_MASK (_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
_TIF_NOTIFY_RESUME | _TIF_UPROBE | \
_TIF_RESTORE_TM | _TIF_PATCH_PENDING | \
- _TIF_NOTIFY_SIGNAL)
+ _TIF_NOTIFY_SIGNAL | _TIF_NEED_RESCHED_LAZY)
#define _TIF_PERSYSCALL_MASK (_TIF_RESTOREALL|_TIF_NOERROR)
/* Bits in local_flags */
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index af62ec974b97..86fe60295de7 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -185,7 +185,7 @@ interrupt_exit_user_prepare_main(unsigned long ret, struct pt_regs *regs)
ti_flags = read_thread_flags();
while (unlikely(ti_flags & (_TIF_USER_WORK_MASK & ~_TIF_RESTORE_TM))) {
local_irq_enable();
- if (ti_flags & _TIF_NEED_RESCHED) {
+ if (ti_flags & (_TIF_NEED_RESCHED | _TIF_NEED_RESCHED_LAZY)) {
schedule();
} else {
/*
@@ -396,7 +396,8 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
/* Returning to a kernel context with local irqs enabled. */
WARN_ON_ONCE(!(regs->msr & MSR_EE));
again:
- if (IS_ENABLED(CONFIG_PREEMPT)) {
+
+ if (IS_ENABLED(CONFIG_PREEMPTION)) {
/* Return to preemptible kernel context */
if (unlikely(read_thread_flags() & _TIF_NEED_RESCHED)) {
if (preempt_count() == 0)
--
2.43.5On 2024-10-09 09:54:11 [-0700], Ankur Arora wrote: > From: Shrikanth Hegde <sshegde@linux.ibm.com> > > Add PowerPC arch support for PREEMPT_LAZY by defining LAZY bits. > > Since PowerPC doesn't use generic exit to functions, check for > NEED_RESCHED_LAZY when exiting to user or to the kernel from > interrupt routines. > > Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com> > [ Changed TIF_NEED_RESCHED_LAZY to now be defined unconditionally. ] > Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com> > --- > arch/powerpc/Kconfig | 1 + > arch/powerpc/include/asm/thread_info.h | 5 ++++- > arch/powerpc/kernel/interrupt.c | 5 +++-- > 3 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index 8094a01974cc..593a1d60d443 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -270,6 +270,7 @@ config PPC > select HAVE_PERF_REGS > select HAVE_PERF_USER_STACK_DUMP > select HAVE_RETHOOK if KPROBES > + select ARCH_HAS_PREEMPT_LAZY > select HAVE_REGS_AND_STACK_ACCESS_API > select HAVE_RELIABLE_STACKTRACE > select HAVE_RSEQ I would move this up to the ARCH_HAS_ block. > diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h > index 6ebca2996f18..ae7793dae763 100644 > --- a/arch/powerpc/include/asm/thread_info.h > +++ b/arch/powerpc/include/asm/thread_info.h > @@ -117,11 +117,14 @@ void arch_setup_new_exec(void); > #endif > #define TIF_POLLING_NRFLAG 19 /* true if poll_idle() is polling TIF_NEED_RESCHED */ > #define TIF_32BIT 20 /* 32 bit binary */ > +#define TIF_NEED_RESCHED_LAZY 21 /* Lazy rescheduling */ I don't see any of the bits being used in assembly anymore. If you group the _TIF_USER_WORK_MASK bits it a single 16 bit block then the compiler could issue a single andi. Sebastian
Sebastian Andrzej Siewior <bigeasy@linutronix.de> writes: > On 2024-10-09 09:54:11 [-0700], Ankur Arora wrote: >> From: Shrikanth Hegde <sshegde@linux.ibm.com> >> >> Add PowerPC arch support for PREEMPT_LAZY by defining LAZY bits. >> >> Since PowerPC doesn't use generic exit to functions, check for >> NEED_RESCHED_LAZY when exiting to user or to the kernel from >> interrupt routines. >> >> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com> >> [ Changed TIF_NEED_RESCHED_LAZY to now be defined unconditionally. ] >> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com> >> --- >> arch/powerpc/Kconfig | 1 + >> arch/powerpc/include/asm/thread_info.h | 5 ++++- >> arch/powerpc/kernel/interrupt.c | 5 +++-- >> 3 files changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >> index 8094a01974cc..593a1d60d443 100644 >> --- a/arch/powerpc/Kconfig >> +++ b/arch/powerpc/Kconfig >> @@ -270,6 +270,7 @@ config PPC >> select HAVE_PERF_REGS >> select HAVE_PERF_USER_STACK_DUMP >> select HAVE_RETHOOK if KPROBES >> + select ARCH_HAS_PREEMPT_LAZY >> select HAVE_REGS_AND_STACK_ACCESS_API >> select HAVE_RELIABLE_STACKTRACE >> select HAVE_RSEQ > > I would move this up to the ARCH_HAS_ block. Makes sense. >> diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h >> index 6ebca2996f18..ae7793dae763 100644 >> --- a/arch/powerpc/include/asm/thread_info.h >> +++ b/arch/powerpc/include/asm/thread_info.h >> @@ -117,11 +117,14 @@ void arch_setup_new_exec(void); >> #endif >> #define TIF_POLLING_NRFLAG 19 /* true if poll_idle() is polling TIF_NEED_RESCHED */ >> #define TIF_32BIT 20 /* 32 bit binary */ >> +#define TIF_NEED_RESCHED_LAZY 21 /* Lazy rescheduling */ > > I don't see any of the bits being used in assembly anymore. > If you group the _TIF_USER_WORK_MASK bits it a single 16 bit block then > the compiler could issue a single andi. I don't know power asm at all, but this seems like a good idea. Shrikanth? -- ankur
On 10/10/24 23:40, Ankur Arora wrote: > > Sebastian Andrzej Siewior <bigeasy@linutronix.de> writes: > >> On 2024-10-09 09:54:11 [-0700], Ankur Arora wrote: >>> From: Shrikanth Hegde <sshegde@linux.ibm.com> >>> >>> Add PowerPC arch support for PREEMPT_LAZY by defining LAZY bits. >>> >>> Since PowerPC doesn't use generic exit to functions, check for >>> NEED_RESCHED_LAZY when exiting to user or to the kernel from >>> interrupt routines. >>> >>> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com> >>> [ Changed TIF_NEED_RESCHED_LAZY to now be defined unconditionally. ] >>> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com> >>> --- >>> arch/powerpc/Kconfig | 1 + >>> arch/powerpc/include/asm/thread_info.h | 5 ++++- >>> arch/powerpc/kernel/interrupt.c | 5 +++-- >>> 3 files changed, 8 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig >>> index 8094a01974cc..593a1d60d443 100644 >>> --- a/arch/powerpc/Kconfig >>> +++ b/arch/powerpc/Kconfig >>> @@ -270,6 +270,7 @@ config PPC >>> select HAVE_PERF_REGS >>> select HAVE_PERF_USER_STACK_DUMP >>> select HAVE_RETHOOK if KPROBES >>> + select ARCH_HAS_PREEMPT_LAZY >>> select HAVE_REGS_AND_STACK_ACCESS_API >>> select HAVE_RELIABLE_STACKTRACE >>> select HAVE_RSEQ >> >> I would move this up to the ARCH_HAS_ block. > > Makes sense. > >>> diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h >>> index 6ebca2996f18..ae7793dae763 100644 >>> --- a/arch/powerpc/include/asm/thread_info.h >>> +++ b/arch/powerpc/include/asm/thread_info.h >>> @@ -117,11 +117,14 @@ void arch_setup_new_exec(void); >>> #endif >>> #define TIF_POLLING_NRFLAG 19 /* true if poll_idle() is polling TIF_NEED_RESCHED */ >>> #define TIF_32BIT 20 /* 32 bit binary */ >>> +#define TIF_NEED_RESCHED_LAZY 21 /* Lazy rescheduling */ >> >> I don't see any of the bits being used in assembly anymore. >> If you group the _TIF_USER_WORK_MASK bits it a single 16 bit block then >> the compiler could issue a single andi. > That's a good find. since by default powerpc uses 4 byte fixed ISA, compiler would generate extra code for _TIF_USER_WORK_MASK. Looked at the objdump. It indeed does. I see that value 9 isn't being used. It was last used for TIF_NOHZ which is removed now. That value could be used for RESCHED_LAZY. Using that value i see the code generated is similar to what we have now. +mpe Ankur, Could you please change the value to 9? --------------------------------------------------------------------- I see with value 9, andi is used again. 254: 80 00 dc eb ld r30,128(r28) 258: 4e 62 c9 73 andi. r9,r30,25166 --- a/arch/powerpc/include/asm/thread_info.h +++ b/arch/powerpc/include/asm/thread_info.h @@ -103,6 +103,7 @@ void arch_setup_new_exec(void); #define TIF_PATCH_PENDING 6 /* pending live patching update */ #define TIF_SYSCALL_AUDIT 7 /* syscall auditing active */ #define TIF_SINGLESTEP 8 /* singlestepping active */ +#define TIF_NEED_RESCHED_LAZY 9 /* Lazy rescheduling */ #define TIF_SECCOMP 10 /* secure computing */ #define TIF_RESTOREALL 11 /* Restore all regs (implies NOERROR) */ #define TIF_NOERROR 12 /* Force successful syscall return */ @@ -117,7 +118,6 @@ void arch_setup_new_exec(void); #endif #define TIF_POLLING_NRFLAG 19 /* true if poll_idle() is polling TIF_NEED_RESCHED */ #define TIF_32BIT 20 /* 32 bit binary */ -#define TIF_NEED_RESCHED_LAZY 21 /* Lazy rescheduling */
Shrikanth Hegde <sshegde@linux.ibm.com> writes: > On 10/10/24 23:40, Ankur Arora wrote: >> >> Sebastian Andrzej Siewior <bigeasy@linutronix.de> writes: >> >>> On 2024-10-09 09:54:11 [-0700], Ankur Arora wrote: >>>> From: Shrikanth Hegde <sshegde@linux.ibm.com> >>>> >>>> Add PowerPC arch support for PREEMPT_LAZY by defining LAZY bits. >> ... >>>> diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h >>>> index 6ebca2996f18..ae7793dae763 100644 >>>> --- a/arch/powerpc/include/asm/thread_info.h >>>> +++ b/arch/powerpc/include/asm/thread_info.h >>>> @@ -117,11 +117,14 @@ void arch_setup_new_exec(void); >>>> #endif >>>> #define TIF_POLLING_NRFLAG 19 /* true if poll_idle() is polling TIF_NEED_RESCHED */ >>>> #define TIF_32BIT 20 /* 32 bit binary */ >>>> +#define TIF_NEED_RESCHED_LAZY 21 /* Lazy rescheduling */ >>> >>> I don't see any of the bits being used in assembly anymore. >>> If you group the _TIF_USER_WORK_MASK bits it a single 16 bit block then >>> the compiler could issue a single andi. > > That's a good find. since by default powerpc uses 4 byte fixed ISA, > compiler would generate extra code for _TIF_USER_WORK_MASK. Looked at > the objdump. It indeed does. > > I see that value 9 isn't being used. It was last used for TIF_NOHZ which > is removed now. That value could be used for RESCHED_LAZY. Using that > value i see the code generated is similar to what we have now. > > +mpe Yep, 9 looks good. I don't think it *really* matters that it's a single andi. on modern CPUs, but seeing as bit 9 is free we may as well use it. cheers
© 2016 - 2024 Red Hat, Inc.