From owner-svn-src-all@FreeBSD.ORG Sat Jan 21 06:50:31 2012 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B0AD3106564A; Sat, 21 Jan 2012 06:50:31 +0000 (UTC) (envelope-from andreast@FreeBSD.org) Received: from smtp.fgznet.ch (mail.fgznet.ch [81.92.96.47]) by mx1.freebsd.org (Postfix) with ESMTP id 52A778FC15; Sat, 21 Jan 2012 06:50:30 +0000 (UTC) Received: from deuterium.andreas.nets (dhclient-91-190-14-19.flashcable.ch [91.190.14.19]) by smtp.fgznet.ch (8.13.8/8.13.8/Submit_SMTPAUTH) with ESMTP id q0L6ixKh022685; Sat, 21 Jan 2012 07:45:00 +0100 (CET) (envelope-from andreast@FreeBSD.org) Message-ID: <4F1A6032.2040900@FreeBSD.org> Date: Sat, 21 Jan 2012 07:50:26 +0100 From: Andreas Tobler User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.5; rv:9.0) Gecko/20111222 Thunderbird/9.0.1 MIME-Version: 1.0 To: Bruce Evans References: <201201201849.q0KInmic054086@svn.freebsd.org> <20120121131914.W1596@besplex.bde.org> In-Reply-To: <20120121131914.W1596@besplex.bde.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.64 on 81.92.96.47 Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r230390 - head/sys/conf X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 21 Jan 2012 06:50:31 -0000 On 21.01.12 03:52, Bruce Evans wrote: > On Fri, 20 Jan 2012, Andreas Tobler wrote: > >> Log: >> Disable GUPROF on archs other than i386/amd64 since the fine details are not >> implemented. > > This was intentionally not done. Just don't use config -pp on arches that > don't suppport it. "profile 2" is already left out of NOTES for all > arches except amd64, i386 and powerpc. But the configuration of "profile" > in the NOTES for these arches is broken anyway. It doesn't exist. Thus > even normal profiling is not tested by NOTES on these arches. I sent this patch to -CURRENT: http://lists.freebsd.org/pipermail/freebsd-current/2012-January/031095.html ...and got no feedback. Is there a better place to send such patches for review? I got positive feedback from marius regarding the sparc64 case. >> Modified: head/sys/conf/kern.pre.mk >> ============================================================================== >> --- head/sys/conf/kern.pre.mk Fri Jan 20 17:25:15 2012 (r230389) >> +++ head/sys/conf/kern.pre.mk Fri Jan 20 18:49:47 2012 (r230390) >> @@ -103,11 +103,14 @@ ASM_CFLAGS= -x assembler-with-cpp -DLOCO >> >> .if defined(PROFLEVEL)&& ${PROFLEVEL}>= 1 >> CFLAGS+= -DGPROF -falign-functions=16 >> +PROF= -pg >> .if ${PROFLEVEL}>= 2 >> CFLAGS+= -DGPROF4 -DGUPROF >> -PROF= -pg -mprofiler-epilogue >> +.if ${MACHINE_CPUARCH} == "i386" || ${MACHINE_CPUARCH} == "amd64" > > Style bug: unsorted tests. Copied from here: sys/conf/kern.post.mk: line 174 > >> +PROF+= -mprofiler-epilogue > > GUPROF is completely broken in amd64 and i386 too: > - -mprofiler-epilogue is broken in gcc-4. It used to be possible to > work around this by building kernels with gcc-3, but I think there > are now some build problems with this. > - certain optimizations break GUPROF: > - -O2 breaks it even with gcc-3 > - optimizations in gcc-4 break it further. Especially, -funit-at-a-time > -finline-functions-called-once. These also break ordinary profiling, > debugging, and stack traces in debuggers and panics. These and -O2 > are now the defaults :-(. > - GUPROF never worked with SMP (except in my version). > > OTOH, using GUPROF with SMP avoids some deadlocks on amd64 and i386. > See MCOUNT_ENTER() and MCOUNT_EXIT() on these arches. These use an > atomic_cmpset() which deadlocks any time a trap occurs in mcount() > (since the trap code is profiled). Tracing through mcount() always > causes such traps. This bug is accidentally avoided by GUPROF, since > it doesn't pretend to support SMP so it doesn't do any cmpset- type > locking. This is fixed in my version by doing more delicate locking > in mcount() for both GUPROF and !GUPROF. All arches need this. > > Other arches mostly only do intr_disable()/restore() in MCOUNT_ENTER()/ > EXIT(). Thus for the SMP case they have common races instead of not-so- > common deadlocks. These arches don't pretend to support SMP any more > than GUPROF does. The exceptions are mips and sparc64. mips has the > cmpsets, and sparc64 has rdpr()/wrpr() which I think are are just > lower-level forms of interrupt disabling (they mask ALL interrupts, > while intr_disable() only masks most interrupts?) Masking of traps > too would prevent deadlocks and avoid the need for cmpsets, but is not > possible. Important traps like NMIs and debugger traps are not > maskable. > >> .else >> -PROF= -pg >> +.error "GUPROF not supported on ${MACHINE_CPUARCH}." > > Style bug: error messages are not terminated with a "." in KNF. > >> +.endif >> .endif >> .endif >> DEFINED_PROF= ${PROF} Do you want me to revert? Thanks for the review. Andreas