Date: Mon, 18 Jul 2011 12:00:41 -0400 From: John Baldwin <jhb@freebsd.org> To: Alexander Best <arundel@freebsd.org> Cc: current@freebsd.org, Pan Tsu <inyaoo@gmail.com> Subject: Re: [PATCH] Export per-thread resource usage via sysctl Message-ID: <201107181200.41402.jhb@freebsd.org> In-Reply-To: <20110716120458.GA30855@freebsd.org> References: <201107151343.40065.jhb@freebsd.org> <864o2mfpbv.fsf@gmail.com> <20110716120458.GA30855@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Saturday, July 16, 2011 8:04:58 am Alexander Best wrote: > On Sat Jul 16 11, Pan Tsu wrote: > > Alexander Best <arundel@freebsd.org> writes: > > > > > On Fri Jul 15 11, John Baldwin wrote: > > >> This change exports each individual thread's resource usage via sysctl when > > >> individual threads are requested via KERN_PROC_INC_THREAD. This generally > > >> works correctly with 'top -m io' after the previous change to revert top(1) > > >> back to using KERN_PROC_PROC when threads are not enabled. There is one issue > > >> in that top doesn't necessarily DTRT when disabling/enabling threads via 'H' > > >> at runtime while in io mode. I may do some further work to clean that up. > > >> However, for just top run it will now show per-thread stats instead of > > >> duplicating the per-process stats for each thread. > > > > > > i'm not sure, if i understand what the patch is supposed to do. however after > > > applying it, and recompiling/reinstalling the kernel, 'top -mio' displays the > > > same stats for each thread of a process. if i understood you correctly, each > > > thread should have individual stats. > > > > > > i'm running r224068 on amd64 and just reinstalled 'top'. anything i am missing? > > > > FWIW, I see different numbers for a few threads of firefox-bin with top-3.8b1. > > > > http://img233.imageshack.us/img233/1570/81482202.png > > > > Which is an improvement compared to how all threads showed same numbers > > before applying the patch. > > hmmm...not here. i did the following: 'top -mio -b -H -d2 999999' and had a > look at the second output, where if found these lines: Hmm, I never saw that. However, I do need this patch to top to get consistently sane numbers (otherwise top picks one thread as the old thread for all threads causing diffs): Index: machine.c =================================================================== --- machine.c (revision 224062) +++ machine.c (working copy) @@ -235,6 +235,7 @@ static int *pcpu_cpu_states; static int compare_jid(const void *a, const void *b); static int compare_pid(const void *a, const void *b); +static int compare_tid(const void *a, const void *b); static const char *format_nice(const struct kinfo_proc *pp); static void getsysctl(const char *name, void *ptr, size_t len); static int swapmode(int *retavail, int *retfree); @@ -557,7 +558,7 @@ get_old_proc(struct kinfo_proc *pp) * cache it. */ oldpp = bsearch(&pp, previous_pref, previous_proc_count, - sizeof(*previous_pref), compare_pid); + sizeof(*previous_pref), ps.thread ? compare_tid : compare_pid); if (oldpp == NULL) { pp->ki_udata = NOPROC; return (NULL); @@ -652,7 +653,7 @@ get_process_info(struct system_info *si, struct pr previous_pref[i] = &previous_procs[i]; bcopy(pbase, previous_procs, nproc * sizeof(*previous_procs)); qsort(previous_pref, nproc, sizeof(*previous_pref), - compare_pid); + ps.thread ? compare_tid : compare_pid); } previous_proc_count = nproc; @@ -1059,6 +1060,18 @@ compare_pid(const void *p1, const void *p2) return ((*pp1)->ki_pid - (*pp2)->ki_pid); } +static int +compare_tid(const void *p1, const void *p2) +{ + const struct kinfo_proc * const *pp1 = p1; + const struct kinfo_proc * const *pp2 = p2; + + if ((*pp2)->ki_tid < 0 || (*pp1)->ki_tid < 0) + abort(); + + return ((*pp1)->ki_tid - (*pp2)->ki_tid); +} + /* * proc_compare - comparison function for "qsort" * Compares the resource consumption of two processes using five However, using this test app: #include <sys/types.h> #include <err.h> #include <fcntl.h> #include <pthread.h> #include <unistd.h> #include <stdio.h> #define BUFFER_LEN 16384 #define FILE_LEN (BUFFER_LEN * 128) char buffer[BUFFER_LEN]; int fd; static void * work(void *arg) { int mode = (intptr_t)arg; ssize_t result; off_t offset; static const char* names[] = { "idle", "hog", "read", "write" }; pthread_set_name_np(pthread_self(), names[mode]); offset = 0; for (;;) { switch (mode) { case 0: sleep(60); break; case 1: pthread_yield(); break; case 2: case 3: if (mode == 2) result = pread(fd, buffer, sizeof(buffer), offset); else result = pwrite(fd, buffer, sizeof(buffer), offset); if (result < 0) warn("%s", mode == 2 ? "pread" : "pwrite"); offset += sizeof(buffer); if (offset >= FILE_LEN) offset = 0; usleep(1000 * 100); break; } } return (NULL); } int main(int ac, char **av) { pthread_t thread; char template[] = "/tmp/t.XXXXXXXX"; int error, flags; fd = mkstemp(template); if (fd < 0) err(1, "mkstemp"); if (unlink(template) < 0) err(1, "unlink(%s)", template); if (ftruncate(fd, FILE_LEN) < 0) err(1, "ftruncate"); flags = fcntl(fd, F_GETFL); if (flags < 0) err(1, "fcntl(F_GETFL)"); if (fcntl(fd, F_SETFL, flags | O_DIRECT) < 0) err(1, "fcntl(F_SETFL, O_DIRECT)"); printf("PID: %ld\n", (long)getpid()); error = pthread_create(&thread, NULL, work, (void *)0); if (error) errc(1, error, "pthread_create"); /* error = pthread_create(&thread, NULL, work, (void *)1); if (error) errc(1, error, "pthread_create"); */ error = pthread_create(&thread, NULL, work, (void *)2); if (error) errc(1, error, "pthread_create"); (void)work((void *)3); return (0); } I get top output like so with this patch and the above patch applied to the latest top: PID USERNAME VCSW IVCSW READ WRITE FAULT TOTAL PERCENT COMMAND 25003 jhb 20 0 0 6 0 6 75.00% {write} 25003 jhb 21 0 1 0 0 1 12.50% {read} 25003 jhb 0 0 0 0 0 0 0.00% {idle} Without the top patch I get bogus output like so: 25003 jhb 20 0 0 0 0 0 -0.00% {write} 25003 jhb 41 5 34 -75 0 -41 33.33% {read} 25003 jhb -2326 -1 -7 -75 0 -82 66.67% {idle} -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201107181200.41402.jhb>