KTR_SPAREx

[Konstantin Belousov]

On Tue, Jun 05, 2012 at 02:25:33PM +0200, Dag-Erling Sm??rgrav wrote:
> While working on Capsicum last year, I noticed that some of the spare
> KTR types are (ab)used for different purposes by different parts of the
> code.  KTR_SPARE[234] are all documented as "/* XXX Used by cxgb */",
> but KTR_SPARE3, for instance, is widely used for clock events.  Here is
> a complete list:
> 
> sys/sys/ktr.h: #define  KTR_SPARE2      0x00000800              /* XXX Used by cxgb */
> sys/sys/ktr.h: #define  KTR_SPARE3      0x00008000              /* XXX Used by cxgb */
> sys/sys/ktr.h: #define  KTR_SPARE4      0x00010000              /* XXX Used by cxgb */
> sys/geom/sched/gs_scheduler.h: #define  KTR_GSCHED      KTR_SPARE4
> sys/kern/kern_clocksource.c:    CTR4(KTR_SPARE2, "ipi  at %d:    now  %d.%08x%08x",
> sys/kern/kern_clocksource.c:    CTR4(KTR_SPARE2, "handle at %d:  now  %d.%08x%08x",
> sys/kern/kern_clocksource.c:            CTR2(KTR_SPARE2, "skip   at %d: %d", curcpu, skip);
> sys/kern/kern_clocksource.c:    CTR5(KTR_SPARE2, "next at %d:    next %d.%08x%08x by %d",
> sys/kern/kern_clocksource.c:    CTR4(KTR_SPARE2, "intr at %d:    now  %d.%08x%08x",
> sys/kern/kern_clocksource.c:                    CTR5(KTR_SPARE2, "load p at %d:   now %d.%08x first in %d.%08x",
> sys/kern/kern_clocksource.c:            CTR5(KTR_SPARE2, "load at %d:    next %d.%08x%08x eq %d",
> sys/kern/kern_clocksource.c:    CTR4(KTR_SPARE2, "idle at %d:    now  %d.%08x%08x",
> sys/kern/kern_clocksource.c:    CTR4(KTR_SPARE2, "active at %d:  now  %d.%08x%08x",
> sys/kern/kern_clocksource.c:    CTR4(KTR_SPARE2, "set_cyc at %d:  now  %d.%08x%08x",
> sys/kern/kern_clocksource.c:    CTR4(KTR_SPARE2, "set_cyc at %d:  t  %d.%08x%08x",
> sys/kern/kern_clocksource.c:    CTR3(KTR_SPARE2, "new co at %d:    on %d in %d",
> sys/amd64/amd64/machdep.c:      CTR2(KTR_SPARE2, "cpu_idle(%d) at %d",
> sys/amd64/amd64/machdep.c:      CTR2(KTR_SPARE2, "cpu_idle(%d) at %d done",
> sys/dev/cxgb/cxgb_osdep.h: #define      KTR_CXGB        KTR_SPARE2
> sys/dev/cxgb/ulp/iw_cxgb/iw_cxgb_hal.h: #define KTR_IW_CXGB KTR_SPARE4
> sys/dev/cxgb/ulp/tom/cxgb_defs.h: #define       KTR_TOM KTR_SPARE2
> sys/dev/cxgb/ulp/tom/cxgb_defs.h: #define       KTR_TCB KTR_SPARE3
> sys/dev/cxgb/ulp/tom/cxgb_cpl_io.c:     CTR2(KTR_SPARE2, "wr_ack: snd_una=%u credits=%d", snd_una, credits);
> sys/dev/cxgb/ulp/tom/cxgb_cpl_io.c:             CTR1(KTR_SPARE2, "wr_ack: sbdrop(%d)", bytes);
> sys/dev/gem/if_gem.c: #define   KTR_GEM         KTR_SPARE2
> sys/dev/drm2/drmP.h: #define    KTR_DRM_REG     KTR_SPARE3
> sys/dev/hme/if_hme.c: #define   KTR_HME         KTR_SPARE2      /* XXX */
> sys/dev/cas/if_cas.c: #define   KTR_CAS         KTR_SPARE2
> sys/dev/ath/if_ath.c: #define   ATH_KTR_INTR    KTR_SPARE4
> sys/dev/ath/if_ath.c: #define   ATH_KTR_ERR     KTR_SPARE3
> sys/dev/ath/if_ath_rx.c: #define        ATH_KTR_INTR    KTR_SPARE4
> sys/dev/ath/if_ath_rx.c: #define        ATH_KTR_ERR     KTR_SPARE3
> sys/i386/xen/xen_machdep.c:     CTR0(KTR_SPARE2, "ni_cli disabling interrupts");
> sys/i386/xen/xen_machdep.c:     CTR2(KTR_SPARE2, "%x xen_restore_flags eflags %x", rebp(), eflags);
> sys/i386/xen/xen_machdep.c:     CTR1(KTR_SPARE2, "%x xen_cli disabling interrupts", rebp());
> sys/i386/xen/xen_machdep.c:     CTR1(KTR_SPARE2, "%x xen_sti enabling interrupts", rebp());
> sys/i386/i386/machdep.c:        CTR2(KTR_SPARE2, "cpu_idle(%d) at %d",
> sys/i386/i386/machdep.c:        CTR2(KTR_SPARE2, "cpu_idle(%d) at %d done",
> sys/powerpc/powerpc/cpu.c:      CTR2(KTR_SPARE2, "cpu_idle(%d) at %d",
> sys/powerpc/powerpc/cpu.c:      CTR2(KTR_SPARE2, "cpu_idle(%d) at %d done",
> sys/pc98/pc98/machdep.c:        CTR2(KTR_SPARE2, "cpu_idle(%d) at %d",
> sys/pc98/pc98/machdep.c:        CTR2(KTR_SPARE2, "cpu_idle(%d) at %d done",
> sys/sparc64/sparc64/pmap.c:             CTR5(KTR_SPARE2,
> sys/sparc64/sparc64/tsb.c:              CTR5(KTR_SPARE2,
> sys/sparc64/include/bus.h: #define      KTR_BUS                         KTR_SPARE2
> 
> Most of this is in device drivers, which should use KTR_DEV.  There is
> one major use of KTR_SPAREx in common code: KTR_SPARE2 is used for clock
> events.  It is also used incorrectly by the sparc64 pmap core (there is
> a separate KTR_PMAP for that).
> 
> I suggest that we
> 
> 1) rename one of the spare KTRs to KTR_CLOCK and use that for clock
>    events.  I already have a patch for that.
> 
> 2) eliminate all other use of KTR_SPARE[0-9] in non-device code.  I
>    think the existing KTRs should already cover most cases.
> 
> 3) modify device drivers to use KTR_DEV for events that aren't covered
>    by existing, more specific KTRs, which is almost none.  For instance,
>    there is no reason why cxgb shouldn't just use KTR_NET.
Moving all device drivers to KTR_DEV makes the KTR unusable for device
driver debugging. When looking at the drm2 and gem traces, I do not want
to see other devices tracepoints. Amount of data from GEM is huge, and
obfuscating it with unrelated debugging recycles the ktr ring faster, aside
of making noise that cayses log to be meaningless.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-arch/attachments/20120605/31a5e90a/attachment.pgp