On Thu, Nov 04, 2021 at 04:04:43PM -0500, Scott Cheloha wrote:
> During userret() we can take/release the kernel lock up to four times.
> Nobody actually uses SIGPROF or SIGVTALRM, but a double-lock with a
> signal and a temporary signal mask is plausible.
>
> Is there any reason we can't take/release the kernel lock just once?
>
> Do we need to drop the kernel lock in between these blocks to let
> other signals arrive?
Here's a simpler patch.
Index: kern_sig.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_sig.c,v
retrieving revision 1.287
diff -u -p -r1.287 kern_sig.c
--- kern_sig.c 24 Oct 2021 00:02:25 -0000 1.287
+++ kern_sig.c 4 Nov 2021 23:51:00 -0000
@@ -1897,27 +1897,35 @@ filt_signal(struct knote *kn, long hint)
void
userret(struct proc *p)
{
- int signum;
+ int need_lock, signum;
+
+ need_lock = 1;
/* send SIGPROF or SIGVTALRM if their timers interrupted this thread */
if (p->p_flag & P_PROFPEND) {
atomic_clearbits_int(&p->p_flag, P_PROFPEND);
- KERNEL_LOCK();
+ if (need_lock) {
+ need_lock = 0;
+ KERNEL_LOCK();
+ }
psignal(p, SIGPROF);
- KERNEL_UNLOCK();
}
if (p->p_flag & P_ALRMPEND) {
atomic_clearbits_int(&p->p_flag, P_ALRMPEND);
- KERNEL_LOCK();
+ if (need_lock) {
+ need_lock = 0;
+ KERNEL_LOCK();
+ }
psignal(p, SIGVTALRM);
- KERNEL_UNLOCK();
}
if (SIGPENDING(p) != 0) {
- KERNEL_LOCK();
+ if (need_lock) {
+ need_lock = 0;
+ KERNEL_LOCK();
+ }
while ((signum = cursig(p)) != 0)
postsig(p, signum);
- KERNEL_UNLOCK();
}
/*
@@ -1929,12 +1937,16 @@ userret(struct proc *p)
if (p->p_flag & P_SIGSUSPEND) {
atomic_clearbits_int(&p->p_flag, P_SIGSUSPEND);
p->p_sigmask = p->p_oldmask;
-
- KERNEL_LOCK();
+ if (need_lock) {
+ need_lock = 0;
+ KERNEL_LOCK();
+ }
while ((signum = cursig(p)) != 0)
postsig(p, signum);
- KERNEL_UNLOCK();
}
+
+ if (!need_lock)
+ KERNEL_UNLOCK();
if (p->p_flag & P_SUSPSINGLE)
single_thread_check(p, 0);
