svn commit: r230390 - head/sys/conf

[Andreas Tobler]

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