Re: [PATCH -mm 0/1] signal: simplify set_user_sigmask/restore_user_sigmask

From: Linus Torvalds
Date: Wed Jun 05 2019 - 13:29:16 EST


On Wed, Jun 5, 2019 at 8:58 AM Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
>
> To simplify the review, please see the code with this patch applied.
> I am using epoll_pwait() as an example because it looks very simple.

I like it.

However.

I think I'd like it even more if we just said "we don't need
restore_saved_sigmask AT ALL".

Which would be fairly easy to do with something like the attached...

(Yes, this only does x86, which is a problem, but I'm bringing this up
as a RFC..)

Is it worth another TIF flag? This sure would simplify things, and it
really fits the concept too: this really is a do_signal() issue, and
fundamentally goes together with TIF_SIGPENDING.

Linus
arch/x86/entry/common.c | 2 +-
arch/x86/include/asm/thread_info.h | 2 ++
kernel/signal.c | 1 +
3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index a986b3c8294c..eb538ecd6603 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -160,7 +160,7 @@ static void exit_to_usermode_loop(struct pt_regs *regs, u32 cached_flags)
klp_update_patch_state(current);

/* deal with pending signal delivery */
- if (cached_flags & _TIF_SIGPENDING)
+ if (cached_flags & (_TIF_SIGPENDING | _TIF_RESTORE_SIGMASK))
do_signal(regs);

if (cached_flags & _TIF_NOTIFY_RESUME) {
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index f9453536f9bb..d77a9f841455 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -92,6 +92,7 @@ struct thread_info {
#define TIF_NOCPUID 15 /* CPUID is not accessible in userland */
#define TIF_NOTSC 16 /* TSC is not accessible in userland */
#define TIF_IA32 17 /* IA32 compatibility process */
+#define TIF_RESTORE_SIGMASK 18 /* Restore saved sigmask on return to user space */
#define TIF_NOHZ 19 /* in adaptive nohz mode */
#define TIF_MEMDIE 20 /* is terminating due to OOM killer */
#define TIF_POLLING_NRFLAG 21 /* idle is polling for TIF_NEED_RESCHED */
@@ -122,6 +123,7 @@ struct thread_info {
#define _TIF_NOCPUID (1 << TIF_NOCPUID)
#define _TIF_NOTSC (1 << TIF_NOTSC)
#define _TIF_IA32 (1 << TIF_IA32)
+#define _TIF_RESTORE_SIGMASK (1 << TIF_RESTORE_SIGMASK)
#define _TIF_NOHZ (1 << TIF_NOHZ)
#define _TIF_POLLING_NRFLAG (1 << TIF_POLLING_NRFLAG)
#define _TIF_IO_BITMAP (1 << TIF_IO_BITMAP)
diff --git a/kernel/signal.c b/kernel/signal.c
index 328a01e1a2f0..a3346da1a4f5 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2877,6 +2877,7 @@ int set_user_sigmask(const sigset_t __user *usigmask, sigset_t *set,

*oldset = current->blocked;
set_current_blocked(set);
+ set_thread_flag(TIF_RESTORE_SIGMASK);

return 0;
}