Re: Updated procstat(1)
On Tue, 27 Nov 2007, Wesley Shields wrote: Here's an updated patch to sys/amd64/amd64/db_trace.c (it's a diff against revision 1.81). It changes register rbp to be register_t rbp and fixes the extra W in TD_IS_SWAPPED. The kernel built fine after these changes. I'll test it out tomorrow. I've gone ahead and applied that change in Perforce, and look forward to hearing back on the testing. I think procstat(1) is getting a lot closer to commitable state for 8-CURRENT, but further feedback would be most welcome (including reports of success on non-i386 architectures, and possibly patches to fix them). For FreeBSD developers with P4 access, you can also check out Thank you for this. I think procstat(1) is going to be very useful. If you can think of other process-inspection related things it could be doing, let me know. The one thing I currently have in mind that I haven't made progress on is dumping the kernel signal state for the process (i.e., what signals have handlers, etc), which may be useful when debugging signal problems for an application. Robert N M Watson Computer Laboratory University of Cambridge ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: Updated procstat(1)
On Wed, 28 Nov 2007, Skip Ford wrote: - -a now means all processes, Thanks. :-) I'm a little surprised. You seemed pretty dedicated to a per-process tool. I was, but then I read your e-mail and became convinced that the first patch that would be submitted against procstat(1) would be a -a patch. :-) I personally would change it to allow either the all flag or a list of pids, rather than at least one of. For pathname, command-line, and credential information, the output will likely not change between showing the pid in the all output and the list output so you're just outputting it twice. If one really wants the same pid to be output multiple times for threads, kstack, or file descriptors, then I'd expect procstat -k 0 0 0 0 0 to be more useful for that. I would think a mistake in usage has been made if a list of pids is specified along with the all flag. But, no real harm is done by doing it the current way. I think your argument is convincing, and have changed it so that only one of -a and a pidlist can be specified. I've also tightened down the syntax checking on flags a bit more. - A new -k has been added, which prints the kernel thread stacks for threads in a process (although not swapped out or actively running threads). This is extremely useful for answering questions of the sort But *why* is the process blocked in UMA. It has both a simple mode (-k_, which lists just kernel function names, and a slightly more detailed mode (-kk), which adds the offset into the function. This is excellent. Does this absolutely have to depend on DDB and KDB? Currently, yes, as stack(9) is conditional on DDB, and the MD bits of stack(9) are defined in db_trace.c (and in some cases, depend on DDB definitions, such as DDB types, although I think not critically so). I've also been pondering breaking out stack(9) from DDB but haven't done that yet. Maybe that will be today's task, as I'd like -k to work without the kernel debugger, as it has use significantly beyond kernel debugging. In sys/amd64/amd64/db_trace.c on line 537, change SWWAPPED to SWAPPED. Fixed, thanks. The newly introducted function stack_save_td() doesn't panic in the MD powerpc code like it does for other arches. I have no idea if this is correct, it just doesn't match the others. Indeed, and I've now fixed this, thanks! Robert N M Watson Computer Laboratory University of Cambridge ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: Updated procstat(1)
Robert Watson wrote: On Wed, 28 Nov 2007, Skip Ford wrote: - -a now means all processes, Thanks. :-) I'm a little surprised. You seemed pretty dedicated to a per-process tool. I was, but then I read your e-mail and became convinced that the first patch that would be submitted against procstat(1) would be a -a patch. :-) Yep, would've happened. Now the first patch submitted will be a -w interval patch... :-) - A new -k has been added, which prints the kernel thread stacks for threads in a process (although not swapped out or actively running threads). This is extremely useful for answering questions of the sort But *why* is the process blocked in UMA. It has both a simple mode (-k_, which lists just kernel function names, and a slightly more detailed mode (-kk), which adds the offset into the function. This is excellent. Does this absolutely have to depend on DDB and KDB? Currently, yes, as stack(9) is conditional on DDB, and the MD bits of stack(9) are defined in db_trace.c (and in some cases, depend on DDB definitions, such as DDB types, although I think not critically so). I've also been pondering breaking out stack(9) from DDB but haven't done that yet. Maybe that will be today's task, as I'd like -k to work without the kernel debugger, as it has use significantly beyond kernel debugging. That'd be great if it worked without DDB. It just feels like it should. This tool is a very nice addition. Thanks for writing it and for asking for feedback, then putting up with the responses. -- Skip ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: Updated procstat(1)
Skip Ford wrote:
Robert Watson wrote:
On Wed, 28 Nov 2007, Skip Ford wrote:
- -a now means all processes,
Thanks. :-) I'm a little surprised. You seemed pretty dedicated to a
per-process tool.
I was, but then I read your e-mail and became convinced that the first
patch that would be submitted against procstat(1) would be a -a patch. :-)
Yep, would've happened. Now the first patch submitted will be a
-w interval patch... :-)
I couldn't resist implementing a crude interval arg just for kicks.
Here's the output of find(1) every second. This is so cool:
$ procstat -k -w 1 948
PIDTID COMM KSTACK
948 100099 find mi_switch thread_suspend_check userret
syscall Xint0x80_syscall
948 100099 find mi_switch sleepq_switch sleepq_wait _sleep
bwait bufwait breadn bread ffs_read VOP_READ_APV ufs_readdir VOP_READDIR_APV
getdirentries syscall Xint0x80_syscall
948 100099 find mi_switch turnstile_wait _mtx_lock_sleep
fdalloc falloc kern_open open syscall Xint0x80_syscall
948 100099 find mi_switch sleepq_switch sleepq_wait _sleep
bwait bufwait breadn bread ffs_read VOP_READ_APV ufs_readdir VOP_READDIR_APV
getdirentries syscall Xint0x80_syscall
948 100099 find mi_switch critical_exit intr_execute_handlers
atpic_handle_intr Xatpic_intr0
948 100099 find mi_switch ast doreti_ast
948 100099 find mi_switch sleepq_switch sleepq_wait _sleep
bwait bufwait breadn bread ffs_read VOP_READ_APV ufs_readdir VOP_READDIR_APV
getdirentries syscall Xint0x80_syscall
948 100099 find mi_switch turnstile_wait _mtx_lock_sleep
fdalloc falloc kern_open open syscall Xint0x80_syscall
948 100099 find mi_switch critical_exit intr_execute_handlers
atpic_handle_intr Xatpic_intr0 priv_check vn_stat vn_statfile kern_fstat fstat
syscall Xint0x80_syscall
948 100099 find mi_switch sleepq_switch sleepq_wait _sleep
bwait bufwait breadn bread ffs_vget ufs_lookup VOP_CACHEDLOOKUP_APV
vfs_cache_lookup VOP_LOOKUP_APV lookup namei kern_lstat lstat syscall
948 100099 find mi_switch sleepq_switch sleepq_wait _sleep
bwait bufwait breadn bread ffs_read VOP_READ_APV ufs_readdir VOP_READDIR_APV
getdirentries syscall Xint0x80_syscall
For someone who isn't intimately familiar with the kernel, this is really
a nice feature for this tool.
I'm attaching the very crude interval patch just because it seems rude
not to, but you probably either don't want it or will do it differently
and either is perfectly fine. If you do implement it on your own, don't
bother printing a header per screenful since it would be most useful for
kstacks and that output usually spills over to multiple lines anyway.
--
Skip
diff -u src~/usr.bin/procstat/procstat.c src/usr.bin/procstat/procstat.c
--- src~/usr.bin/procstat/procstat.c 2007-11-27 10:23:39.0 -0500
+++ src/usr.bin/procstat/procstat.c 2007-11-28 07:01:23.0 -0500
@@ -106,13 +106,14 @@
main(int argc, char *argv[])
{
struct kinfo_proc *kipp;
- int ch, i, name[4], tmp;
+ int ch, i, name[4], tmp, interval;
size_t len;
long l;
pid_t pid;
char *dummy;
- while ((ch = getopt(argc, argv, abcfkhstv)) != -1) {
+ interval = 0;
+ while ((ch = getopt(argc, argv, abcfkhstvw:)) != -1) {
switch (ch) {
case 'a':
aflag = 1;
@@ -150,6 +151,10 @@
vflag = 1;
break;
+ case 'w':
+ interval = atoi(optarg);
+ break;
+
case '?':
default:
usage();
@@ -166,6 +171,7 @@
/* Must specify at least one of -a and a list of pids. */
if (!aflag argc 1)
usage();
+loop:
if (aflag) {
name[0] = CTL_KERN;
name[1] = KERN_PROC;
@@ -233,5 +239,11 @@
/* Suppress header after first process. */
hflag = 1;
}
+
+ if (interval) {
+ sleep(interval);
+ goto loop;
+ }
+
exit(0);
}
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: Updated procstat(1)
On Wed, 28 Nov 2007, Skip Ford wrote: Skip Ford wrote: Robert Watson wrote: On Wed, 28 Nov 2007, Skip Ford wrote: - -a now means all processes, Thanks. :-) I'm a little surprised. You seemed pretty dedicated to a per-process tool. I was, but then I read your e-mail and became convinced that the first patch that would be submitted against procstat(1) would be a -a patch. :-) Yep, would've happened. Now the first patch submitted will be a -w interval patch... :-) I couldn't resist implementing a crude interval arg just for kicks. Here's the output of find(1) every second. This is so cool: Very neat :-). If you like this, you'll love DTrace, which allows you to do all sorts of things along these lines. I'll add a -w mode, but be aware that if you want to do the below, what you really want is DTrace :-), which allows you do do things like sample kernel stack traces on the clock timer, based on function invocations, etc, so you can do things like say sample all the paths to a particular kernel function. Now that John is updating DTrace again, I hope that we'll be seeing it in the 8-CURRENT source RSN. Robert N M Watson Computer Laboratory University of Cambridge ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: Updated procstat(1)
On Nov 28, 2007, at 5:46 AM, Robert Watson wrote: On Wed, 28 Nov 2007, Skip Ford wrote: Skip Ford wrote: Robert Watson wrote: On Wed, 28 Nov 2007, Skip Ford wrote: - -a now means all processes, Thanks. :-) I'm a little surprised. You seemed pretty dedicated to a per-process tool. I was, but then I read your e-mail and became convinced that the first patch that would be submitted against procstat(1) would be a -a patch. :-) Yep, would've happened. Now the first patch submitted will be a - w interval patch... :-) I couldn't resist implementing a crude interval arg just for kicks. Here's the output of find(1) every second. This is so cool: Very neat :-). If you like this, you'll love DTrace, which allows you to do all sorts of things along these lines. I'll add a -w mode, but be aware that if you want to do the below, what you really want is DTrace :-), which allows you do do things like sample kernel stack traces on the clock timer, based on function invocations, etc, so you can do things like say sample all the paths to a particular kernel function. Now that John is updating DTrace again, I hope that we'll be seeing it in the 8-CURRENT source RSN. Robert N M Watson Computer Laboratory University of Cambridge Sorry, just a bit off topic Have the licensing issues been resolved with regards to DTrace? This is a feature I was looking forward to in 7.0-RELEASE but it had been delayed because of the licensing. Bert JW Regeer
Re: Updated procstat(1)
On Wed, 28 Nov 2007, Bert JW Regeer wrote: Have the licensing issues been resolved with regards to DTrace? This is a feature I was looking forward to in 7.0-RELEASE but it had been delayed because of the licensing. The problems had to do with non-alignment of the licensing vs. software boundaries, and I believe have been addressed by moving the boundaries a bit (i.e., making some more DTrace data structures opaque, etc). The key point is that the CDDL parts will be compartmentalized as we do for other licenses, but that DTrace will still be loadable as a module with a GENERIC kernel, as is the case with ZFS already. Unfortunately, DTrace won't ship in 7.0, but we believe that it can be MFC'd to RELENG_7. I've not checked in with John Birrell in a few days, but when I last checked he was in the throes of updating the code and cleaning up the integration of the Solaris parts, so my hope is that we'll see CVS progress soon. I know a lot of people are very eager to see this happen. Robert N M Watson Computer Laboratory University of Cambridge ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: Updated procstat(1)
On Tue, Nov 27, 2007 at 05:18:47PM +, Robert Watson wrote:
The last of these required new kernel changes, including an MD component.
I've tested the MD parts only on i386, although I have quick hacks at what
they should look like on amd64, arm, powerpc, sparc64, sun4v. I don't
promise these compile or work, but they might do.
The kernel build didn't work on AMD64...
cc -c -O2 -frename-registers -pipe -fno-strict-aliasing -std=c99 -g -Wall
-Wredundant-decls -Wnested-externs -Wstrict-prototypes -Wmissing-prototypes
-Wpointer-arith -Winline -Wcast-qual -Wundef -Wno-pointer-sign
-fformat-extensions -nostdinc -I. -I/usr/src/sys -I/usr/src/sys/contrib/altq
-D_KERNEL -DHAVE_KERNEL_OPTION_HEADERS -include opt_global.h -fno-common
-finline-limit=8000 --param inline-unit-growth=100 --param
large-function-growth=1000 -fno-omit-frame-pointer -mcmodel=kernel
-mno-red-zone -mfpmath=387 -mno-sse -mno-sse2 -mno-mmx -mno-3dnow
-msoft-float -fno-asynchronous-unwind-tables -ffreestanding -Werror
/usr/src/sys/amd64/amd64/db_trace.c
cc1: warnings being treated as errors
/usr/src/sys/amd64/amd64/db_trace.c: In function 'stack_save_td':
/usr/src/sys/amd64/amd64/db_trace.c:535: warning: type defaults to 'int' in
declaration of 'rbp'
/usr/src/sys/amd64/amd64/db_trace.c:537: warning: implicit declaration of
function 'TD_IS_SWWAPPED'
/usr/src/sys/amd64/amd64/db_trace.c:537: warning: nested extern declaration of
'TD_IS_SWWAPPED'
*** Error code 1
Stop in /usr/obj/usr/src/sys/GENERIC.
*** Error code 1
Stop in /usr/src.
*** Error code 1
Stop in /usr/src.
[EMAIL PROTECTED] /usr/src %
Here's an updated patch to sys/amd64/amd64/db_trace.c (it's a diff
against revision 1.81). It changes register rbp to be register_t
rbp and fixes the extra W in TD_IS_SWAPPED. The kernel built fine
after these changes. I'll test it out tomorrow.
--- sys/amd64/amd64/db_trace.c.orig 2007-11-15 17:00:56.0 -0500
+++ sys/amd64/amd64/db_trace.c 2007-11-27 22:59:29.0 -0500
@@ -505,15 +505,13 @@
ctx-pcb_rip, count));
}
-void
-stack_save(struct stack *st)
+static void
+stack_capture(struct stack *st, register_t rbp)
{
struct amd64_frame *frame;
vm_offset_t callpc;
- register_t rbp;
stack_zero(st);
- __asm __volatile(movq %%rbp,%0 : =r (rbp));
frame = (struct amd64_frame *)rbp;
while (1) {
if (!INKERNEL((long)frame))
@@ -531,6 +529,29 @@
}
}
+void
+stack_save_td(struct stack *st, struct thread *td)
+{
+ register_t rbp;
+
+ if (TD_IS_SWAPPED(td))
+ panic(stack_save_td: swapped);
+ if (TD_IS_RUNNING(td))
+ panic(stack_save_td: running);
+
+ rbp = td-td_pcb-pcb_rbp;
+ stack_capture(st, rbp);
+}
+
+void
+stack_save(struct stack *st)
+{
+ register_t rbp;
+
+ __asm __volatile(movq %%rbp,%0 : =r (rbp));
+ stack_capture(st, rbp);
+}
+
int
amd64_set_watch(watchnum, watchaddr, size, access, d)
int watchnum;
I think procstat(1) is getting a lot closer to commitable state for
8-CURRENT, but further feedback would be most welcome (including reports of
success on non-i386 architectures, and possibly patches to fix them). For
FreeBSD developers with P4 access, you can also check out
Thank you for this. I think procstat(1) is going to be very useful.
-- WXS
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to [EMAIL PROTECTED]
Re: Updated procstat(1)
Robert Watson wrote: I've updated the procstat(1) kernel patch and userland tool; the updated version can be found at: http://www.watson.org/~robert/freebsd/20071127-procstat.tgz The new version includes a number of changes from the old version, including: - -a now means all processes, Thanks. :-) I'm a little surprised. You seemed pretty dedicated to a per-process tool. I personally would change it to allow either the all flag or a list of pids, rather than at least one of. For pathname, command-line, and credential information, the output will likely not change between showing the pid in the all output and the list output so you're just outputting it twice. If one really wants the same pid to be output multiple times for threads, kstack, or file descriptors, then I'd expect procstat -k 0 0 0 0 0 to be more useful for that. I would think a mistake in usage has been made if a list of pids is specified along with the all flag. But, no real harm is done by doing it the current way. - Threads and processes are now sorted by pid and then tid. If processes are specified manually by pid, they are not sorted, although their threads will be. Nice. - A new -k has been added, which prints the kernel thread stacks for threads in a process (although not swapped out or actively running threads). This is extremely useful for answering questions of the sort But *why* is the process blocked in UMA. It has both a simple mode (-k_, which lists just kernel function names, and a slightly more detailed mode (-kk), which adds the offset into the function. This is excellent. Does this absolutely have to depend on DDB and KDB? The last of these required new kernel changes, including an MD component. I've tested the MD parts only on i386, although I have quick hacks at what they should look like on amd64, arm, powerpc, sparc64, sun4v. I don't promise these compile or work, but they might do. In sys/amd64/amd64/db_trace.c on line 537, change SWWAPPED to SWAPPED. The newly introducted function stack_save_td() doesn't panic in the MD powerpc code like it does for other arches. I have no idea if this is correct, it just doesn't match the others. Unfortunately, I can only test i386. -- Skip ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to [EMAIL PROTECTED]
