From owner-cvs-all@FreeBSD.ORG Thu Mar 3 05:29:38 2005 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id DDE0516A4CE; Thu, 3 Mar 2005 05:29:37 +0000 (GMT) Received: from VARK.MIT.EDU (VARK.MIT.EDU [18.95.3.179]) by mx1.FreeBSD.org (Postfix) with ESMTP id 596A343D1D; Thu, 3 Mar 2005 05:29:37 +0000 (GMT) (envelope-from das@FreeBSD.ORG) Received: from VARK.MIT.EDU (localhost [127.0.0.1]) by VARK.MIT.EDU (8.13.3/8.13.1) with ESMTP id j235T2qw014061; Thu, 3 Mar 2005 00:29:02 -0500 (EST) (envelope-from das@FreeBSD.ORG) Received: (from das@localhost) by VARK.MIT.EDU (8.13.3/8.13.1/Submit) id j235T2b0014060; Thu, 3 Mar 2005 00:29:02 -0500 (EST) (envelope-from das@FreeBSD.ORG) Date: Thu, 3 Mar 2005 00:29:02 -0500 From: David Schultz To: David Xu Message-ID: <20050303052902.GA14011@VARK.MIT.EDU> Mail-Followup-To: David Xu , John Baldwin , src-committers@FreeBSD.ORG, cvs-src@FreeBSD.ORG, cvs-all@FreeBSD.ORG References: <200503021343.j22DhpQ3075008@repoman.freebsd.org> <200503020915.28512.jhb@FreeBSD.org> <4226446B.7020406@freebsd.org> <20050303033115.GA13174@VARK.MIT.EDU> <42269DB0.6070107@freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <42269DB0.6070107@freebsd.org> cc: cvs-src@FreeBSD.ORG cc: src-committers@FreeBSD.ORG cc: cvs-all@FreeBSD.ORG cc: John Baldwin Subject: Re: cvs commit: src/sys/kern kern_sig.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 Mar 2005 05:29:38 -0000 On Thu, Mar 03, 2005, David Xu wrote: > 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. You have to worry about that anyway, though. A and B need to know that they're not allowed to hold locks across the calls if C calls msleep(), for instance. Anyway, your proposal if having a flag for msleep() is basically the same as my proposal of having a separate function. (The only difference is that adding a separate function doesn't break the ABI.) So it sounds like we're more or less in agreement here. > >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. True. If we changed msleep() to disable swapping by default, then we wouldn't have to worry about correctness problems related to missing some.