Date: Thu, 03 Mar 2005 13:16:32 +0800 From: David Xu <davidxu@freebsd.org> To: David Schultz <das@freebsd.org> Cc: John Baldwin <jhb@freebsd.org> Subject: Re: cvs commit: src/sys/kern kern_sig.c Message-ID: <42269DB0.6070107@freebsd.org> In-Reply-To: <20050303033115.GA13174@VARK.MIT.EDU> References: <200503021343.j22DhpQ3075008@repoman.freebsd.org> <200503020915.28512.jhb@FreeBSD.org> <4226446B.7020406@freebsd.org> <20050303033115.GA13174@VARK.MIT.EDU>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?42269DB0.6070107>