cvs commit: src/sys/kern kern_sig.c

[David Schultz]

On Thu, Mar 03, 2005, David Xu wrote:
> John Baldwin wrote:
> 
> >On Wednesday 02 March 2005 08:43 am, David Xu wrote:
> > 
> >
> >>davidxu     2005-03-02 13:43:51 UTC
> >>
> >> FreeBSD src repository
> >>
> >> Modified files:
> >>   sys/kern             kern_sig.c
> >> Log:
> >> In kern_sigtimedwait, remove waitset bits for td_sigmask before
> >> sleeping, so in do_tdsignal, we no longer need to test td_waitset.
> >> now td_waitset is only used to give a thread higher priority when
> >> delivering signal to multithreads process.
> >> This also fixes a bug:
> >> when a thread in sigwait states was suspended and later resumed
> >> by SIGCONT, it can no longer receive signals belong to waitset.
> >>   
> >>
> >
> >Is this related at all to Peter Holm's panic where sigwait() + swapping 
> >invokes a panic?
> >
> > 
> >
> No. Peter Holm's found is a swapping problem. vm swaps out sleeping
> thread's stack under memory stressing case. but I think that's not safe,
> that means, following code can not be used in kernel:
> 
> int *p;
> 
> func()
> {
>    int n;
> 
>    n = 0;
>    p = &n;
>    msleep(p);
>    /* check variable n ...
> }
> 
> func2()
> {
>   *p = 2;
>   wakeup(p);
> }
> 
> unless million lines of kernel code are reviewed, I don't think the
> vm code is safe. The following patch should avoid the problem:
[...]

KSE already mostly broke swapping, so I'm not sure we need to
break it some more.  I think a better fix would be to mark threads
as unswappable in msleep() and cv_wait().  There would probably
need to be a separate msleep_swapok() for places where swapping
the process out is okay.  (IIRC, Solaris has something like this,
but they use it because their cv_wait() works with locks held, and
so the swapok variant is for situations where no locks are held.)

The alternative, of course, is to just fix the code that assumes
that swapping doesn't exist.