On Fri, Nov 05, 2021 at 03:15:26PM +0100, Christian Ludwig wrote: > > comments inline. > > [...] > > Unlocking if (!need_lock) below looks odd. I think it would make more > sense to reverse the logic: > > have_lock = 0; > > if (flags) { > if (!have_lock) { > have_lock = 1; > KERNEL_LOCK() > } > } > > if (have_lock) > KERNEL_UNLOCK();
... sure, but at that point we may as well use the more idiomatic adjective "locked". Updated patch attached. Do you (or does anyone else) know whether there is a technical reason for having discrete kernel lock sections here? I am running with this now without apparent issue. 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 6 Nov 2021 19:05:58 -0000 @@ -1897,27 +1897,35 @@ filt_signal(struct knote *kn, long hint) void userret(struct proc *p) { - int signum; + int locked, signum; + + locked = 0; /* 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 (!locked) { + locked = 1; + KERNEL_LOCK(); + } psignal(p, SIGPROF); - KERNEL_UNLOCK(); } if (p->p_flag & P_ALRMPEND) { atomic_clearbits_int(&p->p_flag, P_ALRMPEND); - KERNEL_LOCK(); + if (!locked) { + locked = 1; + KERNEL_LOCK(); + } psignal(p, SIGVTALRM); - KERNEL_UNLOCK(); } if (SIGPENDING(p) != 0) { - KERNEL_LOCK(); + if (!locked) { + locked = 1; + 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 (!locked) { + locked = 1; + KERNEL_LOCK(); + } while ((signum = cursig(p)) != 0) postsig(p, signum); - KERNEL_UNLOCK(); } + + if (locked) + KERNEL_UNLOCK(); if (p->p_flag & P_SUSPSINGLE) single_thread_check(p, 0);