Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 17 Dec 2005 14:54:50 -0500
From:      John Baldwin <jhb@freebsd.org>
To:        Nate Lawson <nate@root.org>
Cc:        cvs-src@freebsd.org, Scott Long <scottl@samsco.org>, src-committers@freebsd.org, cvs-all@freebsd.org
Subject:   Re: cvs commit: src/sys/sys ktr.h src/sys/kern kern_clock.c kern_switch.c
Message-ID:  <200512171454.52514.jhb@freebsd.org>
In-Reply-To: <43A547F2.2090401@root.org>
References:  <43A547F2.2090401@root.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sunday 18 December 2005 06:28 am, Nate Lawson wrote:
> Scott Long wrote:
> > Nate Lawson wrote:
> >>njl         2005-12-17 03:57:10 UTC
> >>
> >>  FreeBSD src repository
> >>
> >>  Modified files:
> >>    sys/sys              ktr.h
> >>    sys/kern             kern_clock.c kern_switch.c
> >>  Log:
> >>  Clean up unused or poorly utilized KTR values.  Remove KTR_FS,
> >> KTR_KGDB, and KTR_IO as they were never used.  Remove KTR_CLK since it
> >> was only used for hardclock firing and use KTR_INTR there instead.=20
> >> Remove KTR_CRITICAL since it was only used for crit enter/exit and use
> >> KTR_CONTENTION instead.
> >>
> >>  Revision  Changes    Path
> >>  1.183     +1 -1      src/sys/kern/kern_clock.c
> >>  1.118     +2 -2      src/sys/kern/kern_switch.c
> >>  1.35      +12 -12    src/sys/sys/ktr.h
> >
> > Um, I was using KTR_CRITICAL for schedgraph.  It was actually quite
> > useful.  Compressing the option space only makes the options less
> > useful.  Surely there has to be a better solution.  Or, at least you
> > could call for comments before you alter this stuff.
>
> You didn't speak up about that in the previous discussion on arch@,
> starting 10/31/2005.  The only comment was jhb@ saying it was not useful
> alone, and he's the only one doing work on critical sections lately.

I didn't say to merge it to KTR_CONTENTION, I said to make it use KTR_SUBSY=
S=20
but have it optional (I just checked the thread, and in my first e-mail I=20
remembered incorrectly) (i.e. put a #if 0 #define KTR_CRITICAL KTR_CONTENTI=
ON=20
#else #define KTR_CRITICAL 0 #endif in kern_switch.c).  Given Scott's recen=
t=20
changes to schedgraph, it would probably be best to just make then under=20
KTR_SCHED, though maybe have it optional, thus:

#if 0
#define KTR_CRITICAL KTR_SCHED
#else
#define KTR_CRITICAL 0
#endif

or something.

> If you can think of another use for this besides one event (enter/exit),
> feel free to add it back.  Or, consider adding KTR_SUBSYS as a one-off
> use like KTR_DEV is for other parts of the system.  KTR_CRITICAL would
> be conditionally defined as KTR_SUBSYS when needed.

That's what I asked you to do and you ignored that part of what I said. :( =
 It=20
can be ok to have a KTR class limited to a small number of events if those=
=20
events fire very often which these do.  I think you can also make KTR_WITNE=
SS=20
optional and use KTR_SUBSYS as well.  (That's also in my original replies.)=
 =20
And I'd still be interested in feedback on my proposal to axe the whole=20
bitmask concept for KTR entirely.

=2D-=20
John Baldwin <jhb@FreeBSD.org> =A0<>< =A0http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve" =A0=3D =A0http://www.FreeBSD.org



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200512171454.52514.jhb>