cvs commit: src/sys/kern kern_sig.c

[David Xu]

David Schultz wrote:

>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.)
>
>  
>
This only partly resolves the problem,  if function A call B, B call C,  
C is unknown to A,
and C does a msleep(),  problem still lhappens.
However, if there needs a flag,  I would like  PNOSWAP for msleep just 
like PCATCH
does.

>The alternative, of course, is to just fix the code that assumes
>that swapping doesn't exist.
>
>  
>
First find all code written in such way, but it is not that easy.

David Xu