| Message ID | 5fcc82b4-89ae-3bca-10ab-6ad933565cee@kernel.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | sh: add support for TIF_NOTIFY_SIGNAL | expand |
Gentle nudge on this one. On 10/29/20 10:21 AM, Jens Axboe wrote: > Wire up TIF_NOTIFY_SIGNAL handling for sh. > > Cc: linux-sh@vger.kernel.org > Signed-off-by: Jens Axboe <axboe@kernel.dk> > --- > > 5.11 has support queued up for TIF_NOTIFY_SIGNAL, see this posting > for details: > > https://lore.kernel.org/io-uring/20201026203230.386348-1-axboe@kernel.dk/ > > As part of that work, I'm adding TIF_NOTIFY_SIGNAL support to all archs, > as that will enable a set of cleanups once all of them support it. I'm > happy carrying this patch if need be, or it can be funelled through the > arch tree. Let me know. > > arch/sh/include/asm/thread_info.h | 4 +++- > arch/sh/kernel/signal_32.c | 7 +++++-- > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/arch/sh/include/asm/thread_info.h b/arch/sh/include/asm/thread_info.h > index 243ea5150aa0..598d0184ffea 100644 > --- a/arch/sh/include/asm/thread_info.h > +++ b/arch/sh/include/asm/thread_info.h > @@ -105,6 +105,7 @@ extern void init_thread_xstate(void); > #define TIF_SYSCALL_TRACE 0 /* syscall trace active */ > #define TIF_SIGPENDING 1 /* signal pending */ > #define TIF_NEED_RESCHED 2 /* rescheduling necessary */ > +#define TIF_NOTIFY_SIGNAL 3 /* signal notifications exist */ > #define TIF_SINGLESTEP 4 /* singlestepping active */ > #define TIF_SYSCALL_AUDIT 5 /* syscall auditing active */ > #define TIF_SECCOMP 6 /* secure computing */ > @@ -116,6 +117,7 @@ extern void init_thread_xstate(void); > #define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) > #define _TIF_SIGPENDING (1 << TIF_SIGPENDING) > #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED) > +#define _TIF_NOTIFY_SIGNAL (1 << TIF_NOTIFY_SIGNAL) > #define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP) > #define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) > #define _TIF_SECCOMP (1 << TIF_SECCOMP) > @@ -132,7 +134,7 @@ extern void init_thread_xstate(void); > #define _TIF_ALLWORK_MASK (_TIF_SYSCALL_TRACE | _TIF_SIGPENDING | \ > _TIF_NEED_RESCHED | _TIF_SYSCALL_AUDIT | \ > _TIF_SINGLESTEP | _TIF_NOTIFY_RESUME | \ > - _TIF_SYSCALL_TRACEPOINT) > + _TIF_SYSCALL_TRACEPOINT | _TIF_NOTIFY_SIGNAL) > > /* work to do on interrupt/exception return */ > #define _TIF_WORK_MASK (_TIF_ALLWORK_MASK & ~(_TIF_SYSCALL_TRACE | \ > diff --git a/arch/sh/kernel/signal_32.c b/arch/sh/kernel/signal_32.c > index 1add47fd31f6..8cfae5a75edb 100644 > --- a/arch/sh/kernel/signal_32.c > +++ b/arch/sh/kernel/signal_32.c > @@ -466,7 +466,10 @@ static void do_signal(struct pt_regs *regs, unsigned int save_r0) > if (!user_mode(regs)) > return; > > - if (get_signal(&ksig)) { > + if (ti_work & _TIF_NOTIFY_SIGNAL) > + tracehook_notify_signal(); > + > + if ((ti_work & _TIF_SIGPENDING) && get_signal(&ksig)) { > handle_syscall_restart(save_r0, regs, &ksig.ka.sa); > > /* Whee! Actually deliver the signal. */ > @@ -499,7 +502,7 @@ asmlinkage void do_notify_resume(struct pt_regs *regs, unsigned int save_r0, > unsigned long thread_info_flags) > { > /* deal with pending signal delivery */ > - if (thread_info_flags & _TIF_SIGPENDING) > + if (thread_info_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL)) > do_signal(regs, save_r0); > > if (thread_info_flags & _TIF_NOTIFY_RESUME) >
Hi Jens!
On 11/5/20 5:17 PM, Jens Axboe wrote:
> Gentle nudge on this one.
I can build- and boot-test on SH and IA64.
I assume Rich will be fine with the SH changes, not sure about the IA64 and Tony.
Adrian
On 11/5/20 9:20 AM, John Paul Adrian Glaubitz wrote: > Hi Jens! > > On 11/5/20 5:17 PM, Jens Axboe wrote: >> Gentle nudge on this one. > > I can build- and boot-test on SH and IA64. That'd be great, thanks! > I assume Rich will be fine with the SH changes, not sure about the IA64 and Tony. Let's add Tony - maybe he'll have a chance to take a look at the ia64 change.
On 11/5/20 11:15 AM, Jens Axboe wrote: > On 11/5/20 9:20 AM, John Paul Adrian Glaubitz wrote: >> Hi Jens! >> >> On 11/5/20 5:17 PM, Jens Axboe wrote: >>> Gentle nudge on this one. >> >> I can build- and boot-test on SH and IA64. > > That'd be great, thanks! > >> I assume Rich will be fine with the SH changes, not sure about the IA64 and Tony. > > Let's add Tony - maybe he'll have a chance to take a look at the ia64 change. It breaks my ARCH=sh j2_defconfig build: arch/sh/kernel/signal_32.c: In function 'do_signal': arch/sh/kernel/signal_32.c:469:6: error: 'ti_work' undeclared (first use in this function) Admittedly I'm testing a stack of 6 other patches at the same time: [PATCH -next v2] sh: intc: Convert to DEFINE_SHOW_ATTRIBUTE.eml [PATCH] sh: dma: fix kconfig dependency for G2_DMA.eml [PATCH] sh: remove CONFIG_IDE from most defconfig.eml [PATCH] sh: Remove unused HAVE_COPY_THREAD_TLS macro.eml [PATCH v1] sh: Drop ARCH_NR_GPIOS definition.eml [PATCH v2 RESEND +TRIVIAL] arch_sh: hyphenate Non-Uniform in Kconfig prompt.eml But this is the one I need to revert to get 5.10-rc3 to build, the rest compile. Rob
Hi Jens! On 11/5/20 6:15 PM, Jens Axboe wrote: > On 11/5/20 9:20 AM, John Paul Adrian Glaubitz wrote: >> Hi Jens! >> >> On 11/5/20 5:17 PM, Jens Axboe wrote: >>> Gentle nudge on this one. >> >> I can build- and boot-test on SH and IA64. > > That'd be great, thanks! Sorry for the delay. I'm busy at the moment and my SH board is currently building the Perl 5.32 package for Debian. Will try to test your patches by tomorrow, also ia64. Adrian
On 11/9/20 1:15 AM, Rob Landley wrote: > On 11/5/20 11:15 AM, Jens Axboe wrote: >> On 11/5/20 9:20 AM, John Paul Adrian Glaubitz wrote: >>> Hi Jens! >>> >>> On 11/5/20 5:17 PM, Jens Axboe wrote: >>>> Gentle nudge on this one. >>> >>> I can build- and boot-test on SH and IA64. >> >> That'd be great, thanks! >> >>> I assume Rich will be fine with the SH changes, not sure about the IA64 and Tony. >> >> Let's add Tony - maybe he'll have a chance to take a look at the ia64 change. > > It breaks my ARCH=sh j2_defconfig build: > > arch/sh/kernel/signal_32.c: In function 'do_signal': > arch/sh/kernel/signal_32.c:469:6: error: 'ti_work' undeclared (first use in this > function) > > Admittedly I'm testing a stack of 6 other patches at the same time: > > [PATCH -next v2] sh: intc: Convert to DEFINE_SHOW_ATTRIBUTE.eml > [PATCH] sh: dma: fix kconfig dependency for G2_DMA.eml > [PATCH] sh: remove CONFIG_IDE from most defconfig.eml > [PATCH] sh: Remove unused HAVE_COPY_THREAD_TLS macro.eml > [PATCH v1] sh: Drop ARCH_NR_GPIOS definition.eml > [PATCH v2 RESEND +TRIVIAL] arch_sh: hyphenate Non-Uniform in Kconfig prompt.eml > > But this is the one I need to revert to get 5.10-rc3 to build, the rest compile. Yeah that's my fault, this one should be a lot better... commit de2791b15d47a56854054da71064b9634896728b Author: Jens Axboe <axboe@kernel.dk> Date: Fri Oct 9 15:36:35 2020 -0600 sh: add support for TIF_NOTIFY_SIGNAL Wire up TIF_NOTIFY_SIGNAL handling for sh. Cc: linux-sh@vger.kernel.org Signed-off-by: Jens Axboe <axboe@kernel.dk> diff --git a/arch/sh/include/asm/thread_info.h b/arch/sh/include/asm/thread_info.h index 243ea5150aa0..598d0184ffea 100644 --- a/arch/sh/include/asm/thread_info.h +++ b/arch/sh/include/asm/thread_info.h @@ -105,6 +105,7 @@ extern void init_thread_xstate(void); #define TIF_SYSCALL_TRACE 0 /* syscall trace active */ #define TIF_SIGPENDING 1 /* signal pending */ #define TIF_NEED_RESCHED 2 /* rescheduling necessary */ +#define TIF_NOTIFY_SIGNAL 3 /* signal notifications exist */ #define TIF_SINGLESTEP 4 /* singlestepping active */ #define TIF_SYSCALL_AUDIT 5 /* syscall auditing active */ #define TIF_SECCOMP 6 /* secure computing */ @@ -116,6 +117,7 @@ extern void init_thread_xstate(void); #define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) #define _TIF_SIGPENDING (1 << TIF_SIGPENDING) #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED) +#define _TIF_NOTIFY_SIGNAL (1 << TIF_NOTIFY_SIGNAL) #define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP) #define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) #define _TIF_SECCOMP (1 << TIF_SECCOMP) @@ -132,7 +134,7 @@ extern void init_thread_xstate(void); #define _TIF_ALLWORK_MASK (_TIF_SYSCALL_TRACE | _TIF_SIGPENDING | \ _TIF_NEED_RESCHED | _TIF_SYSCALL_AUDIT | \ _TIF_SINGLESTEP | _TIF_NOTIFY_RESUME | \ - _TIF_SYSCALL_TRACEPOINT) + _TIF_SYSCALL_TRACEPOINT | _TIF_NOTIFY_SIGNAL) /* work to do on interrupt/exception return */ #define _TIF_WORK_MASK (_TIF_ALLWORK_MASK & ~(_TIF_SYSCALL_TRACE | \ diff --git a/arch/sh/kernel/signal_32.c b/arch/sh/kernel/signal_32.c index 1add47fd31f6..e78d3e10a203 100644 --- a/arch/sh/kernel/signal_32.c +++ b/arch/sh/kernel/signal_32.c @@ -453,7 +453,8 @@ handle_signal(struct ksignal *ksig, struct pt_regs *regs, unsigned int save_r0) * the kernel can handle, and then we build all the user-level signal handling * stack-frames in one go after that. */ -static void do_signal(struct pt_regs *regs, unsigned int save_r0) +static void do_signal(struct pt_regs *regs, unsigned int save_r0, + unsigned long ti_work) { struct ksignal ksig; @@ -466,7 +467,10 @@ static void do_signal(struct pt_regs *regs, unsigned int save_r0) if (!user_mode(regs)) return; - if (get_signal(&ksig)) { + if (ti_work & _TIF_NOTIFY_SIGNAL) + tracehook_notify_signal(); + + if ((ti_work & _TIF_SIGPENDING) && get_signal(&ksig)) { handle_syscall_restart(save_r0, regs, &ksig.ka.sa); /* Whee! Actually deliver the signal. */ @@ -499,8 +503,8 @@ asmlinkage void do_notify_resume(struct pt_regs *regs, unsigned int save_r0, unsigned long thread_info_flags) { /* deal with pending signal delivery */ - if (thread_info_flags & _TIF_SIGPENDING) - do_signal(regs, save_r0); + if (thread_info_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL)) + do_signal(regs, save_r0, thread_info_flags); if (thread_info_flags & _TIF_NOTIFY_RESUME) tracehook_notify_resume(regs);
On 11/9/20 3:59 AM, John Paul Adrian Glaubitz wrote: > Hi Jens! > > On 11/5/20 6:15 PM, Jens Axboe wrote: >> On 11/5/20 9:20 AM, John Paul Adrian Glaubitz wrote: >>> Hi Jens! >>> >>> On 11/5/20 5:17 PM, Jens Axboe wrote: >>>> Gentle nudge on this one. >>> >>> I can build- and boot-test on SH and IA64. >> >> That'd be great, thanks! > > Sorry for the delay. I'm busy at the moment and my SH board is currently > building the Perl 5.32 package for Debian. Will try to test your patches > by tomorrow, also ia64. Thanks, both would be appreciated! Just CC'ed you on the updated patch for sh.
On 11/9/20 8:15 AM, Rob Landley wrote: > > > On 11/9/20 8:14 AM, Jens Axboe wrote: >> On 11/9/20 1:15 AM, Rob Landley wrote: >>> On 11/5/20 11:15 AM, Jens Axboe wrote: >>>> On 11/5/20 9:20 AM, John Paul Adrian Glaubitz wrote: >>>>> Hi Jens! >>>>> >>>>> On 11/5/20 5:17 PM, Jens Axboe wrote: >>>>>> Gentle nudge on this one. >>>>> >>>>> I can build- and boot-test on SH and IA64. >>>> >>>> That'd be great, thanks! >>>> >>>>> I assume Rich will be fine with the SH changes, not sure about the IA64 and Tony. >>>> >>>> Let's add Tony - maybe he'll have a chance to take a look at the ia64 change. >>> >>> It breaks my ARCH=sh j2_defconfig build: >>> >>> arch/sh/kernel/signal_32.c: In function 'do_signal': >>> arch/sh/kernel/signal_32.c:469:6: error: 'ti_work' undeclared (first use in this >>> function) >>> >>> Admittedly I'm testing a stack of 6 other patches at the same time: >>> >>> [PATCH -next v2] sh: intc: Convert to DEFINE_SHOW_ATTRIBUTE.eml >>> [PATCH] sh: dma: fix kconfig dependency for G2_DMA.eml >>> [PATCH] sh: remove CONFIG_IDE from most defconfig.eml >>> [PATCH] sh: Remove unused HAVE_COPY_THREAD_TLS macro.eml >>> [PATCH v1] sh: Drop ARCH_NR_GPIOS definition.eml >>> [PATCH v2 RESEND +TRIVIAL] arch_sh: hyphenate Non-Uniform in Kconfig prompt.eml >>> >>> But this is the one I need to revert to get 5.10-rc3 to build, the rest compile. >> >> Yeah that's my fault, this one should be a lot better... > > arch/sh/kernel/signal_32.c: In function 'do_signal': > arch/sh/kernel/signal_32.c:471:3: error: implicit declaration of function > 'tracehook_notify_signal'; did you mean 'tracehook_notify_resume'? > [-Werror=implicit-function-declaration] > > Keep 'em coming... Gah, it was still using the old style. This one should work and be correct, promise, double checked :-) commit 748887e0b8e7557d79a04e0f8e930027770d7b28 Author: Jens Axboe <axboe@kernel.dk> Date: Fri Oct 9 15:36:35 2020 -0600 sh: add support for TIF_NOTIFY_SIGNAL Wire up TIF_NOTIFY_SIGNAL handling for sh. Cc: linux-sh@vger.kernel.org Signed-off-by: Jens Axboe <axboe@kernel.dk> diff --git a/arch/sh/include/asm/thread_info.h b/arch/sh/include/asm/thread_info.h index 243ea5150aa0..598d0184ffea 100644 --- a/arch/sh/include/asm/thread_info.h +++ b/arch/sh/include/asm/thread_info.h @@ -105,6 +105,7 @@ extern void init_thread_xstate(void); #define TIF_SYSCALL_TRACE 0 /* syscall trace active */ #define TIF_SIGPENDING 1 /* signal pending */ #define TIF_NEED_RESCHED 2 /* rescheduling necessary */ +#define TIF_NOTIFY_SIGNAL 3 /* signal notifications exist */ #define TIF_SINGLESTEP 4 /* singlestepping active */ #define TIF_SYSCALL_AUDIT 5 /* syscall auditing active */ #define TIF_SECCOMP 6 /* secure computing */ @@ -116,6 +117,7 @@ extern void init_thread_xstate(void); #define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) #define _TIF_SIGPENDING (1 << TIF_SIGPENDING) #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED) +#define _TIF_NOTIFY_SIGNAL (1 << TIF_NOTIFY_SIGNAL) #define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP) #define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) #define _TIF_SECCOMP (1 << TIF_SECCOMP) @@ -132,7 +134,7 @@ extern void init_thread_xstate(void); #define _TIF_ALLWORK_MASK (_TIF_SYSCALL_TRACE | _TIF_SIGPENDING | \ _TIF_NEED_RESCHED | _TIF_SYSCALL_AUDIT | \ _TIF_SINGLESTEP | _TIF_NOTIFY_RESUME | \ - _TIF_SYSCALL_TRACEPOINT) + _TIF_SYSCALL_TRACEPOINT | _TIF_NOTIFY_SIGNAL) /* work to do on interrupt/exception return */ #define _TIF_WORK_MASK (_TIF_ALLWORK_MASK & ~(_TIF_SYSCALL_TRACE | \ diff --git a/arch/sh/kernel/signal_32.c b/arch/sh/kernel/signal_32.c index 1add47fd31f6..dd3092911efa 100644 --- a/arch/sh/kernel/signal_32.c +++ b/arch/sh/kernel/signal_32.c @@ -499,7 +499,7 @@ asmlinkage void do_notify_resume(struct pt_regs *regs, unsigned int save_r0, unsigned long thread_info_flags) { /* deal with pending signal delivery */ - if (thread_info_flags & _TIF_SIGPENDING) + if (thread_info_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL)) do_signal(regs, save_r0); if (thread_info_flags & _TIF_NOTIFY_RESUME)
On 11/9/20 8:14 AM, Jens Axboe wrote: > On 11/9/20 1:15 AM, Rob Landley wrote: >> On 11/5/20 11:15 AM, Jens Axboe wrote: >>> On 11/5/20 9:20 AM, John Paul Adrian Glaubitz wrote: >>>> Hi Jens! >>>> >>>> On 11/5/20 5:17 PM, Jens Axboe wrote: >>>>> Gentle nudge on this one. >>>> >>>> I can build- and boot-test on SH and IA64. >>> >>> That'd be great, thanks! >>> >>>> I assume Rich will be fine with the SH changes, not sure about the IA64 and Tony. >>> >>> Let's add Tony - maybe he'll have a chance to take a look at the ia64 change. >> >> It breaks my ARCH=sh j2_defconfig build: >> >> arch/sh/kernel/signal_32.c: In function 'do_signal': >> arch/sh/kernel/signal_32.c:469:6: error: 'ti_work' undeclared (first use in this >> function) >> >> Admittedly I'm testing a stack of 6 other patches at the same time: >> >> [PATCH -next v2] sh: intc: Convert to DEFINE_SHOW_ATTRIBUTE.eml >> [PATCH] sh: dma: fix kconfig dependency for G2_DMA.eml >> [PATCH] sh: remove CONFIG_IDE from most defconfig.eml >> [PATCH] sh: Remove unused HAVE_COPY_THREAD_TLS macro.eml >> [PATCH v1] sh: Drop ARCH_NR_GPIOS definition.eml >> [PATCH v2 RESEND +TRIVIAL] arch_sh: hyphenate Non-Uniform in Kconfig prompt.eml >> >> But this is the one I need to revert to get 5.10-rc3 to build, the rest compile. > > Yeah that's my fault, this one should be a lot better... arch/sh/kernel/signal_32.c: In function 'do_signal': arch/sh/kernel/signal_32.c:471:3: error: implicit declaration of function 'tracehook_notify_signal'; did you mean 'tracehook_notify_resume'? [-Werror=implicit-function-declaration] Keep 'em coming... Rob
On 11/9/20 9:34 AM, Rob Landley wrote: > On 11/9/20 9:10 AM, Jens Axboe wrote: >> On 11/9/20 8:15 AM, Rob Landley wrote: >>> >>> >>> On 11/9/20 8:14 AM, Jens Axboe wrote: >>>> On 11/9/20 1:15 AM, Rob Landley wrote: >>>>> On 11/5/20 11:15 AM, Jens Axboe wrote: >>>>>> On 11/5/20 9:20 AM, John Paul Adrian Glaubitz wrote: >>>>>>> Hi Jens! >>>>>>> >>>>>>> On 11/5/20 5:17 PM, Jens Axboe wrote: >>>>>>>> Gentle nudge on this one. >>>>>>> >>>>>>> I can build- and boot-test on SH and IA64. >>>>>> >>>>>> That'd be great, thanks! >>>>>> >>>>>>> I assume Rich will be fine with the SH changes, not sure about the IA64 and Tony. >>>>>> >>>>>> Let's add Tony - maybe he'll have a chance to take a look at the ia64 change. >>>>> >>>>> It breaks my ARCH=sh j2_defconfig build: >>>>> >>>>> arch/sh/kernel/signal_32.c: In function 'do_signal': >>>>> arch/sh/kernel/signal_32.c:469:6: error: 'ti_work' undeclared (first use in this >>>>> function) >>>>> >>>>> Admittedly I'm testing a stack of 6 other patches at the same time: >>>>> >>>>> [PATCH -next v2] sh: intc: Convert to DEFINE_SHOW_ATTRIBUTE.eml >>>>> [PATCH] sh: dma: fix kconfig dependency for G2_DMA.eml >>>>> [PATCH] sh: remove CONFIG_IDE from most defconfig.eml >>>>> [PATCH] sh: Remove unused HAVE_COPY_THREAD_TLS macro.eml >>>>> [PATCH v1] sh: Drop ARCH_NR_GPIOS definition.eml >>>>> [PATCH v2 RESEND +TRIVIAL] arch_sh: hyphenate Non-Uniform in Kconfig prompt.eml >>>>> >>>>> But this is the one I need to revert to get 5.10-rc3 to build, the rest compile. >>>> >>>> Yeah that's my fault, this one should be a lot better... >>> >>> arch/sh/kernel/signal_32.c: In function 'do_signal': >>> arch/sh/kernel/signal_32.c:471:3: error: implicit declaration of function >>> 'tracehook_notify_signal'; did you mean 'tracehook_notify_resume'? >>> [-Werror=implicit-function-declaration] >>> >>> Keep 'em coming... >> >> Gah, it was still using the old style. This one should work and be correct, >> promise, double checked :-) > > This one compiled fine. What does it do? (I ask having read > https://lwn.net/Articles/835340/ and come out none the wiser.) The motivation is in another patch: https://git.kernel.dk/cgit/linux-block/commit/?h=tif-task_work&id=fdb5f027ce662d1e10d8d16793b1f588b8543277 Basically it decouples TWA_SIGNAL task_work from actual signals, since they contend on the sighand lock, particularly for threads. Hence the goal is to get all archs supporting TIF_NOTIFY_SIGNAL, so we can use that for TWA_SIGNAL. The patches are done such that the signal handling core still handles running the task_work, but if invoked without TIF_SIGPENDING, we don't do actual signal delivery, just the syscall restart part. > I can try it on hardware in the morning, it's 1am in this time zone... Thanks, that'd be great! It really should be a no-op, as you can see from the final patch, it's just masking in an extra flag for when to call get_signal(). At least it should've been, if I hadn't neglected to properly updated the 'sh' patch for the cleaner setup...
On 11/9/20 9:10 AM, Jens Axboe wrote: > On 11/9/20 8:15 AM, Rob Landley wrote: >> >> >> On 11/9/20 8:14 AM, Jens Axboe wrote: >>> On 11/9/20 1:15 AM, Rob Landley wrote: >>>> On 11/5/20 11:15 AM, Jens Axboe wrote: >>>>> On 11/5/20 9:20 AM, John Paul Adrian Glaubitz wrote: >>>>>> Hi Jens! >>>>>> >>>>>> On 11/5/20 5:17 PM, Jens Axboe wrote: >>>>>>> Gentle nudge on this one. >>>>>> >>>>>> I can build- and boot-test on SH and IA64. >>>>> >>>>> That'd be great, thanks! >>>>> >>>>>> I assume Rich will be fine with the SH changes, not sure about the IA64 and Tony. >>>>> >>>>> Let's add Tony - maybe he'll have a chance to take a look at the ia64 change. >>>> >>>> It breaks my ARCH=sh j2_defconfig build: >>>> >>>> arch/sh/kernel/signal_32.c: In function 'do_signal': >>>> arch/sh/kernel/signal_32.c:469:6: error: 'ti_work' undeclared (first use in this >>>> function) >>>> >>>> Admittedly I'm testing a stack of 6 other patches at the same time: >>>> >>>> [PATCH -next v2] sh: intc: Convert to DEFINE_SHOW_ATTRIBUTE.eml >>>> [PATCH] sh: dma: fix kconfig dependency for G2_DMA.eml >>>> [PATCH] sh: remove CONFIG_IDE from most defconfig.eml >>>> [PATCH] sh: Remove unused HAVE_COPY_THREAD_TLS macro.eml >>>> [PATCH v1] sh: Drop ARCH_NR_GPIOS definition.eml >>>> [PATCH v2 RESEND +TRIVIAL] arch_sh: hyphenate Non-Uniform in Kconfig prompt.eml >>>> >>>> But this is the one I need to revert to get 5.10-rc3 to build, the rest compile. >>> >>> Yeah that's my fault, this one should be a lot better... >> >> arch/sh/kernel/signal_32.c: In function 'do_signal': >> arch/sh/kernel/signal_32.c:471:3: error: implicit declaration of function >> 'tracehook_notify_signal'; did you mean 'tracehook_notify_resume'? >> [-Werror=implicit-function-declaration] >> >> Keep 'em coming... > > Gah, it was still using the old style. This one should work and be correct, > promise, double checked :-) This one compiled fine. What does it do? (I ask having read https://lwn.net/Articles/835340/ and come out none the wiser.) I can try it on hardware in the morning, it's 1am in this time zone... Rob
Hi Jens! On 11/9/20 3:14 PM, Jens Axboe wrote: >> Sorry for the delay. I'm busy at the moment and my SH board is currently >> building the Perl 5.32 package for Debian. Will try to test your patches >> by tomorrow, also ia64. > > Thanks, both would be appreciated! Just CC'ed you on the updated patch > for sh. Is this still relevant for testing? I'm ready to test now, much later than I thought, sorry. I'm going to build Linus' latest kernel for my SH and IA64 machines now and then I can test additional patches on top of it. Adrian
On 11/16/20 10:26 PM, John Paul Adrian Glaubitz wrote: > Hi Jens! > > On 11/9/20 3:14 PM, Jens Axboe wrote: >>> Sorry for the delay. I'm busy at the moment and my SH board is currently >>> building the Perl 5.32 package for Debian. Will try to test your patches >>> by tomorrow, also ia64. >> >> Thanks, both would be appreciated! Just CC'ed you on the updated patch >> for sh. > > Is this still relevant for testing? I'm ready to test now, much later than > I thought, sorry. > > I'm going to build Linus' latest kernel for my SH and IA64 machines now > and then I can test additional patches on top of it. Thanks, would definitely still appreciate testing. You can just run linux-next if you want, it's got everything in there. Or apply the separate patches to -git, either approach is fine.
Hi Jens! On 11/17/20 4:06 PM, Jens Axboe wrote: > On 11/16/20 10:26 PM, John Paul Adrian Glaubitz wrote: >> Hi Jens! >> >> On 11/9/20 3:14 PM, Jens Axboe wrote: >>>> Sorry for the delay. I'm busy at the moment and my SH board is currently >>>> building the Perl 5.32 package for Debian. Will try to test your patches >>>> by tomorrow, also ia64. >>> >>> Thanks, both would be appreciated! Just CC'ed you on the updated patch >>> for sh. >> >> Is this still relevant for testing? I'm ready to test now, much later than >> I thought, sorry. >> >> I'm going to build Linus' latest kernel for my SH and IA64 machines now >> and then I can test additional patches on top of it. > > Thanks, would definitely still appreciate testing. You can just run > linux-next if you want, it's got everything in there. Or apply the > separate patches to -git, either approach is fine. Apologies for the late reply. I just pulled Linus' latest tree today with your patch for SH included and I'm not seeing any regressions. Is there away to test the change itself? Adrian
On 1/1/21 7:06 AM, John Paul Adrian Glaubitz wrote: > Hi Jens! > > On 11/17/20 4:06 PM, Jens Axboe wrote: >> On 11/16/20 10:26 PM, John Paul Adrian Glaubitz wrote: >>> Hi Jens! >>> >>> On 11/9/20 3:14 PM, Jens Axboe wrote: >>>>> Sorry for the delay. I'm busy at the moment and my SH board is currently >>>>> building the Perl 5.32 package for Debian. Will try to test your patches >>>>> by tomorrow, also ia64. >>>> >>>> Thanks, both would be appreciated! Just CC'ed you on the updated patch >>>> for sh. >>> >>> Is this still relevant for testing? I'm ready to test now, much later than >>> I thought, sorry. >>> >>> I'm going to build Linus' latest kernel for my SH and IA64 machines now >>> and then I can test additional patches on top of it. >> >> Thanks, would definitely still appreciate testing. You can just run >> linux-next if you want, it's got everything in there. Or apply the >> separate patches to -git, either approach is fine. > > Apologies for the late reply. > > I just pulled Linus' latest tree today with your patch for SH included and > I'm not seeing any regressions. > > Is there away to test the change itself? The only user of TWA_SIGNAL, which uses TIF_NOTIFY_SIGNAL, so far is io_uring. You need something that triggers deferred task_work processing, which is basically anything that ends up being poll driven for data/space readiness. Here's a small test app from the liburing test suite, that'll trigger it. If you install liburing, compile with: gcc -Wall -O2 -o socket-rw socket-rw.c -luring and run it without any arguments.
Hi Jens! On 1/1/21 4:08 PM, Jens Axboe wrote: > On 1/1/21 7:06 AM, John Paul Adrian Glaubitz wrote: >> Is there away to test the change itself? > > The only user of TWA_SIGNAL, which uses TIF_NOTIFY_SIGNAL, so far is io_uring. > You need something that triggers deferred task_work processing, which is > basically anything that ends up being poll driven for data/space readiness. > Here's a small test app from the liburing test suite, that'll trigger it. > > If you install liburing, compile with: > > gcc -Wall -O2 -o socket-rw socket-rw.c -luring > > and run it without any arguments. How long is this test supposed to run? It's already been running for some minutes on my 600 MHz machine. Adrian
On 1/1/21 8:30 AM, John Paul Adrian Glaubitz wrote: > Hi Jens! > > On 1/1/21 4:08 PM, Jens Axboe wrote: >> On 1/1/21 7:06 AM, John Paul Adrian Glaubitz wrote: >>> Is there away to test the change itself? >> >> The only user of TWA_SIGNAL, which uses TIF_NOTIFY_SIGNAL, so far is io_uring. >> You need something that triggers deferred task_work processing, which is >> basically anything that ends up being poll driven for data/space readiness. >> Here's a small test app from the liburing test suite, that'll trigger it. >> >> If you install liburing, compile with: >> >> gcc -Wall -O2 -o socket-rw socket-rw.c -luring >> >> and run it without any arguments. > > How long is this test supposed to run? It's already been running for some minutes > on my 600 MHz machine. It's supposed to finish very quickly: axboe@p1 ~> time ./socket-rw 0.000s
On 1/1/21 4:35 PM, Jens Axboe wrote:> It's supposed to finish very quickly: > > axboe@p1 ~> time ./socket-rw 0.000s > > ________________________________________________________ > Executed in 1.10 millis fish external > usr time 888.00 micros 278.00 micros 610.00 micros > sys time 35.00 micros 35.00 micros 0.00 micros > > If it doesn't, can you try: > > # echo 1 > /sys/kernel/debug/tracing/events/io_uring/enable > > Then run the socket-rw app, and then do: > > # cat /sys/kernel/debug/tracing/trace > > and send that output? Might also be useful to include the strace > of the socket-rw just in case, so maybe run it ala > > strace -o foo ./socket-rw > > and include foo in the reply as well. Odd, I just ran it through strace and it exited immediately: root@tirpitz:~> strace ./socket-rw execve("./socket-rw", ["./socket-rw"], 0x7bb26670 /* 25 vars */) = 0 brk(NULL) = 0x412000 uname({sysname="Linux", nodename="tirpitz", ...}) = 0 access("/etc/ld.so.preload", R_OK) = -1 ENOENT (No such file or directory) openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 3 fstat64(3, {st_mode=S_IFREG|0644, st_size=64232, ...}) = 0 mmap2(NULL, 64232, PROT_READ, MAP_PRIVATE, 3, 0) = 0x29584000 close(3) = 0 mmap2(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x29574000 openat(AT_FDCWD, "/usr/lib/sh4-linux-gnu/liburing.so.1", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 3 read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0*\0\1\0\0\0\324\17\0\0004\0\0\0"..., 512) = 512 fstat64(3, {st_mode=S_IFREG|0644, st_size=9536, ...}) = 0 mmap2(NULL, 73852, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x29594000 mprotect(0x29596000, 61440, PROT_NONE) = 0 mmap2(0x295a5000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1000) = 0x295a5000 close(3) = 0 openat(AT_FDCWD, "/lib/sh4-linux-gnu/libc.so.6", O_RDONLY|O_LARGEFILE|O_CLOEXEC) = 3 read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0*\0\1\0\0\0\224\306\1\0004\0\0\0"..., 512) = 512 pread64(3, "\4\0\0\0\20\0\0\0\1\0\0\0GNU\0\0\0\0\0\3\0\0\0\2\0\0\0\0\0\0\0", 32, 1290836) = 32 fstat64(3, {st_mode=S_IFREG|0755, st_size=1297036, ...}) = 0 mmap2(NULL, 1366256, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x295a8000 mprotect(0x296e1000, 61440, PROT_NONE) = 0 mmap2(0x296f0000, 16384, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x138000) = 0x296f0000 mmap2(0x296f4000, 6384, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x296f4000 close(3) = 0 mprotect(0x296f0000, 8192, PROT_READ) = 0 mprotect(0x295a5000, 4096, PROT_READ) = 0 mprotect(0x410000, 4096, PROT_READ) = 0 mprotect(0x29582000, 4096, PROT_READ) = 0 munmap(0x29584000, 64232) = 0 socket(AF_INET, SOCK_STREAM|SOCK_CLOEXEC, IPPROTO_TCP) = 3 setsockopt(3, SOL_SOCKET, SO_REUSEPORT, [1], 4) = 0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0 bind(3, {sa_family=AF_INET, sin_port=htons(13586), sin_addr=inet_addr("127.0.0.1")}, 16) = 0 listen(3, 128) = 0 socket(AF_INET, SOCK_STREAM|SOCK_CLOEXEC, IPPROTO_TCP) = 4 setsockopt(4, SOL_TCP, TCP_NODELAY, [1], 4) = 0 fcntl64(4, F_GETFL) = 0x2 (flags O_RDWR) fcntl64(4, F_SETFL, O_RDWR|O_NONBLOCK) = 0 connect(4, {sa_family=AF_INET, sin_port=htons(13586), sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRESS (Operation now in progress) fcntl64(4, F_GETFL) = 0x802 (flags O_RDWR|O_NONBLOCK) fcntl64(4, F_SETFL, O_RDWR) = 0 accept(3, NULL, NULL) = 5 getsockopt(4, SOL_SOCKET, SO_ERROR, [0], [4]) = 0 io_uring_setup(32, {flags=0, sq_thread_cpu=0, sq_thread_idle=0, sq_entries=32, cq_entries=64, features=IORING_FEAT_SINGLE_MMAP|IORING_FEAT_NODROP|IORING_FEAT_SUBMIT_STABLE|IORING_FEAT_RW_CUR_POS|IORING_FEAT_CUR_PERSONALITY|0x1e0, sq_off={head=0, tail=4, ring_mask=16, ring_entries=24, flags=36, dropped=32, array=1072}, cq_off={head=8, tail=12, ring_mask=20, ring_entries=28, overflow=44, cqes=48, resv=[0x28, 0]}}) = 6 mmap2(NULL, 1200, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_POPULATE, 6, 0) = 0x29576000 mmap2(NULL, 2048, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_POPULATE, 6, 0x10000000) = 0x29578000 io_uring_enter(6, 2, 2, IORING_ENTER_GETEVENTS, NULL, 8) = 2 munmap(0x29578000, 2048) = 0 munmap(0x29576000, 1200) = 0 close(6) = 0 exit_group(0) = ? +++ exited with 0 +++ root@tirpitz:~>
On 1/1/21 11:16 AM, John Paul Adrian Glaubitz wrote: > On 1/1/21 4:35 PM, Jens Axboe wrote:> It's supposed to finish very quickly: >> >> axboe@p1 ~> time ./socket-rw 0.000s >> >> ________________________________________________________ >> Executed in 1.10 millis fish external >> usr time 888.00 micros 278.00 micros 610.00 micros >> sys time 35.00 micros 35.00 micros 0.00 micros >> >> If it doesn't, can you try: >> >> # echo 1 > /sys/kernel/debug/tracing/events/io_uring/enable >> >> Then run the socket-rw app, and then do: >> >> # cat /sys/kernel/debug/tracing/trace >> >> and send that output? Might also be useful to include the strace >> of the socket-rw just in case, so maybe run it ala >> >> strace -o foo ./socket-rw >> >> and include foo in the reply as well. > > Odd, I just ran it through strace and it exited immediately: That looks correct, it sets up a ring, issues and waits for two requests. Both of those requests require TWA_SIGNAL processing.
diff --git a/arch/sh/include/asm/thread_info.h b/arch/sh/include/asm/thread_info.h index 243ea5150aa0..598d0184ffea 100644 --- a/arch/sh/include/asm/thread_info.h +++ b/arch/sh/include/asm/thread_info.h @@ -105,6 +105,7 @@ extern void init_thread_xstate(void); #define TIF_SYSCALL_TRACE 0 /* syscall trace active */ #define TIF_SIGPENDING 1 /* signal pending */ #define TIF_NEED_RESCHED 2 /* rescheduling necessary */ +#define TIF_NOTIFY_SIGNAL 3 /* signal notifications exist */ #define TIF_SINGLESTEP 4 /* singlestepping active */ #define TIF_SYSCALL_AUDIT 5 /* syscall auditing active */ #define TIF_SECCOMP 6 /* secure computing */ @@ -116,6 +117,7 @@ extern void init_thread_xstate(void); #define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) #define _TIF_SIGPENDING (1 << TIF_SIGPENDING) #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED) +#define _TIF_NOTIFY_SIGNAL (1 << TIF_NOTIFY_SIGNAL) #define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP) #define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) #define _TIF_SECCOMP (1 << TIF_SECCOMP) @@ -132,7 +134,7 @@ extern void init_thread_xstate(void); #define _TIF_ALLWORK_MASK (_TIF_SYSCALL_TRACE | _TIF_SIGPENDING | \ _TIF_NEED_RESCHED | _TIF_SYSCALL_AUDIT | \ _TIF_SINGLESTEP | _TIF_NOTIFY_RESUME | \ - _TIF_SYSCALL_TRACEPOINT) + _TIF_SYSCALL_TRACEPOINT | _TIF_NOTIFY_SIGNAL) /* work to do on interrupt/exception return */ #define _TIF_WORK_MASK (_TIF_ALLWORK_MASK & ~(_TIF_SYSCALL_TRACE | \ diff --git a/arch/sh/kernel/signal_32.c b/arch/sh/kernel/signal_32.c index 1add47fd31f6..8cfae5a75edb 100644 --- a/arch/sh/kernel/signal_32.c +++ b/arch/sh/kernel/signal_32.c @@ -466,7 +466,10 @@ static void do_signal(struct pt_regs *regs, unsigned int save_r0) if (!user_mode(regs)) return; - if (get_signal(&ksig)) { + if (ti_work & _TIF_NOTIFY_SIGNAL) + tracehook_notify_signal(); + + if ((ti_work & _TIF_SIGPENDING) && get_signal(&ksig)) { handle_syscall_restart(save_r0, regs, &ksig.ka.sa); /* Whee! Actually deliver the signal. */ @@ -499,7 +502,7 @@ asmlinkage void do_notify_resume(struct pt_regs *regs, unsigned int save_r0, unsigned long thread_info_flags) { /* deal with pending signal delivery */ - if (thread_info_flags & _TIF_SIGPENDING) + if (thread_info_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL)) do_signal(regs, save_r0); if (thread_info_flags & _TIF_NOTIFY_RESUME)
Wire up TIF_NOTIFY_SIGNAL handling for sh. Cc: linux-sh@vger.kernel.org Signed-off-by: Jens Axboe <axboe@kernel.dk> --- 5.11 has support queued up for TIF_NOTIFY_SIGNAL, see this posting for details: https://lore.kernel.org/io-uring/20201026203230.386348-1-axboe@kernel.dk/ As part of that work, I'm adding TIF_NOTIFY_SIGNAL support to all archs, as that will enable a set of cleanups once all of them support it. I'm happy carrying this patch if need be, or it can be funelled through the arch tree. Let me know. arch/sh/include/asm/thread_info.h | 4 +++- arch/sh/kernel/signal_32.c | 7 +++++-- 2 files changed, 8 insertions(+), 3 deletions(-)