On Wed, Sep 09, 2020 at 08:33:30AM +0200, Martin Pieuchot wrote:
> Per-process data structures needed to suspend the execution of threads
> are since recently protected by the SCHED_LOCK(). So the KERNEL_LOCK()
> dance inside issignal() is no longer necessary and can be removed, ok?
This is not quite right. single_thread_set() still needs the
KERNEL_LOCK() to avoid racing against itself.
> Note that CURSIG() is currently always called with the KERNEL_LOCK()
> held so the code below is redundant.
This is a very weak argument. The goal is to call CURSIG() without the
KERNEL_LOCK() but it is currently obvious that single_thread_set() needs
the KERNEL_LOCK() and needs to be fixed first. Fixing single_thread_set
and single_thread_clear (which I think is actually fine) is still necessary.
While the diff is OK it is mainly so because of side-effects that will be
removed in the future.
> This is a step towards getting signal handling out of ze big lock.
>
> Index: kern/kern_sig.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_sig.c,v
> retrieving revision 1.260
> diff -u -p -r1.260 kern_sig.c
> --- kern/kern_sig.c 26 Aug 2020 03:16:53 -0000 1.260
> +++ kern/kern_sig.c 8 Sep 2020 05:48:51 -0000
> @@ -1203,11 +1203,7 @@ issignal(struct proc *p)
> signum != SIGKILL) {
> pr->ps_xsig = signum;
>
> - if (dolock)
> - KERNEL_LOCK();
> single_thread_set(p, SINGLE_PTRACE, 0);
> - if (dolock)
> - KERNEL_UNLOCK();
>
> if (dolock)
> SCHED_LOCK(s);
> @@ -1215,11 +1211,7 @@ issignal(struct proc *p)
> if (dolock)
> SCHED_UNLOCK(s);
>
> - if (dolock)
> - KERNEL_LOCK();
> single_thread_clear(p, 0);
> - if (dolock)
> - KERNEL_UNLOCK();
>
> /*
> * If we are no longer being traced, or the parent
> @@ -1484,6 +1476,22 @@ sigexit(struct proc *p, int signum)
> }
> exit1(p, 0, signum, EXIT_NORMAL);
> /* NOTREACHED */
> +}
> +
> +/*
> + * Return 1 if `sig', a given signal, is ignored or masked for `p', a given
> + * thread, and 0 otherwise.
> + */
> +int
> +sigismasked(struct proc *p, int sig)
> +{
> + struct process *pr = p->p_p;
> +
> + if ((pr->ps_sigacts->ps_sigignore & sigmask(sig)) ||
> + (p->p_sigmask & sigmask(sig)))
> + return 1;
> +
> + return 0;
> }
>
> int nosuidcoredump = 1;
>
--
:wq Claudio
