LinuxLists.cc - [PATCH 1/4] set_restore

2008-03-29 00:13:20

Subject: [PATCH 1/4] set_restore_sigmask

This adds the set_restore_sigmask() inline in <linux/thread_info.h> and
replaces every set_thread_flag(TIF_RESTORE_SIGMASK) with a call to it.
No change, but abstracts the flag protocol details from all the calls.

Signed-off-by: Roland McGrath <[email protected]>
---
fs/compat.c | 6 +++---
fs/eventpoll.c | 3 +--
fs/select.c | 4 ++--
include/linux/thread_info.h | 15 ++++++++++++++-
kernel/compat.c | 3 +--
kernel/signal.c | 2 +-
6 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/fs/compat.c b/fs/compat.c
index 2ce4456..9964d54 100644
--- a/fs/compat.c
+++ b/fs/compat.c
@@ -1720,7 +1720,7 @@ sticky:
if (sigmask) {
memcpy(&current->saved_sigmask, &sigsaved,
sizeof(sigsaved));
- set_thread_flag(TIF_RESTORE_SIGMASK);
+ set_restore_sigmask();
}
} else if (sigmask)
sigprocmask(SIG_SETMASK, &sigsaved, NULL);
@@ -1791,7 +1791,7 @@ asmlinkage long compat_sys_ppoll(struct pollfd __user *ufds,
if (sigmask) {
memcpy(&current->saved_sigmask, &sigsaved,
sizeof(sigsaved));
- set_thread_flag(TIF_RESTORE_SIGMASK);
+ set_restore_sigmask();
}
ret = -ERESTARTNOHAND;
} else if (sigmask)
@@ -2117,7 +2117,7 @@ asmlinkage long compat_sys_epoll_pwait(int epfd,
if (err == -EINTR) {
memcpy(&current->saved_sigmask, &sigsaved,
sizeof(sigsaved));
- set_thread_flag(TIF_RESTORE_SIGMASK);
+ set_restore_sigmask();
} else
sigprocmask(SIG_SETMASK, &sigsaved, NULL);
}
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index a415f42..503ffa4 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -1300,7 +1300,7 @@ asmlinkage long sys_epoll_pwait(int epfd, struct epoll_event __user *events,
if (error == -EINTR) {
memcpy(&current->saved_sigmask, &sigsaved,
sizeof(sigsaved));
- set_thread_flag(TIF_RESTORE_SIGMASK);
+ set_restore_sigmask();
} else
sigprocmask(SIG_SETMASK, &sigsaved, NULL);
}
@@ -1330,4 +1330,3 @@ static int __init eventpoll_init(void)
return 0;
}
fs_initcall(eventpoll_init);
-
diff --git a/fs/select.c b/fs/select.c
index 5633fe9..bbd351c 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -498,7 +498,7 @@ sticky:
if (sigmask) {
memcpy(&current->saved_sigmask, &sigsaved,
sizeof(sigsaved));
- set_thread_flag(TIF_RESTORE_SIGMASK);
+ set_restore_sigmask();
}
} else if (sigmask)
sigprocmask(SIG_SETMASK, &sigsaved, NULL);
@@ -805,7 +805,7 @@ asmlinkage long sys_ppoll(struct pollfd __user *ufds, unsigned int nfds,
if (sigmask) {
memcpy(&current->saved_sigmask, &sigsaved,
sizeof(sigsaved));
- set_thread_flag(TIF_RESTORE_SIGMASK);
+ set_restore_sigmask();
}
ret = -ERESTARTNOHAND;
} else if (sigmask)
diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index 421323e..d82c073 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -80,6 +80,19 @@ static inline int test_ti_thread_flag(struct thread_info *ti, int flag)
#define set_need_resched() set_thread_flag(TIF_NEED_RESCHED)
#define clear_need_resched() clear_thread_flag(TIF_NEED_RESCHED)

-#endif
+#ifdef TIF_RESTORE_SIGMASK
+/**
+ * set_restore_sigmask() - make sure saved_sigmask processing gets done
+ *
+ * This sets TIF_RESTORE_SIGMASK and ensures that the arch signal code
+ * will run before returning to user mode, to process the flag.
+ */
+static inline void set_restore_sigmask(void)
+{
+ set_thread_flag(TIF_RESTORE_SIGMASK);
+}
+#endif /* TIF_RESTORE_SIGMASK */
+
+#endif /* __KERNEL__ */

#endif /* _LINUX_THREAD_INFO_H */
diff --git a/kernel/compat.c b/kernel/compat.c
index 5f0e201..a830c84 100644
--- a/kernel/compat.c
+++ b/kernel/compat.c
@@ -899,7 +899,7 @@ asmlinkage long compat_sys_rt_sigsuspend(compat_sigset_t __user *unewset, compat

current->state = TASK_INTERRUPTIBLE;
schedule();
- set_thread_flag(TIF_RESTORE_SIGMASK);
+ set_restore_sigmask();
return -ERESTARTNOHAND;
}
#endif /* __ARCH_WANT_COMPAT_SYS_RT_SIGSUSPEND */
@@ -1081,4 +1081,3 @@ compat_sys_sysinfo(struct compat_sysinfo __user *info)

return 0;
}
-
diff --git a/kernel/signal.c b/kernel/signal.c
index 6af1210..977b3cd 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2610,7 +2610,7 @@ asmlinkage long sys_rt_sigsuspend(sigset_t __user *unewset, size_t sigsetsize)

current->state = TASK_INTERRUPTIBLE;
schedule();
- set_thread_flag(TIF_RESTORE_SIGMASK);
+ set_restore_sigmask();
return -ERESTARTNOHAND;
}
#endif /* __ARCH_WANT_SYS_RT_SIGSUSPEND */

2008-03-29 00:14:25

by Roland McGrath

[permalink] [raw]

Subject: [PATCH 2/4] set_restore_sigmask TIF_SIGPENDING

Set TIF_SIGPENDING in set_restore_sigmask. This lets arch code take
TIF_RESTORE_SIGMASK out of the set of bits that will be noticed on
return to user mode. On some machines those bits are scarce, and we
can free this unneeded one up for other uses.

It is probably the case that TIF_SIGPENDING is always set anyway
everywhere set_restore_sigmask() is used. But this is some cheap
paranoia in case there is an arcane case where it might not be.

Signed-off-by: Roland McGrath <[email protected]>
---
include/linux/thread_info.h | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index d82c073..4a89477 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -85,11 +85,17 @@ static inline int test_ti_thread_flag(struct thread_info *ti, int flag)
* set_restore_sigmask() - make sure saved_sigmask processing gets done
*
* This sets TIF_RESTORE_SIGMASK and ensures that the arch signal code
- * will run before returning to user mode, to process the flag.
+ * will run before returning to user mode, to process the flag. For
+ * all callers, TIF_SIGPENDING is already set or it's no harm to set
+ * it. TIF_RESTORE_SIGMASK need not be in the set of bits that the
+ * arch code will notice on return to user mode, in case those bits
+ * are scarce. We set TIF_SIGPENDING here to ensure that the arch
+ * signal code always gets run when TIF_RESTORE_SIGMASK is set.
*/
static inline void set_restore_sigmask(void)
{
set_thread_flag(TIF_RESTORE_SIGMASK);
+ set_thread_flag(TIF_SIGPENDING);
}
#endif /* TIF_RESTORE_SIGMASK */

2008-03-29 00:15:11

by Roland McGrath

[permalink] [raw]

Subject: [PATCH 3/4] s390 renumber TIF_RESTORE_SIGMASK

TIF_RESTORE_SIGMASK no longer needs to be in the _TIF_WORK_* masks.
Those low bits are scarce, and are all used up now.
Renumber TIF_RESTORE_SIGMASK to free one up.

Signed-off-by: Roland McGrath <[email protected]>
---
arch/s390/kernel/entry.S | 14 +++++++-------
arch/s390/kernel/entry64.S | 12 ++++++------
include/asm-s390/thread_info.h | 2 +-
3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
index 6766e37..bdbb3bc 100644
--- a/arch/s390/kernel/entry.S
+++ b/arch/s390/kernel/entry.S
@@ -49,9 +49,9 @@ SP_ILC = STACK_FRAME_OVERHEAD + __PT_ILC
SP_TRAP = STACK_FRAME_OVERHEAD + __PT_TRAP
SP_SIZE = STACK_FRAME_OVERHEAD + __PT_SIZE

-_TIF_WORK_SVC = (_TIF_SIGPENDING | _TIF_RESTORE_SIGMASK | _TIF_NEED_RESCHED | \
+_TIF_WORK_SVC = (_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
_TIF_MCCK_PENDING | _TIF_RESTART_SVC | _TIF_SINGLE_STEP )
-_TIF_WORK_INT = (_TIF_SIGPENDING | _TIF_RESTORE_SIGMASK | _TIF_NEED_RESCHED | \
+_TIF_WORK_INT = (_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
_TIF_MCCK_PENDING)

STACK_SHIFT = PAGE_SHIFT + THREAD_ORDER
@@ -316,7 +316,7 @@ sysc_work:
bo BASED(sysc_mcck_pending)
tm __TI_flags+3(%r9),_TIF_NEED_RESCHED
bo BASED(sysc_reschedule)
- tm __TI_flags+3(%r9),(_TIF_SIGPENDING | _TIF_RESTORE_SIGMASK)
+ tm __TI_flags+3(%r9),_TIF_SIGPENDING
bnz BASED(sysc_sigpending)
tm __TI_flags+3(%r9),_TIF_RESTART_SVC
bo BASED(sysc_restart)
@@ -342,7 +342,7 @@ sysc_mcck_pending:
br %r1 # TIF bit will be cleared by handler

#
-# _TIF_SIGPENDING or _TIF_RESTORE_SIGMASK is set, call do_signal
+# _TIF_SIGPENDING is set, call do_signal
#
sysc_sigpending:
ni __TI_flags+3(%r9),255-_TIF_SINGLE_STEP # clear TIF_SINGLE_STEP
@@ -657,7 +657,7 @@ io_work:
lr %r15,%r1
#
# One of the work bits is on. Find out which one.
-# Checked are: _TIF_SIGPENDING, _TIF_RESTORE_SIGMASK, _TIF_NEED_RESCHED
+# Checked are: _TIF_SIGPENDING, _TIF_NEED_RESCHED
# and _TIF_MCCK_PENDING
#
io_work_loop:
@@ -665,7 +665,7 @@ io_work_loop:
bo BASED(io_mcck_pending)
tm __TI_flags+3(%r9),_TIF_NEED_RESCHED
bo BASED(io_reschedule)
- tm __TI_flags+3(%r9),(_TIF_SIGPENDING | _TIF_RESTORE_SIGMASK)
+ tm __TI_flags+3(%r9),_TIF_SIGPENDING
bnz BASED(io_sigpending)
b BASED(io_restore)
io_work_done:
@@ -693,7 +693,7 @@ io_reschedule:
b BASED(io_work_loop)

#
-# _TIF_SIGPENDING or _TIF_RESTORE_SIGMASK is set, call do_signal
+# _TIF_SIGPENDING is set, call do_signal
#
io_sigpending:
TRACE_IRQS_ON
diff --git a/arch/s390/kernel/entry64.S b/arch/s390/kernel/entry64.S
index efde6e1..0338579 100644
--- a/arch/s390/kernel/entry64.S
+++ b/arch/s390/kernel/entry64.S
@@ -52,9 +52,9 @@ SP_SIZE = STACK_FRAME_OVERHEAD + __PT_SIZE
STACK_SHIFT = PAGE_SHIFT + THREAD_ORDER
STACK_SIZE = 1 << STACK_SHIFT

-_TIF_WORK_SVC = (_TIF_SIGPENDING | _TIF_RESTORE_SIGMASK | _TIF_NEED_RESCHED | \
+_TIF_WORK_SVC = (_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
_TIF_MCCK_PENDING | _TIF_RESTART_SVC | _TIF_SINGLE_STEP )
-_TIF_WORK_INT = (_TIF_SIGPENDING | _TIF_RESTORE_SIGMASK | _TIF_NEED_RESCHED | \
+_TIF_WORK_INT = (_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
_TIF_MCCK_PENDING)

#define BASED(name) name-system_call(%r13)
@@ -308,7 +308,7 @@ sysc_work:
jo sysc_mcck_pending
tm __TI_flags+7(%r9),_TIF_NEED_RESCHED
jo sysc_reschedule
- tm __TI_flags+7(%r9),(_TIF_SIGPENDING | _TIF_RESTORE_SIGMASK)
+ tm __TI_flags+7(%r9),_TIF_SIGPENDING
jnz sysc_sigpending
tm __TI_flags+7(%r9),_TIF_RESTART_SVC
jo sysc_restart
@@ -332,7 +332,7 @@ sysc_mcck_pending:
jg s390_handle_mcck # TIF bit will be cleared by handler

#
-# _TIF_SIGPENDING or _TIF_RESTORE_SIGMASK is set, call do_signal
+# _TIF_SIGPENDING is set, call do_signal
#
sysc_sigpending:
ni __TI_flags+7(%r9),255-_TIF_SINGLE_STEP # clear TIF_SINGLE_STEP
@@ -647,7 +647,7 @@ io_work_loop:
jo io_mcck_pending
tm __TI_flags+7(%r9),_TIF_NEED_RESCHED
jo io_reschedule
- tm __TI_flags+7(%r9),(_TIF_SIGPENDING | _TIF_RESTORE_SIGMASK)
+ tm __TI_flags+7(%r9),_TIF_SIGPENDING
jnz io_sigpending
j io_restore
io_work_done:
@@ -673,7 +673,7 @@ io_reschedule:
j io_work_loop

#
-# _TIF_SIGPENDING or _TIF_RESTORE_SIGMASK is set, call do_signal
+# _TIF_SIGPENDING or is set, call do_signal
#
io_sigpending:
TRACE_IRQS_ON
diff --git a/include/asm-s390/thread_info.h b/include/asm-s390/thread_info.h
index 0a51891..99bbed9 100644
--- a/include/asm-s390/thread_info.h
+++ b/include/asm-s390/thread_info.h
@@ -89,7 +89,6 @@ static inline struct thread_info *current_thread_info(void)
* thread information flags bit numbers
*/
#define TIF_SYSCALL_TRACE 0 /* syscall trace active */
-#define TIF_RESTORE_SIGMASK 1 /* restore signal mask in do_signal() */
#define TIF_SIGPENDING 2 /* signal pending */
#define TIF_NEED_RESCHED 3 /* rescheduling necessary */
#define TIF_RESTART_SVC 4 /* restart svc with new svc number */
@@ -101,6 +100,7 @@ static inline struct thread_info *current_thread_info(void)
TIF_NEED_RESCHED */
#define TIF_31BIT 18 /* 32bit process */
#define TIF_MEMDIE 19
+#define TIF_RESTORE_SIGMASK 20 /* restore signal mask in do_signal() */

#define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE)
#define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK)

2008-03-29 00:15:33

by Roland McGrath

[permalink] [raw]

Subject: [PATCH 4/4] ia64 renumber TIF_RESTORE_SIGMASK

TIF_RESTORE_SIGMASK no longer needs to be in the _TIF_WORK_* masks.
Those low bits are scarce. Renumber TIF_RESTORE_SIGMASK to free one up.

Signed-off-by: Roland McGrath <[email protected]>
---
arch/ia64/kernel/process.c | 2 +-
include/asm-ia64/thread_info.h | 5 ++---
2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/ia64/kernel/process.c b/arch/ia64/kernel/process.c
index 49937a3..2ef2794 100644
--- a/arch/ia64/kernel/process.c
+++ b/arch/ia64/kernel/process.c
@@ -183,7 +183,7 @@ do_notify_resume_user (sigset_t *unused, struct sigscratch *scr, long in_syscall
#endif

/* deal with pending signal delivery */
- if (test_thread_flag(TIF_SIGPENDING)||test_thread_flag(TIF_RESTORE_SIGMASK))
+ if (test_thread_flag(TIF_SIGPENDING))
ia64_do_signal(scr, in_syscall);

/* copy user rbs to kernel rbs */
diff --git a/include/asm-ia64/thread_info.h b/include/asm-ia64/thread_info.h
index 93d83cb..366e517 100644
--- a/include/asm-ia64/thread_info.h
+++ b/include/asm-ia64/thread_info.h
@@ -87,7 +87,6 @@ extern void tsk_clear_notify_resume(struct task_struct *tsk);
#define TIF_SYSCALL_TRACE 2 /* syscall trace active */
#define TIF_SYSCALL_AUDIT 3 /* syscall auditing active */
#define TIF_SINGLESTEP 4 /* restore singlestep on return to user mode */
-#define TIF_RESTORE_SIGMASK 5 /* restore signal mask in do_signal() */
#define TIF_NOTIFY_RESUME 6 /* resumption notification requested */
#define TIF_POLLING_NRFLAG 16 /* true if poll_idle() is polling TIF_NEED_RESCHED */
#define TIF_MEMDIE 17
@@ -95,6 +94,7 @@ extern void tsk_clear_notify_resume(struct task_struct *tsk);
#define TIF_DB_DISABLED 19 /* debug trap disabled for fsyscall */
#define TIF_FREEZE 20 /* is freezing for suspend */
#define TIF_RESTORE_RSE 21 /* user RBS is newer than kernel RBS */
+#define TIF_RESTORE_SIGMASK 22 /* restore signal mask in do_signal() */

#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE)
#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)
@@ -112,8 +112,7 @@ extern void tsk_clear_notify_resume(struct task_struct *tsk);

/* "work to do on user-return" bits */
#define TIF_ALLWORK_MASK (_TIF_SIGPENDING|_TIF_NOTIFY_RESUME|_TIF_SYSCALL_AUDIT|\
- _TIF_NEED_RESCHED| _TIF_SYSCALL_TRACE|\
- _TIF_RESTORE_SIGMASK)
+ _TIF_NEED_RESCHED|_TIF_SYSCALL_TRACE)
/* like TIF_ALLWORK_BITS but sans TIF_SYSCALL_TRACE or TIF_SYSCALL_AUDIT */
#define TIF_WORK_MASK (TIF_ALLWORK_MASK&~(_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT))

2008-03-29 00:53:59

by Linus Torvalds

[permalink] [raw]

Subject: Re: [PATCH 2/4] set_restore_sigmask TIF_SIGPENDING

On Fri, 28 Mar 2008, Roland McGrath wrote:
>
> Set TIF_SIGPENDING in set_restore_sigmask. This lets arch code take
> TIF_RESTORE_SIGMASK out of the set of bits that will be noticed on
> return to user mode. On some machines those bits are scarce, and we
> can free this unneeded one up for other uses.

Hmm. That probably means that TIF_RESTORE_SIGMASK shouldn't be a "TIF"
flag at all, but a "TS" ("thread status") flag.

The TS flags are faster, because they are thread-synchronous and do not
need atomic accesses (ie they are purely thread-local in setting, testing
and clearing).

Of course, it may well not be worth it. Unlike the TIF flags, the TS flags
have been architecture-specific and I don't think all architectures even
do them (x86 uses them for FP state bits and stuff like that).

I guess TIF_RESTORE_SIGMASK is never *so* performance-critical that we'd
care about the difference between a single cycle (approx) for a non-atomic
"or" into memory and an atomic bitop (~50 cycles or so).

Linus

2008-03-29 02:25:43

by Roland McGrath

[permalink] [raw]

Subject: Re: [PATCH 2/4] set_restore_sigmask TIF_SIGPENDING

> Hmm. That probably means that TIF_RESTORE_SIGMASK shouldn't be a "TIF"
> flag at all, but a "TS" ("thread status") flag.

Yes. It only ever needs to be set or tested by the thread itself.
It's used in an entirely synchronous fashion and never from
interrupts or anything like that.

I didn't tackle changing this at the same time because of the can of
worms you cited (and because of doing one thing at a time).

It could be a PF_* too, I suppose. There aren't too many of those
bits free, but it would have the advantage of being a place for an
arch that doesn't store any TS_* bits anywhere.

Since acting on the flag is in arch signal code anyway, it makes some
sense to let the arch define how it gets that to happen. I'll send
some follow-on patches that change the conditionals to use #ifdef
HAVE_SET_RESTORE_SIGMASK. Then an arch can define that to provide
its own set_restore_sigmask() instead of TIF_RESTORE_SIGMASK, using a
TS_* flag or whatever it likes. It will make for an easy slow
transition by each arch whenever it decides it cares about the cycles.

> I guess TIF_RESTORE_SIGMASK is never *so* performance-critical that we'd
> care about the difference between a single cycle (approx) for a non-atomic
> "or" into memory and an atomic bitop (~50 cycles or so).

I doubt it is. There is always about to be a whole lot more overhead
for taking siglock and all manner of synchronizing hooey, anyway.

OTOH, my patch here just added a second set_thread_flag there purely
for paranoia (it's probably never needed, but I'd have to be Oleg to
be sure ;-). So even if 50 doesn't much matter, 51 sounds better
than 100.

Thanks,
Roland

2008-03-29 02:52:59

by Linus Torvalds

[permalink] [raw]

Subject: Re: [PATCH 2/4] set_restore_sigmask TIF_SIGPENDING

On Fri, 28 Mar 2008, Roland McGrath wrote:
>
> It could be a PF_* too, I suppose. There aren't too many of those
> bits free, but it would have the advantage of being a place for an
> arch that doesn't store any TS_* bits anywhere.

Yeah, I guess PF_ would be a bit more regular. Maybe we should even try to
avoid the use of TS_ in x86, and turn it into PF_. There are probably bad
historical reasons for the duplication of capabilities.

> Since acting on the flag is in arch signal code anyway, it makes some
> sense to let the arch define how it gets that to happen. I'll send
> some follow-on patches that change the conditionals to use #ifdef
> HAVE_SET_RESTORE_SIGMASK.

Let's see if it matters first. No reason to add another arch-specific
thing if nobody can even measure this thing, and from a quick look it
seems like every RESTORE_SIGMASK user is basically an error path for a
system call. Those few extra cycles really won't be noticeable, we almost
certainly have better things we could use our energy on.

So never mind. I think your series is fine, and my TS_ idea doesn't really
look like it's worth it (and using PF_ sounds a bit more palatable since
we could do it with existing infrastructure, but a quick grep shows that
there's more users of test_thread_flag(TIF_RESTORE_SIGMASK) than I would
have expected (and the *testing* is equally cheap for atomic and thread-
synchronous fields, so that's not a performance issue).

Linus

2008-03-29 03:14:39

by Roland McGrath

[permalink] [raw]

Subject: [PATCH 1/2] HAVE_SET_RESTORE_SIGMASK

Change all the #ifdef TIF_RESTORE_SIGMASK conditionals in non-arch
code to #ifdef HAVE_SET_RESTORE_SIGMASK. If arch code defines it
first, the generic set_restore_sigmask() using TIF_RESTORE_SIGMASK
is not defined.

Signed-off-by: Roland McGrath <[email protected]>
---
fs/compat.c | 8 ++++----
fs/eventpoll.c | 4 ++--
fs/select.c | 8 ++++----
include/linux/sched.h | 2 +-
include/linux/thread_info.h | 10 ++++++++--
5 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/fs/compat.c b/fs/compat.c
index 9964d54..139dc93 100644
--- a/fs/compat.c
+++ b/fs/compat.c
@@ -1634,7 +1634,7 @@ sticky:
return ret;
}

-#ifdef TIF_RESTORE_SIGMASK
+#ifdef HAVE_SET_RESTORE_SIGMASK
asmlinkage long compat_sys_pselect7(int n, compat_ulong_t __user *inp,
compat_ulong_t __user *outp, compat_ulong_t __user *exp,
struct compat_timespec __user *tsp, compat_sigset_t __user *sigmask,
@@ -1825,7 +1825,7 @@ sticky:

return ret;
}
-#endif /* TIF_RESTORE_SIGMASK */
+#endif /* HAVE_SET_RESTORE_SIGMASK */

#if defined(CONFIG_NFSD) || defined(CONFIG_NFSD_MODULE)
/* Stuff for NFS server syscalls... */
@@ -2080,7 +2080,7 @@ long asmlinkage compat_sys_nfsservctl(int cmd, void *notused, void *notused2)

#ifdef CONFIG_EPOLL

-#ifdef TIF_RESTORE_SIGMASK
+#ifdef HAVE_SET_RESTORE_SIGMASK
asmlinkage long compat_sys_epoll_pwait(int epfd,
struct compat_epoll_event __user *events,
int maxevents, int timeout,
@@ -2124,7 +2124,7 @@ asmlinkage long compat_sys_epoll_pwait(int epfd,

return err;
}
-#endif /* TIF_RESTORE_SIGMASK */
+#endif /* HAVE_SET_RESTORE_SIGMASK */

#endif /* CONFIG_EPOLL */

diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 503ffa4..e55451e 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -1262,7 +1262,7 @@ error_return:
return error;
}

-#ifdef TIF_RESTORE_SIGMASK
+#ifdef HAVE_SET_RESTORE_SIGMASK

/*
* Implement the event wait interface for the eventpoll file. It is the kernel
@@ -1308,7 +1308,7 @@ asmlinkage long sys_epoll_pwait(int epfd, struct epoll_event __user *events,
return error;
}

-#endif /* #ifdef TIF_RESTORE_SIGMASK */
+#endif /* HAVE_SET_RESTORE_SIGMASK */

static int __init eventpoll_init(void)
{
diff --git a/fs/select.c b/fs/select.c
index bbd351c..c06fe0b 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -425,7 +425,7 @@ sticky:
return ret;
}

-#ifdef TIF_RESTORE_SIGMASK
+#ifdef HAVE_SET_RESTORE_SIGMASK
asmlinkage long sys_pselect7(int n, fd_set __user *inp, fd_set __user *outp,
fd_set __user *exp, struct timespec __user *tsp,
const sigset_t __user *sigmask, size_t sigsetsize)
@@ -528,7 +528,7 @@ asmlinkage long sys_pselect6(int n, fd_set __user *inp, fd_set __user *outp,

return sys_pselect7(n, inp, outp, exp, tsp, up, sigsetsize);
}
-#endif /* TIF_RESTORE_SIGMASK */
+#endif /* HAVE_SET_RESTORE_SIGMASK */

struct poll_list {
struct poll_list *next;
@@ -759,7 +759,7 @@ asmlinkage long sys_poll(struct pollfd __user *ufds, unsigned int nfds,
return ret;
}

-#ifdef TIF_RESTORE_SIGMASK
+#ifdef HAVE_SET_RESTORE_SIGMASK
asmlinkage long sys_ppoll(struct pollfd __user *ufds, unsigned int nfds,
struct timespec __user *tsp, const sigset_t __user *sigmask,
size_t sigsetsize)
@@ -839,4 +839,4 @@ asmlinkage long sys_ppoll(struct pollfd __user *ufds, unsigned int nfds,

return ret;
}
-#endif /* TIF_RESTORE_SIGMASK */
+#endif /* HAVE_SET_RESTORE_SIGMASK */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6a1e7af..6c275e8 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1144,7 +1144,7 @@ struct task_struct {
struct sighand_struct *sighand;

sigset_t blocked, real_blocked;
- sigset_t saved_sigmask; /* To be restored with TIF_RESTORE_SIGMASK */
+ sigset_t saved_sigmask; /* restored if set_restore_sigmask() was used */
struct sigpending pending;

unsigned long sas_ss_sp;
diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index 4a89477..dbf5daa 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -80,7 +80,13 @@ static inline int test_ti_thread_flag(struct thread_info *ti, int flag)
#define set_need_resched() set_thread_flag(TIF_NEED_RESCHED)
#define clear_need_resched() clear_thread_flag(TIF_NEED_RESCHED)

-#ifdef TIF_RESTORE_SIGMASK
+#if defined TIF_RESTORE_SIGMASK && !defined HAVE_SET_RESTORE_SIGMASK
+/*
+ * An arch can define its own version of set_restore_sigmask() to get the
+ * job done however works, with or without TIF_RESTORE_SIGMASK.
+ */
+#define HAVE_SET_RESTORE_SIGMASK 1
+
/**
* set_restore_sigmask() - make sure saved_sigmask processing gets done
*
@@ -97,7 +103,7 @@ static inline void set_restore_sigmask(void)
set_thread_flag(TIF_RESTORE_SIGMASK);
set_thread_flag(TIF_SIGPENDING);
}
-#endif /* TIF_RESTORE_SIGMASK */
+#endif /* TIF_RESTORE_SIGMASK && !HAVE_SET_RESTORE_SIGMASK */

#endif /* __KERNEL__ */

2008-03-29 03:14:56

by Roland McGrath

[permalink] [raw]

Subject: Re: [PATCH 2/4] set_restore_sigmask TIF_SIGPENDING

> So never mind. [...]

Well fooey on you, 'cause I wrote it already. So I'm a'postin'!

2008-03-29 03:15:30

by Roland McGrath

[permalink] [raw]

Subject: [PATCH 1/2] HAVE_SET_RESTORE_SIGMASK

2008-03-29 03:16:02

by Roland McGrath

[permalink] [raw]

Subject: [PATCH 2/2] x86 TS_RESTORE_SIGMASK

Replace TIF_RESTORE_SIGMASK with TS_RESTORE_SIGMASK and define
our own set_restore_sigmask() function. This saves the costly
SMP-safe set_bit operation, which we do not need for the sigmask
flag since TIF_SIGPENDING always has to be set too.

Signed-off-by: Roland McGrath <[email protected]>
---
arch/x86/ia32/ia32_signal.c | 2 +-
arch/x86/kernel/signal_32.c | 19 ++++++++++---------
arch/x86/kernel/signal_64.c | 16 +++++++++-------
include/asm-x86/thread_info_32.h | 13 +++++++++++--
include/asm-x86/thread_info_64.h | 13 +++++++++++--
5 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
index 101b4b8..b7a531f 100644
--- a/arch/x86/ia32/ia32_signal.c
+++ b/arch/x86/ia32/ia32_signal.c
@@ -128,7 +128,7 @@ asmlinkage long sys32_sigsuspend(int history0, int history1, old_sigset_t mask)

current->state = TASK_INTERRUPTIBLE;
schedule();
- set_thread_flag(TIF_RESTORE_SIGMASK);
+ set_restore_sigmask();
return -ERESTARTNOHAND;
}

diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c
index a06fa55..f8fbc20 100644
--- a/arch/x86/kernel/signal_32.c
+++ b/arch/x86/kernel/signal_32.c
@@ -45,7 +45,7 @@ sys_sigsuspend(int history0, int history1, old_sigset_t mask)

current->state = TASK_INTERRUPTIBLE;
schedule();
- set_thread_flag(TIF_RESTORE_SIGMASK);
+ set_restore_sigmask();
return -ERESTARTNOHAND;
}

@@ -591,7 +591,7 @@ static void do_signal(struct pt_regs *regs)
if (!user_mode(regs))
return;

- if (test_thread_flag(TIF_RESTORE_SIGMASK))
+ if (current_thread_info()->status & TS_RESTORE_SIGMASK)
oldset = &current->saved_sigmask;
else
oldset = &current->blocked;
@@ -608,12 +608,13 @@ static void do_signal(struct pt_regs *regs)

/* Whee! Actually deliver the signal. */
if (handle_signal(signr, &info, &ka, oldset, regs) == 0) {
- /* a signal was successfully delivered; the saved
+ /*
+ * A signal was successfully delivered; the saved
* sigmask will have been stored in the signal frame,
* and will be restored by sigreturn, so we can simply
- * clear the TIF_RESTORE_SIGMASK flag */
- if (test_thread_flag(TIF_RESTORE_SIGMASK))
- clear_thread_flag(TIF_RESTORE_SIGMASK);
+ * clear the TS_RESTORE_SIGMASK flag.
+ */
+ current_thread_info()->status &= ~TS_RESTORE_SIGMASK;
}

return;
@@ -639,8 +640,8 @@ static void do_signal(struct pt_regs *regs)

/* if there's no signal to deliver, we just put the saved sigmask
* back */
- if (test_thread_flag(TIF_RESTORE_SIGMASK)) {
- clear_thread_flag(TIF_RESTORE_SIGMASK);
+ if (current_thread_info()->status & TS_RESTORE_SIGMASK) {
+ current_thread_info()->status &= ~TS_RESTORE_SIGMASK;
sigprocmask(SIG_SETMASK, &current->saved_sigmask, NULL);
}
}
@@ -660,7 +661,7 @@ void do_notify_resume(struct pt_regs *regs, void *_unused,
}

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

if (thread_info_flags & _TIF_HRTICK_RESCHED)
diff --git a/arch/x86/kernel/signal_64.c b/arch/x86/kernel/signal_64.c
index 016413b..fc97dec 100644
--- a/arch/x86/kernel/signal_64.c
+++ b/arch/x86/kernel/signal_64.c
@@ -443,7 +443,7 @@ static void do_signal(struct pt_regs *regs)
if (!user_mode(regs))
return;

- if (test_thread_flag(TIF_RESTORE_SIGMASK))
+ if (current_thread_info()->status & TS_RESTORE_SIGMASK)
oldset = &current->saved_sigmask;
else
oldset = &current->blocked;
@@ -460,11 +460,13 @@ static void do_signal(struct pt_regs *regs)

/* Whee! Actually deliver the signal. */
if (handle_signal(signr, &info, &ka, oldset, regs) == 0) {
- /* a signal was successfully delivered; the saved
+ /*
+ * A signal was successfully delivered; the saved
* sigmask will have been stored in the signal frame,
* and will be restored by sigreturn, so we can simply
- * clear the TIF_RESTORE_SIGMASK flag */
- clear_thread_flag(TIF_RESTORE_SIGMASK);
+ * clear the TS_RESTORE_SIGMASK flag.
+ */
+ current_thread_info()->status &= ~TS_RESTORE_SIGMASK;
}
return;
}
@@ -490,8 +492,8 @@ static void do_signal(struct pt_regs *regs)

/* if there's no signal to deliver, we just put the saved sigmask
back. */
- if (test_thread_flag(TIF_RESTORE_SIGMASK)) {
- clear_thread_flag(TIF_RESTORE_SIGMASK);
+ if (current_thread_info()->status & TS_RESTORE_SIGMASK) {
+ current_thread_info()->status &= ~TS_RESTORE_SIGMASK;
sigprocmask(SIG_SETMASK, &current->saved_sigmask, NULL);
}
}
@@ -517,7 +519,7 @@ do_notify_resume(struct pt_regs *regs, void *unused, __u32 thread_info_flags)
#endif /* CONFIG_X86_MCE */

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

if (thread_info_flags & _TIF_HRTICK_RESCHED)
diff --git a/include/asm-x86/thread_info_32.h b/include/asm-x86/thread_info_32.h
index 5bd5082..4c4fd4e 100644
--- a/include/asm-x86/thread_info_32.h
+++ b/include/asm-x86/thread_info_32.h
@@ -131,7 +131,6 @@ static inline struct thread_info *current_thread_info(void)
#define TIF_SYSCALL_EMU 5 /* syscall emulation active */
#define TIF_SYSCALL_AUDIT 6 /* syscall auditing active */
#define TIF_SECCOMP 7 /* secure computing */
-#define TIF_RESTORE_SIGMASK 8 /* restore signal mask in do_signal() */
#define TIF_HRTICK_RESCHED 9 /* reprogram hrtick timer */
#define TIF_MEMDIE 16
#define TIF_DEBUG 17 /* uses debug registers */
@@ -151,7 +150,6 @@ static inline struct thread_info *current_thread_info(void)
#define _TIF_SYSCALL_EMU (1<<TIF_SYSCALL_EMU)
#define _TIF_SYSCALL_AUDIT (1<<TIF_SYSCALL_AUDIT)
#define _TIF_SECCOMP (1<<TIF_SECCOMP)
-#define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK)
#define _TIF_HRTICK_RESCHED (1<<TIF_HRTICK_RESCHED)
#define _TIF_DEBUG (1<<TIF_DEBUG)
#define _TIF_IO_BITMAP (1<<TIF_IO_BITMAP)
@@ -186,9 +184,20 @@ static inline struct thread_info *current_thread_info(void)
*/
#define TS_USEDFPU 0x0001 /* FPU was used by this task this quantum (SMP) */
#define TS_POLLING 0x0002 /* True if in idle loop and not sleeping */
+#define TS_RESTORE_SIGMASK 0x0004 /* restore signal mask in do_signal() */

#define tsk_is_polling(t) (task_thread_info(t)->status & TS_POLLING)

+#ifndef __ASSEMBLY__
+#define HAVE_SET_RESTORE_SIGMASK 1
+static inline void set_restore_sigmask(void)
+{
+ struct thread_info *ti = current_thread_info();
+ ti->status |= TS_RESTORE_SIGMASK;
+ set_bit(TIF_SIGPENDING, &ti->flags);
+}
+#endif /* !__ASSEMBLY__ */
+
#endif /* __KERNEL__ */

#endif /* _ASM_THREAD_INFO_H */
diff --git a/include/asm-x86/thread_info_64.h b/include/asm-x86/thread_info_64.h
index 6c9b214..ead94d5 100644
--- a/include/asm-x86/thread_info_64.h
+++ b/include/asm-x86/thread_info_64.h
@@ -110,7 +110,6 @@ static inline struct thread_info *stack_thread_info(void)
#define TIF_IRET 5 /* force IRET */
#define TIF_SYSCALL_AUDIT 7 /* syscall auditing active */
#define TIF_SECCOMP 8 /* secure computing */
-#define TIF_RESTORE_SIGMASK 9 /* restore signal mask in do_signal */
#define TIF_MCE_NOTIFY 10 /* notify userspace of an MCE */
#define TIF_HRTICK_RESCHED 11 /* reprogram hrtick timer */
/* 16 free */
@@ -133,7 +132,6 @@ static inline struct thread_info *stack_thread_info(void)
#define _TIF_IRET (1<<TIF_IRET)
#define _TIF_SYSCALL_AUDIT (1<<TIF_SYSCALL_AUDIT)
#define _TIF_SECCOMP (1<<TIF_SECCOMP)
-#define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK)
#define _TIF_MCE_NOTIFY (1<<TIF_MCE_NOTIFY)
#define _TIF_HRTICK_RESCHED (1<<TIF_HRTICK_RESCHED)
#define _TIF_IA32 (1<<TIF_IA32)
@@ -174,9 +172,20 @@ static inline struct thread_info *stack_thread_info(void)
#define TS_USEDFPU 0x0001 /* FPU was used by this task this quantum (SMP) */
#define TS_COMPAT 0x0002 /* 32bit syscall active */
#define TS_POLLING 0x0004 /* true if in idle loop and not sleeping */
+#define TS_RESTORE_SIGMASK 0x0008 /* restore signal mask in do_signal() */

#define tsk_is_polling(t) (task_thread_info(t)->status & TS_POLLING)

+#ifndef __ASSEMBLY__
+#define HAVE_SET_RESTORE_SIGMASK 1
+static inline void set_restore_sigmask(void)
+{
+ struct thread_info *ti = current_thread_info();
+ ti->status |= TS_RESTORE_SIGMASK;
+ set_bit(TIF_SIGPENDING, &ti->flags);
+}
+#endif /* !__ASSEMBLY__ */
+
#endif /* __KERNEL__ */

#endif /* _ASM_THREAD_INFO_H */

2008-03-30 10:53:32

by Paul Mackerras

[permalink] [raw]

Subject: Re: [PATCH 2/4] set_restore_sigmask TIF_SIGPENDING

Linus Torvalds writes:

> Of course, it may well not be worth it. Unlike the TIF flags, the TS flags
> have been architecture-specific and I don't think all architectures even
> do them (x86 uses them for FP state bits and stuff like that).

Powerpc has a "local_flags" field. I don't mind changing it if we are
going to have a consistent name across architectures, but "status" (as
x86 uses) seems a bit nondescript to me.

Paul.

2008-03-31 07:54:21

by Martin Schwidefsky

[permalink] [raw]

Subject: Re: [PATCH 3/4] s390 renumber TIF_RESTORE_SIGMASK

On Fri, 2008-03-28 at 17:14 -0700, Roland McGrath wrote:
> TIF_RESTORE_SIGMASK no longer needs to be in the _TIF_WORK_* masks.
> Those low bits are scarce, and are all used up now.
> Renumber TIF_RESTORE_SIGMASK to free one up.
>
> Signed-off-by: Roland McGrath <[email protected]>

Acked-by: Martin Schwidefsky <[email protected]>

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.

2008-03-31 13:01:33

by Ingo Molnar

[permalink] [raw]

Subject: Re: [PATCH 2/2] x86 TS_RESTORE_SIGMASK

* Roland McGrath <[email protected]> wrote:

> Replace TIF_RESTORE_SIGMASK with TS_RESTORE_SIGMASK and define our own
> set_restore_sigmask() function. This saves the costly SMP-safe
> set_bit operation, which we do not need for the sigmask flag since
> TIF_SIGPENDING always has to be set too.
>
> Signed-off-by: Roland McGrath <[email protected]>
> ---
> arch/x86/ia32/ia32_signal.c | 2 +-
> arch/x86/kernel/signal_32.c | 19 ++++++++++---------
> arch/x86/kernel/signal_64.c | 16 +++++++++-------
> include/asm-x86/thread_info_32.h | 13 +++++++++++--
> include/asm-x86/thread_info_64.h | 13 +++++++++++--
> 5 files changed, 42 insertions(+), 21 deletions(-)

the x86 bits look nice - but i guess this all wants to go into 2.6.26
via -mm (or linux-next), due to its cross-arch nature?

Ingo

2008-03-31 19:30:56

by Roland McGrath

[permalink] [raw]

Subject: Re: [PATCH 2/2] x86 TS_RESTORE_SIGMASK

> the x86 bits look nice - but i guess this all wants to go into 2.6.26
> via -mm (or linux-next), due to its cross-arch nature?

Yes, I would not suggest this change for 2.6.25 at this late date.

Thanks,
Roland