From owner-p4-projects@FreeBSD.ORG Fri Feb 3 18:09:14 2012 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id BE38C1065676; Fri, 3 Feb 2012 18:09:14 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 688201065674 for ; Fri, 3 Feb 2012 18:09:14 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from mail-we0-f182.google.com (mail-we0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id EC80E8FC14 for ; Fri, 3 Feb 2012 18:09:13 +0000 (UTC) Received: by werm13 with SMTP id m13so4601229wer.13 for ; Fri, 03 Feb 2012 10:09:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; 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; bh=oCQ3KDdSRSpdAoMLvik912urKNuB2R/ujA5/xOLKrfI=; b=DPgQWmbLbe/ojBbluvXoBw/5dKv6M8UFXlAwYin3lvVg+wKfoEbA3Qgcrc2MJQV92k NYKh2y738qnbVVfkEHJIskLjN6dmoAbn3AsXAKNaMoVvp03rs9+TKLbx2OZSBDsRbtDF HcaB5tOKtwdFZ4U2h5s1KihBZziPpGNUguo7Q= MIME-Version: 1.0 Received: by 10.216.137.129 with SMTP id y1mr5730620wei.59.1328290991185; Fri, 03 Feb 2012 09:43:11 -0800 (PST) Sender: asmrookie@gmail.com Received: by 10.216.177.73 with HTTP; Fri, 3 Feb 2012 09:43:11 -0800 (PST) In-Reply-To: <201202031550.q13FouDw006719@skunkworks.freebsd.org> References: <201202031550.q13FouDw006719@skunkworks.freebsd.org> Date: Fri, 3 Feb 2012 17:43:11 +0000 X-Google-Sender-Auth: QRmP9oCx_9X2OmREvL9o5Xf6kVA Message-ID: From: Attilio Rao To: John Baldwin Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: Perforce Change Reviews Subject: Re: PERFORCE change 205637 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 03 Feb 2012 18:09:15 -0000 Please note that in case the option is disabled you will get no protection on this paths. Said that, I agree with the mutex change, but I would live the interrupts disabling in the proxy paths, in order to force them also when the option is out. Attilio 2012/2/3 John Baldwin : > http://p4web.freebsd.org/@@205637?ac=3D10 > > Change 205637 by jhb@jhb_jhbbsd on 2012/02/03 15:50:53 > > =C2=A0 =C2=A0 =C2=A0 =C2=A0Revert these hacks as they are superseded by t= he SCHEDULER_STOPPED > =C2=A0 =C2=A0 =C2=A0 =C2=A0stuff in HEAD. > > Affected files ... > > .. //depot/projects/smpng/sys/amd64/amd64/vm_machdep.c#55 edit > .. //depot/projects/smpng/sys/i386/i386/vm_machdep.c#103 edit > .. //depot/projects/smpng/sys/kern/kern_mutex.c#162 edit > > Differences ... > > =3D=3D=3D=3D //depot/projects/smpng/sys/amd64/amd64/vm_machdep.c#55 (text= +ko) =3D=3D=3D=3D > > @@ -515,7 +515,6 @@ > =C2=A0{ > =C2=A0 =C2=A0 =C2=A0 =C2=A0cpuset_t tcrp; > > - =C2=A0 =C2=A0 =C2=A0 disable_intr(); > =C2=A0 =C2=A0 =C2=A0 =C2=A0cpu_reset_proxy_active =3D 1; > =C2=A0 =C2=A0 =C2=A0 =C2=A0while (cpu_reset_proxy_active =3D=3D 1) > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0; =C2=A0 =C2=A0 = =C2=A0 /* Wait for other cpu to see that we've started */ > @@ -534,7 +533,6 @@ > =C2=A0 =C2=A0 =C2=A0 =C2=A0cpuset_t map; > =C2=A0 =C2=A0 =C2=A0 =C2=A0u_int cnt; > > - =C2=A0 =C2=A0 =C2=A0 disable_intr(); > =C2=A0 =C2=A0 =C2=A0 =C2=A0if (smp_active) { > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0map =3D all_cpus; > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0CPU_CLR(PCPU_GET(c= puid), &map); > > =3D=3D=3D=3D //depot/projects/smpng/sys/i386/i386/vm_machdep.c#103 (text+= ko) =3D=3D=3D=3D > > @@ -575,7 +575,6 @@ > =C2=A0{ > =C2=A0 =C2=A0 =C2=A0 =C2=A0cpuset_t tcrp; > > - =C2=A0 =C2=A0 =C2=A0 disable_intr(); > =C2=A0 =C2=A0 =C2=A0 =C2=A0cpu_reset_proxy_active =3D 1; > =C2=A0 =C2=A0 =C2=A0 =C2=A0while (cpu_reset_proxy_active =3D=3D 1) > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0; =C2=A0 =C2=A0 = =C2=A0 /* Wait for other cpu to see that we've started */ > @@ -602,7 +601,6 @@ > =C2=A0 =C2=A0 =C2=A0 =C2=A0cpuset_t map; > =C2=A0 =C2=A0 =C2=A0 =C2=A0u_int cnt; > > - =C2=A0 =C2=A0 =C2=A0 disable_intr(); > =C2=A0 =C2=A0 =C2=A0 =C2=A0if (smp_active) { > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0map =3D all_cpus; > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0CPU_CLR(PCPU_GET(c= puid), &map); > > =3D=3D=3D=3D //depot/projects/smpng/sys/kern/kern_mutex.c#162 (text+ko) = =3D=3D=3D=3D > > @@ -349,15 +349,6 @@ > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return; > =C2=A0 =C2=A0 =C2=A0 =C2=A0} > > - =C2=A0 =C2=A0 =C2=A0 /* > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* If we have already panic'd and this is the= thread that called > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* panic(), then don't block on any mutexes b= ut silently succeed. > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* Otherwise, the kernel will deadlock since = the scheduler isn't > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* going to run the thread that holds the loc= k we need. > - =C2=A0 =C2=A0 =C2=A0 =C2=A0*/ > - =C2=A0 =C2=A0 =C2=A0 if (panicstr !=3D NULL && curthread->td_flags & TD= F_INPANIC) > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return; > - > =C2=A0 =C2=A0 =C2=A0 =C2=A0lock_profile_obtain_lock_failed(&m->lock_objec= t, > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0&con= tested, &waittime); > =C2=A0 =C2=A0 =C2=A0 =C2=A0if (LOCK_LOG_TEST(&m->lock_object, opts)) > @@ -674,15 +665,6 @@ > =C2=A0 =C2=A0 =C2=A0 =C2=A0} > > =C2=A0 =C2=A0 =C2=A0 =C2=A0/* > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* If we failed to unlock this lock and we ar= e a thread that has > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* called panic(), it may be due to the bypas= s in _mtx_lock_sleep() > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* above. =C2=A0In that case, just return and= leave the lock alone to > - =C2=A0 =C2=A0 =C2=A0 =C2=A0* avoid changing the state. > - =C2=A0 =C2=A0 =C2=A0 =C2=A0*/ > - =C2=A0 =C2=A0 =C2=A0 if (panicstr !=3D NULL && curthread->td_flags & TD= F_INPANIC) > - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return; > - > - =C2=A0 =C2=A0 =C2=A0 /* > =C2=A0 =C2=A0 =C2=A0 =C2=A0 * We have to lock the chain before the turnst= ile so this turnstile > =C2=A0 =C2=A0 =C2=A0 =C2=A0 * can be removed from the hash list if it is = empty. > =C2=A0 =C2=A0 =C2=A0 =C2=A0 */ --=20 Peace can only be achieved by understanding - A. Einstein