From owner-freebsd-current@FreeBSD.ORG Thu Mar 31 16:56:40 2011 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9CC2A1065673 for ; Thu, 31 Mar 2011 16:56:40 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from mail-gy0-f182.google.com (mail-gy0-f182.google.com [209.85.160.182]) by mx1.freebsd.org (Postfix) with ESMTP id 53E1C8FC1A for ; Thu, 31 Mar 2011 16:56:39 +0000 (UTC) Received: by gyg13 with SMTP id 13so1218775gyg.13 for ; Thu, 31 Mar 2011 09:56:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=9157ZKlKSLvEAdt+6q7EmxhUadSnfnA7xkTF23ZbdZk=; b=Q9yajcyUsL7uJynJfVwlnkeYwFRaDy/xy6htVOam0Knj53KAwFklBE7UtR+TsSy3Ik 9Z8SxX67g12VUVb1Yi2Mz7e6Y6tePLkN+qes9cT4XK/s2CzY5RdByBPCTGijmjE0elOp tdR8/QO8YHKMILX3k10mz/piBJoOGwv2i8hP4= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=BYL8vAVdVl9TJ/2TV+3FUWXyFz1pADPQyFjmjxLLoiBb2sReVLD0QStxYOXtznwGn2 m2UvHD/ChExzfmdouwmzG6UP2BRbTm3V6DHgsJxR0va1DzI8ZnFjTwOgvgCdsR1nwbgK 3lJ1ERZXnGcUWMhnzzavyb23JNewAzpGy6OHM= MIME-Version: 1.0 Received: by 10.236.174.105 with SMTP id w69mr4031353yhl.33.1301589271840; Thu, 31 Mar 2011 09:34:31 -0700 (PDT) Sender: asmrookie@gmail.com Received: by 10.236.110.20 with HTTP; Thu, 31 Mar 2011 09:34:31 -0700 (PDT) In-Reply-To: <201103310958.51416.jhb@freebsd.org> References: <201103310958.51416.jhb@freebsd.org> Date: Thu, 31 Mar 2011 12:34:31 -0400 X-Google-Sender-Auth: ZGZk6EqJMnyGKY-jRxUQScmRdjE Message-ID: From: Attilio Rao To: John Baldwin Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: freebsd-current@freebsd.org, Svatopluk Kraus Subject: Re: schedcpu() in /sys/kern/sched_4bsd.c calls thread_lock() on thread with un-initialized td_lock X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 31 Mar 2011 16:56:40 -0000 2011/3/31 John Baldwin : > On Thursday, March 31, 2011 7:32:26 am Svatopluk Kraus wrote: >> Hi, >> >> =C2=A0 I've got a page fault (because of NULL td_lock) in >> thread_lock_flags() called from schedcpu() in /sys/kern/sched_4bsd.c >> file. During process fork, new thread is linked to new process which >> is linked to allproc list and both allproc_lock and new process lock >> are unlocked before sched_fork() is called, where new thread td_lock >> is initialized. Only PRS_NEW process status is on sentry but not >> checked in schedcpu(). > > I think this should fix it: > > Index: sched_4bsd.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- sched_4bsd.c =C2=A0 =C2=A0 =C2=A0 =C2=A0(revision 220190) > +++ sched_4bsd.c =C2=A0 =C2=A0 =C2=A0 =C2=A0(working copy) > @@ -463,6 +463,10 @@ schedcpu(void) > =C2=A0 =C2=A0 =C2=A0 =C2=A0sx_slock(&allproc_lock); > =C2=A0 =C2=A0 =C2=A0 =C2=A0FOREACH_PROC_IN_SYSTEM(p) { > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0PROC_LOCK(p); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (p->p_state =3D=3D = PRS_NEW) { > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 PROC_UNLOCK(p); > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 continue; > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 } > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0FOREACH_THREAD_IN_= PROC(p, td) { > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0awake =3D 0; > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0thread_lock(td); > I don't really think this fix is right because otherwise, when using sched_4bsd anytime we are going to scan the thread list within a proc we need to check for PRS_NEW. We likely need to change the init scheme for the td_lock by having a scheduler primitive setting it and doing that on thread_init() UMA constructor, or similar approach. Attilio --=20 Peace can only be achieved by understanding - A. Einstein