cvs commit: src/sys/sys ktr.h src/sys/kern kern_clock.c kern_switch.c

[Nate Lawson]

John Baldwin wrote:
> 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. 
>>>>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_SUBSYS 
> but have it optional (I just checked the thread, and in my first e-mail I 
> remembered incorrectly) (i.e. put a #if 0 #define KTR_CRITICAL KTR_CONTENTION 
> #else #define KTR_CRITICAL 0 #endif in kern_switch.c).  Given Scott's recent 
> changes to schedgraph, it would probably be best to just make then under 
> KTR_SCHED, though maybe have it optional, thus:
> 
> #if 0
> #define KTR_CRITICAL KTR_SCHED
> #else
> #define KTR_CRITICAL 0
> #endif
> 
> or something.

Ok, I committed this.

>>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 
> can be ok to have a KTR class limited to a small number of events if those 
> events fire very often which these do.  I think you can also make KTR_WITNESS 
> optional and use KTR_SUBSYS as well.  (That's also in my original replies.)  
> And I'd still be interested in feedback on my proposal to axe the whole 
> bitmask concept for KTR entirely.

I didn't want to add KTR_SUBSYS or touch KTR_WITNESS when I wasn't sure 
of all the ramifications.  I've got enough free bits now; feel free to 
continue this if you want.

-- 
Nate