On Sun, 2012-05-20 at 13:47 +0300, Eugene V. Lyubimkin wrote: > Hello, > > Thank you for the report. > > On 2012-05-20 03:26, shawn wrote: > [...] > > The starttime column can show inaccurate times if you remount your /proc, as > > recently happened in my system. (well, I'm not sure this is what it is, but > > after > > reading the source, this seems the most likely cause-I am using systemd if > > that means anything) > > This is because the information is based of stat()ing the /proc/$PID > > directories. > > > > Reading into the source, I am wondering what it has to do all these stat() > > and time() calls > > for every process, even if the start column is not active in the main > > display, > > especially when there is a "starttime" field in /proc/$PID/stat documented > > in man 5 proc. > > (which we are already accessing and parsing) > > > > I started working on a patch, but as the starttime given in /proc/$PID/stat > > is relative > > to boot time, I am not sure how else than via /proc/uptime to access it, > > which UptimeMeter.c > > does. It would make sense to cache this lookup, to use the /proc/$PID/stat > > source of > > starttime's on processes. > > Would you like me to wait for the your patch before passing the issue to > upstream?
here is the patch(s), there is no [2/5] that was me removing the autogenerated header files from my git so that I wouldn't have to worry about them. (I guess after applying these to SVN the new autogen ones could be checked in, even though make overrides them). Also, 5/5 is differn't than the one i accidentally didn't forward to bugs.debian.org -- -Shawn Landden -- -Shawn Landden
>From ba2f2267b1fddd2ff8e1a5eeaefaec59a4c127f3 Mon Sep 17 00:00:00 2001 From: Shawn Landden <[email protected]> Date: Sun, 20 May 2012 13:52:27 -0700 Subject: [PATCH 1/5] record btime from /proc/stat on bootup as semi-global --- Process.c | 2 ++ htop.c | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/Process.c b/Process.c index 1ae96e1..cc8305c 100644 --- a/Process.c +++ b/Process.c @@ -177,6 +177,8 @@ char* PROCESS_CLASS = "Process"; #define PROCESS_CLASS NULL #endif +extern long long int btime; /* boot time, gets set in htop.c:record_btime() */ + const char *Process_fieldNames[] = { "", "PID", "Command", "STATE", "PPID", "PGRP", "SESSION", "TTY_NR", "TPGID", "FLAGS", "MINFLT", "CMINFLT", "MAJFLT", "CMAJFLT", diff --git a/htop.c b/htop.c index b9092d5..cb7e9ac 100644 --- a/htop.c +++ b/htop.c @@ -41,6 +41,8 @@ in the source distribution for its full text. #define COPYRIGHT "(C) 2004-2011 Hisham Muhammad" +long long int btime = NULL; /* boot time, gets set in record_btime() */ + static void printVersionFlag() { fputs("htop " VERSION " - " COPYRIGHT "\n" "Released under the GNU GPL.\n\n", @@ -262,6 +264,22 @@ static void IncBuffer_reset(IncBuffer* inc) { inc->buffer[0] = 0; } +static void record_btime() { + FILE* file = fopen(PROCSTATFILE, "r"); + assert(file != NULL); + char buffer[4096]; + do { + char buffer[4096]; + fgets(buffer, 4096, file); + if (strncmp(buffer, "btime ", 6) == 0) { + sscanf(buffer, "btime %lld\n", &btime); + break; + } else if (buffer[0] == "\0") + assert(false); /* got to end of file without getting btime */ + } while(true); + return; +} + int main(int argc, char** argv) { int delay = -1; @@ -350,6 +368,8 @@ int main(int argc, char** argv) { bool doRefresh = true; bool doRecalculate = false; Settings* settings; + + record_btime(); Panel* killPanel = NULL; -- 1.7.9.5
>From 4bd2fa82a84c8b08299fb92d99562949e379b058 Mon Sep 17 00:00:00 2001 From: Shawn Landden <[email protected]> Date: Sun, 20 May 2012 14:47:31 -0700 Subject: [PATCH 3/5] add process->starttime and use it for STARTTIME column this way a remount of /proc will not reset starttimes and we can also see startup times for processes started before the mount of /proc --- Process.c | 13 ++++++++++--- ProcessList.c | 9 ++++++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/Process.c b/Process.c index cc8305c..dcb1d78 100644 --- a/Process.c +++ b/Process.c @@ -111,6 +111,7 @@ typedef struct Process_ { long int priority; long int nice; long int nlwp; + unsigned long long int starttime; char starttime_show[8]; time_t starttime_ctime; #ifdef DEBUG @@ -486,7 +487,13 @@ static void Process_writeField(Process* this, RichString* str, ProcessField fiel } break; } - case STARTTIME: snprintf(buffer, n, "%s", this->starttime_show); break; + case STARTTIME: { + struct tm date; + time_t starttimewall = btime + (this->starttime / sysconf(_SC_CLK_TCK)); + (void) localtime_r(&starttimewall, &date); + strftime(buffer, n, ((starttimewall > time(NULL) - 86400) ? "%R " : "%b%d "), &date); + break; + } #ifdef HAVE_OPENVZ case CTID: snprintf(buffer, n, "%5u ", this->ctid); break; case VPID: snprintf(buffer, n, "%5u ", this->vpid); break; @@ -700,10 +707,10 @@ int Process_compare(const void* v1, const void* v2) { case NLWP: return (p1->nlwp - p2->nlwp); case STARTTIME: { - if (p1->starttime_ctime == p2->starttime_ctime) + if (p1->starttime == p2->starttime) return (p1->pid - p2->pid); else - return (p1->starttime_ctime - p2->starttime_ctime); + return (p1->starttime - p2->starttime); } #ifdef HAVE_OPENVZ case CTID: diff --git a/ProcessList.c b/ProcessList.c index e02e5f8..1cf3516 100644 --- a/ProcessList.c +++ b/ProcessList.c @@ -402,21 +402,24 @@ static bool ProcessList_readStatFile(Process *process, const char* dirname, cons command[commsize] = '\0'; location = end + 2; - int num = sscanf(location, + /* if you change this make sure num == n below matches exp. # of fields */ + int num = sscanf(location, "%c %d %u %u %u " "%d %lu " "%*u %*u %*u %*u " "%llu %llu %llu %llu " "%ld %ld %ld " - "%*d %*u %*u %*d %*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*u " + "%*d %llu " + "%*u %*d %*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*u " "%d %d", &process->state, &process->ppid, &process->pgrp, &process->session, &process->tty_nr, &process->tpgid, &process->flags, &process->utime, &process->stime, &process->cutime, &process->cstime, &process->priority, &process->nice, &process->nlwp, + &process->starttime, &process->exit_signal, &process->processor); fclose(file); - return (num == 16); + return (num == 17); } static bool ProcessList_statProcessDir(Process* process, const char* dirname, char* name) { -- 1.7.9.5
>From 934fa3aed589921b4258551fa750cb4af08f650d Mon Sep 17 00:00:00 2001 From: Shawn Landden <[email protected]> Date: Sun, 20 May 2012 15:02:02 -0700 Subject: [PATCH 4/5] remove some now unneeded code and members of process. I'd like to get rid of the stat() on /proc/$PID altogether...but /proc/$PID/stat doesn't seem to have a uid field (but has a pgrp...) --- Process.c | 2 -- ProcessList.c | 11 ++--------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/Process.c b/Process.c index dcb1d78..853da0b 100644 --- a/Process.c +++ b/Process.c @@ -112,8 +112,6 @@ typedef struct Process_ { long int nice; long int nlwp; unsigned long long int starttime; - char starttime_show[8]; - time_t starttime_ctime; #ifdef DEBUG long int itrealvalue; unsigned long int vsize; diff --git a/ProcessList.c b/ProcessList.c index 1cf3516..b339373 100644 --- a/ProcessList.c +++ b/ProcessList.c @@ -428,17 +428,10 @@ static bool ProcessList_statProcessDir(Process* process, const char* dirname, ch snprintf(filename, MAX_NAME, "%s/%s", dirname, name); struct stat sstat; - int statok = stat(filename, &sstat); - if (statok == -1) + if (stat(filename, &sstat) < 0) return false; process->st_uid = sstat.st_uid; - - struct tm date; - time_t ctime = sstat.st_ctime; - process->starttime_ctime = ctime; - (void) localtime_r((time_t*) &ctime, &date); - strftime(process->starttime_show, 7, ((ctime > time(NULL) - 86400) ? "%R " : "%b%d "), &date); - + return true; } -- 1.7.9.5
>From 05c9a00edafc8d8d2310e3ac781f79c6451a3ca9 Mon Sep 17 00:00:00 2001 From: Shawn Landden <[email protected]> Date: Sun, 20 May 2012 16:56:43 -0700 Subject: [PATCH 5/5] simplify uptime bar by using sysinfo() instead of /proc/uptime --- UptimeMeter.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/UptimeMeter.c b/UptimeMeter.c index d31f911..46345ee 100644 --- a/UptimeMeter.c +++ b/UptimeMeter.c @@ -11,6 +11,7 @@ in the source distribution for its full text. #include "CRT.h" #include <math.h> +#include <sys/sysinfo.h> /*{ #include "Meter.h" @@ -21,13 +22,10 @@ int UptimeMeter_attributes[] = { }; static void UptimeMeter_setValues(Meter* this, char* buffer, int len) { - double uptime = 0; - FILE* fd = fopen(PROCDIR "/uptime", "r"); - if (fd) { - fscanf(fd, "%64lf", &uptime); - fclose(fd); - } - int totalseconds = (int) ceil(uptime); + struct sysinfo si; + if (sysinfo(&si) < 0) + assert(false); + long totalseconds = si.uptime; int seconds = totalseconds % 60; int minutes = (totalseconds/60) % 60; int hours = (totalseconds/3600) % 24; -- 1.7.9.5
