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);
