KTR changes

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

KTR changes

Nate Lawson
I've done a quick audit of the tree and found this number of KTR uses
for each label.

Label           Count
----            -----
KTR_GEN 30
KTR_NET 0
KTR_DEV 1
KTR_LOCK 36
KTR_SMP 55
KTR_FS 0
KTR_PMAP 28
KTR_MALLOC 0
KTR_TRAP 31
KTR_INTR 35
KTR_SIG 21
KTR_CLK 1
KTR_PROC 31
KTR_SYSC 27
KTR_INIT 10
KTR_KGDB 0
KTR_IO 0
KTR_EVH 9
KTR_VFS 19
KTR_VOP 102 (Due to dynamically generated vop files)
KTR_VM 5
KTR_WITNESS 10
KTR_RUNQ 30
KTR_CONTENTION 2
KTR_UMA 2
KTR_CALLOUT 3
KTR_GEOM 23
KTR_BUSDMA 56
KTR_CRITICAL 2
KTR_SCHED 25
KTR_BUF 26

As such, I'd like to mark the following unused and free for allocation:
KTR_NET, KTR_FS, KTR_MALLOC, KTR_KGDB, KTR_IO

These should be merged the following under KTR_MALLOC (KTR_MEM?):
KTR_UMA: uma_zalloc_arg, uma_zfree_arg
KTR_VM: vmspace_alloc, vmspace_free, vm_map_create, vm_map_entry_unlink

Merge under KTR_SYNCH (KTR_LOCK?):
KTR_CRITICAL: critical enter/exit
KTR_CONTENTION: mutex contention start/end

Merge under KTR_TIMER:
KTR_CLK: hard clock firing
KTR_CALLOUT: Giant or mutex-based callout run

Also, it appears that we overran into KTR_CTx space with KTR_UMA
(rwatson).  Is this something that needs to be changed or should we
reduce KTR_CTx?

Last, I'd like to add a new level, KTR_POWER, for use with power
management events.  Since we only have 32 bits of KTR levels, it's
important to use them carefully.  Comments on all this are welcome.

-Nate

_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: KTR changes

John Baldwin

On Oct 31, 2005, at 5:24 PM, Nate Lawson wrote:
> KTR_WITNESS    10

This is isolated to one file and should probably be aliased
to some other bit kind of like how I made KTR_TULIP map to
KTR_DEV when it was enabled and 0 otherwise based on #ifdef's
in if_de.c.  We might should have one generic bit for
subsystems to use similar to how device drivers can use
KTR_DEV.

> As such, I'd like to mark the following unused and free for  
> allocation:
> KTR_NET, KTR_FS, KTR_MALLOC, KTR_KGDB, KTR_IO

I'd rather someone add KTR_MALLOC traces than get rid of
it. :)  If we fleshed out our more general logging for
in-kernel facilities that helps to get a general feel of what
the system is doing when debugging.  I think some traces are
for debugging specific subsystems such as KTR_WITNESS or
KTR_TULIP where as some are more general to tell you what the
kernel is doing (KTR_PROC, KTR_INTR, and KTR_LOCK fall into
this category as probably would KTR_MALLOC and KTR_CONTENTION).
I think subsystem trace classes should be mapped to generic
bits where as kernel-wide tracing should get their own bits if
possible.

> These should be merged the following under KTR_MALLOC (KTR_MEM?):
> KTR_UMA: uma_zalloc_arg, uma_zfree_arg
> KTR_VM: vmspace_alloc, vmspace_free, vm_map_create,  
> vm_map_entry_unlink

Well, I'm not sure about those as the KTR_VM ones aren't related
to kernel malloc at all.  vmspaces are the user virtual address
mappings with vm_map being individual mappings. :)  That's like
saying that mmap() should be part of malloc(). :)  I also think
KTR_UMA is probably useful for getting a general feel for memory
allocation activity in the kernel.
> Merge under KTR_SYNCH (KTR_LOCK?):
> KTR_CRITICAL: critical enter/exit
> KTR_CONTENTION: mutex contention start/end

Perhaps default KTR_CRITICAL to off and let people who want it
map it to a generic bit.  It's very expensive and I'm not sure
it's very useful for all but a few people.  Then I would leave
KTR_CONTENTION alone.

> Merge under KTR_TIMER:
> KTR_CLK: hard clock firing

KTR_CLK is useless, just drop it.  Instead, maybe add KTR_INTR
traces for clock interrupts where they are missing.

> KTR_CALLOUT: Giant or mutex-based callout run

Then you can leave KTR_CALLOUT as is. :)

> Also, it appears that we overran into KTR_CTx space with KTR_UMA  
> (rwatson).  Is this something that needs to be changed or should we  
> reduce KTR_CTx?

I don't think anyone uses KTR_CTx currently?

> Last, I'd like to add a new level, KTR_POWER, for use with power  
> management events.  Since we only have 32 bits of KTR levels, it's  
> important to use them carefully.  Comments on all this are welcome.

Is this for debugging stuff inside ACPI or is it supposed to record
general system-wide activity?

Another option is to dispense with the whole bitmask idea altogether  
and give each compiled in KTR class its own boolean char with an  
associoted sysctl and loader tunable.   You could then allow users to  
specify each class they want to compile in via a separate kernel  
option (KTR_CLASS_FOO) with a special case that KTR_CLASS_ALL would  
#define all of the classes in sys/ktr.h.

--
John Baldwin <[hidden email]>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org

_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: KTR changes

Frank Mayhar-2
On Mon, 2005-10-31 at 20:23 -0500, John Baldwin wrote:
> I don't think anyone uses KTR_CTx currently?

Odd that this should come up at just this time, since I'm just now using
the KTR_CTx set to debug the iir driver.  (This is in reference to my
possibly iir-related panic recently.)

Not that this will ever find its way into the tree, and even if it did
it should probably use KTR_DEV, but it was certainly convenient for my
purposes.
--
Frank Mayhar [hidden email]     http://www.exit.com/
Exit Consulting                 http://www.gpsclock.com/
                                http://www.exit.com/blog/frank/
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: KTR changes

Gleb Smirnoff
In reply to this post by Nate Lawson
On Mon, Oct 31, 2005 at 02:24:27PM -0800, Nate Lawson wrote:
N> As such, I'd like to mark the following unused and free for allocation:
N> KTR_NET, KTR_FS, KTR_MALLOC, KTR_KGDB, KTR_IO

 Please don't touch KTR_NET. Network stack is a big subsystem that should
make a use of ktr(4) someday. I use KTR_NET in my testing patches, when
I work on chasing bugs. May be I will eventually commit parts of them.

--
Totus tuus, Glebius.
GLEBIUS-RIPN GLEB-RIPE
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: KTR changes

Nate Lawson
In reply to this post by John Baldwin
John Baldwin wrote:

>
> On Oct 31, 2005, at 5:24 PM, Nate Lawson wrote:
>
>> KTR_WITNESS    10
>
>
> This is isolated to one file and should probably be aliased
> to some other bit kind of like how I made KTR_TULIP map to
> KTR_DEV when it was enabled and 0 otherwise based on #ifdef's
> in if_de.c.  We might should have one generic bit for
> subsystems to use similar to how device drivers can use
> KTR_DEV.

Sounds good.  KTR_KERN?

>> As such, I'd like to mark the following unused and free for  allocation:
>> KTR_NET, KTR_FS, KTR_MALLOC, KTR_KGDB, KTR_IO
>
> I'd rather someone add KTR_MALLOC traces than get rid of
> it. :)  If we fleshed out our more general logging for
> in-kernel facilities that helps to get a general feel of what
> the system is doing when debugging.  I think some traces are
> for debugging specific subsystems such as KTR_WITNESS or
> KTR_TULIP where as some are more general to tell you what the
> kernel is doing (KTR_PROC, KTR_INTR, and KTR_LOCK fall into
> this category as probably would KTR_MALLOC and KTR_CONTENTION).
> I think subsystem trace classes should be mapped to generic
> bits where as kernel-wide tracing should get their own bits if
> possible.

So since no one defended KTR_FS, KGDB, and IO, I'll reclaim those.  I'm
out of bits and need one for KTR_POWER.

>> These should be merged the following under KTR_MALLOC (KTR_MEM?):
>> KTR_UMA: uma_zalloc_arg, uma_zfree_arg
>> KTR_VM: vmspace_alloc, vmspace_free, vm_map_create,  vm_map_entry_unlink
>
> Well, I'm not sure about those as the KTR_VM ones aren't related
> to kernel malloc at all.  vmspaces are the user virtual address
> mappings with vm_map being individual mappings. :)  That's like
> saying that mmap() should be part of malloc(). :)  I also think
> KTR_UMA is probably useful for getting a general feel for memory
> allocation activity in the kernel.

What I'm trying to achieve is a general "KTR_MEMORY_STUFF" key.  If
you're interested in allocations, getting info about mappings wouldn't
hurt either.

>> Merge under KTR_SYNCH (KTR_LOCK?):
>> KTR_CRITICAL: critical enter/exit
>> KTR_CONTENTION: mutex contention start/end
>
> Perhaps default KTR_CRITICAL to off and let people who want it
> map it to a generic bit.  It's very expensive and I'm not sure
> it's very useful for all but a few people.  Then I would leave
> KTR_CONTENTION alone.

Ok, so KTR_CRITICAL can be reclaimed also.

>> Merge under KTR_TIMER:
>> KTR_CLK: hard clock firing
>
> KTR_CLK is useless, just drop it.  Instead, maybe add KTR_INTR
> traces for clock interrupts where they are missing.

Ok.

> I don't think anyone uses KTR_CTx currently?
>
>> Last, I'd like to add a new level, KTR_POWER, for use with power  
>> management events.  Since we only have 32 bits of KTR levels, it's  
>> important to use them carefully.  Comments on all this are welcome.
>
> Is this for debugging stuff inside ACPI or is it supposed to record
> general system-wide activity?

System-wide power activity.  Devices being powered up or down,
suspend/resume methods, ACPI control of mosfets, etc.

> Another option is to dispense with the whole bitmask idea altogether  
> and give each compiled in KTR class its own boolean char with an  
> associoted sysctl and loader tunable.   You could then allow users to  
> specify each class they want to compile in via a separate kernel  option
> (KTR_CLASS_FOO) with a special case that KTR_CLASS_ALL would  #define
> all of the classes in sys/ktr.h.

I think that would increase the overhead in checking each boolean in a
loop vs. just a bitmask &, right?

--
Nate
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: KTR changes

Nate Lawson
In reply to this post by Gleb Smirnoff
Gleb Smirnoff wrote:
> On Mon, Oct 31, 2005 at 02:24:27PM -0800, Nate Lawson wrote:
> N> As such, I'd like to mark the following unused and free for allocation:
> N> KTR_NET, KTR_FS, KTR_MALLOC, KTR_KGDB, KTR_IO
>
>  Please don't touch KTR_NET. Network stack is a big subsystem that should
> make a use of ktr(4) someday. I use KTR_NET in my testing patches, when
> I work on chasing bugs. May be I will eventually commit parts of them.

Ok, I'll leave that one.  However, I'm very unsympathetic to the general
argument of "leave that for the future" when it's been unused since
version 1.1 in 2000.  It looks like I'll be able to reclaim enough to
leave this one.

--
Nate
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: KTR changes

John Baldwin
In reply to this post by Nate Lawson
On Tuesday 01 November 2005 03:25 pm, Nate Lawson wrote:

> John Baldwin wrote:
> > On Oct 31, 2005, at 5:24 PM, Nate Lawson wrote:
> >> KTR_WITNESS    10
> >
> > This is isolated to one file and should probably be aliased
> > to some other bit kind of like how I made KTR_TULIP map to
> > KTR_DEV when it was enabled and 0 otherwise based on #ifdef's
> > in if_de.c.  We might should have one generic bit for
> > subsystems to use similar to how device drivers can use
> > KTR_DEV.
>
> Sounds good.  KTR_KERN?

Maybe KTR_SUBSYS or something.  It's all in the kernel. :)  If we go with the
non-bitmask route this doesn't matter though.

> > Another option is to dispense with the whole bitmask idea altogether
> > and give each compiled in KTR class its own boolean char with an
> > associoted sysctl and loader tunable.   You could then allow users to
> > specify each class they want to compile in via a separate kernel  option
> > (KTR_CLASS_FOO) with a special case that KTR_CLASS_ALL would  #define
> > all of the classes in sys/ktr.h.
>
> I think that would increase the overhead in checking each boolean in a
> loop vs. just a bitmask &, right?

Huh?  Each check does a bitmask and taht wouldn't change, you'd just be
checking the boolean instead.  Actually, we do two checks, one is a compile
time check, and the other is a runtime check.  Here's the CTR() macro:

#define CTR6(m, format, p1, p2, p3, p4, p5, p6) do { \
        if (KTR_COMPILE & (m)) \
                ktr_tracepoint((m), __FILE__, __LINE__, format, \
                    (u_long)(p1), (u_long)(p2), (u_long)(p3), \
                    (u_long)(p4), (u_long)(p5), (u_long)(p6)); \
        } while(0)

So you have one & test there and basically the same thing in the function
against the run-time mask.  What I'm suggesting would be something like this:

#define CTR6(m, format, p1, p2, p3, p4, p5, p6) do { \
        if (KTR_COMPILED(m)) \
                ktr_tracepoint(KTR_RUNTIME(m), __FILE__, __LINE__, format, \
                    (u_long)(p1), (u_long)(p2), (u_long)(p3), \
                    (u_long)(p4), (u_long)(p5), (u_long)(p6)); \
        } while(0)

Where KTR_COMPILED is a macro that works something like this:

#ifdef KTR_ALL
#define KTR_COMPILED(x) 1
#else
#define KTR_COMPILED(m) m
#endif

Then for each KTR class, we'd have to have something in sys/ktr.h that did:

#ifdef KTR_CLASS_FOO
#define KTR_FOO 1
#else
#define KTR_FOO 0
#endif

(It would be nice if we could hide that in a macro, but I'm not sure cpp will
let us do something like:

#define KTR_CLASS(m) \
#ifdef KTR_CLASS_##m \
#define KTR_##m 1 \
extern int ktr_class_##m; \
#else \
#define KTR_##m 0 \
#endif

)

For the runtime check, you would have:

#define KTR_RUNTIME(m) ktr_class_##m // m has to be FOO though, not KTR_FOO

You can have some more helper macros ala MALLOC_DEFINE() and MALLOC_DECLARE()
for setting up KTR classes to hide as much of this as possible.  This should
give you the general idea though of what could be done.  KTR_DEFINE() would
setup the loader tunable and sysctl if the class was enabled, etc.  This
would get us away from the 32-bit limit with the caveat that you can no
longer have a trace be used for multiple classes.  That is, you can't do:

CTR6(KTR_INTR | KTR_PROC, "blah blah");

If you wanted to do that (I'm not sure we do that every often anyway) you'd
now have to have two different trace events for the different classes.  We
might even be able to store the names of the ktr type in KTR_DEFINE() and
make it available in 'show ktr' in ddb as well.

--
John Baldwin <[hidden email]>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"
Reply | Threaded
Open this post in threaded view
|

Re: KTR changes

Robert N. M. Watson-2
In reply to this post by Nate Lawson

On Tue, 1 Nov 2005, Nate Lawson wrote:

>>> These should be merged the following under KTR_MALLOC (KTR_MEM?):
>>> KTR_UMA: uma_zalloc_arg, uma_zfree_arg
>>> KTR_VM: vmspace_alloc, vmspace_free, vm_map_create,  vm_map_entry_unlink
>>
>> Well, I'm not sure about those as the KTR_VM ones aren't related
>> to kernel malloc at all.  vmspaces are the user virtual address
>> mappings with vm_map being individual mappings. :)  That's like
>> saying that mmap() should be part of malloc(). :)  I also think
>> KTR_UMA is probably useful for getting a general feel for memory
>> allocation activity in the kernel.
>
> What I'm trying to achieve is a general "KTR_MEMORY_STUFF" key.  If
> you're interested in allocations, getting info about mappings wouldn't
> hurt either.

I'm not entirely sure I agree -- I think combining malloc and zone
allocation makes sense, but I think keeping vm tracking separate also
makes sense, as one tends not to be interested in both VM pieces and
explicit kernel allocation simultaneously.  Just like one probably doesn't
want VFS events and device driver operations at the same time -- you can
combine them, but since both at the same time tend not to be desirable...
:-)

Robert N M Watson
_______________________________________________
[hidden email] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "[hidden email]"