Re: [systemd-devel] [PATCH 1/2] bootchart: don't parse /proc/uptime, use CLOCK_BOOTTIME

2014-08-15 Thread Kok, Auke-jan H
thumbs up from me, thanks for sending this.

Auke



On Thu, Jul 31, 2014 at 1:15 AM, Karel Zak k...@redhat.com wrote:

 * systemd-bootchart always parses /proc/uptime, although the
   information is unnecessary when --rel specified

 * use /proc/uptime is overkill, since Linux 2.6.39 we have
   clock_gettime(CLOCK_BOOTTIME, ...). The backend on kernel side is
   get_monotonic_boottime() in both cases.

 * main() uses if (graph_start = 0.0) to detect that /proc is
   available.

   This is fragile solution as graph_start is always smaller than zero
   on all systems after suspend/resume (e.g. laptops), because in this
   case the system uptime includes suspend time and uptime is always
   greater number than monotonic time. For example right now difference
   between uptime and monotonic time is 37 hours on my laptop.

   Note that main() calls log_uptime() (to parse /proc/uptime) for each
   sample when it believes that /proc is not available. So on my laptop
   systemd-boochars spends all live with /proc/uptime parsing +
   nanosleep(), try

 strace  /usr/lib/systemd/systemd-bootchart

   to see the never ending loop.

   This patch uses access(/proc/vmstat, F_OK) to detect procfs.
 ---
  man/systemd-bootchart.xml |  4 +++-
  src/bootchart/bootchart.c | 11 +++
  src/bootchart/store.c | 29 -
  3 files changed, 22 insertions(+), 22 deletions(-)

 diff --git a/man/systemd-bootchart.xml b/man/systemd-bootchart.xml
 index e19bbc1..150ca48 100644
 --- a/man/systemd-bootchart.xml
 +++ b/man/systemd-bootchart.xml
 @@ -131,7 +131,9 @@
  not graph the time elapsed since boot
  and before systemd-bootchart was
  started, as it may result in extremely
 -large graphs.  /para/listitem
 +large graphs. The time elapsed since boot
 +might also include any time that the
 system
 +was suspended./para/listitem
  /varlistentry
  /variablelist
  /refsect1
 diff --git a/src/bootchart/bootchart.c b/src/bootchart/bootchart.c
 index cbfc28d..909ef46 100644
 --- a/src/bootchart/bootchart.c
 +++ b/src/bootchart/bootchart.c
 @@ -310,6 +310,7 @@ int main(int argc, char *argv[]) {
  time_t t = 0;
  int r;
  struct rlimit rlim;
 +bool has_procfs = false;

  parse_conf();

 @@ -349,6 +350,8 @@ int main(int argc, char *argv[]) {

  log_uptime();

 +has_procfs = access(/proc/vmstat, F_OK) == 0;
 +
  LIST_HEAD_INIT(head);

  /* main program loop */
 @@ -385,11 +388,11 @@ int main(int argc, char *argv[]) {
  parse_env_file(/usr/lib/os-release,
 NEWLINE, PRETTY_NAME, build, NULL);
  }

 -/* wait for /proc to become available, discarding samples
 */
 -if (graph_start = 0.0)
 -log_uptime();
 -else
 +if (has_procfs)
  log_sample(samples, sampledata);
 +else
 +/* wait for /proc to become available, discarding
 samples */
 +has_procfs = access(/proc/vmstat, F_OK) == 0;

  sample_stop = gettime_ns();

 diff --git a/src/bootchart/store.c b/src/bootchart/store.c
 index e071983..cedcba8 100644
 --- a/src/bootchart/store.c
 +++ b/src/bootchart/store.c
 @@ -57,27 +57,22 @@ double gettime_ns(void) {
  return (n.tv_sec + (n.tv_nsec / 10.0));
  }

 -void log_uptime(void) {
 -_cleanup_fclose_ FILE *f = NULL;
 -char str[32];
 -double uptime;
 -
 -f = fopen(/proc/uptime, re);
 -
 -if (!f)
 -return;
 -if (!fscanf(f, %s %*s, str))
 -return;
 -
 -uptime = strtod(str, NULL);
 +static double gettime_up(void) {
 +struct timespec n;

 -log_start = gettime_ns();
 +clock_gettime(CLOCK_BOOTTIME, n);
 +return (n.tv_sec + (n.tv_nsec / 10.0));
 +}

 -/* start graph at kernel boot time */
 +void log_uptime(void) {
  if (arg_relative)
 -graph_start = log_start;
 -else
 +graph_start = log_start = gettime_ns();
 +else {
 +double uptime = gettime_up();
 +
 +log_start = gettime_ns();
  graph_start = log_start - uptime;
 +}
  }

  static char *bufgetline(char *buf) {
 --
 1.9.3

 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org

Re: [systemd-devel] [PATCH 1/2] bootchart: don't parse /proc/uptime, use CLOCK_BOOTTIME

2014-08-15 Thread Ronny Chevalier
2014-07-31 10:15 GMT+02:00 Karel Zak k...@redhat.com:
Hi,

 * systemd-bootchart always parses /proc/uptime, although the
   information is unnecessary when --rel specified

 * use /proc/uptime is overkill, since Linux 2.6.39 we have
   clock_gettime(CLOCK_BOOTTIME, ...). The backend on kernel side is
   get_monotonic_boottime() in both cases.

 * main() uses if (graph_start = 0.0) to detect that /proc is
   available.

   This is fragile solution as graph_start is always smaller than zero
   on all systems after suspend/resume (e.g. laptops), because in this
   case the system uptime includes suspend time and uptime is always
   greater number than monotonic time. For example right now difference
   between uptime and monotonic time is 37 hours on my laptop.

   Note that main() calls log_uptime() (to parse /proc/uptime) for each
   sample when it believes that /proc is not available. So on my laptop
   systemd-boochars spends all live with /proc/uptime parsing +
   nanosleep(), try

 strace  /usr/lib/systemd/systemd-bootchart

   to see the never ending loop.

   This patch uses access(/proc/vmstat, F_OK) to detect procfs.
 ---
  man/systemd-bootchart.xml |  4 +++-
  src/bootchart/bootchart.c | 11 +++
  src/bootchart/store.c | 29 -
  3 files changed, 22 insertions(+), 22 deletions(-)

 diff --git a/man/systemd-bootchart.xml b/man/systemd-bootchart.xml
 index e19bbc1..150ca48 100644
 --- a/man/systemd-bootchart.xml
 +++ b/man/systemd-bootchart.xml
 @@ -131,7 +131,9 @@
  not graph the time elapsed since boot
  and before systemd-bootchart was
  started, as it may result in extremely
 -large graphs.  /para/listitem
 +large graphs. The time elapsed since boot
 +might also include any time that the system
 +was suspended./para/listitem
  /varlistentry
  /variablelist
  /refsect1
 diff --git a/src/bootchart/bootchart.c b/src/bootchart/bootchart.c
 index cbfc28d..909ef46 100644
 --- a/src/bootchart/bootchart.c
 +++ b/src/bootchart/bootchart.c
 @@ -310,6 +310,7 @@ int main(int argc, char *argv[]) {
  time_t t = 0;
  int r;
  struct rlimit rlim;
 +bool has_procfs = false;

  parse_conf();

 @@ -349,6 +350,8 @@ int main(int argc, char *argv[]) {

  log_uptime();

 +has_procfs = access(/proc/vmstat, F_OK) == 0;
 +
  LIST_HEAD_INIT(head);

  /* main program loop */
 @@ -385,11 +388,11 @@ int main(int argc, char *argv[]) {
  parse_env_file(/usr/lib/os-release, 
 NEWLINE, PRETTY_NAME, build, NULL);
  }

 -/* wait for /proc to become available, discarding samples */
 -if (graph_start = 0.0)
 -log_uptime();
 -else
 +if (has_procfs)
  log_sample(samples, sampledata);
 +else
 +/* wait for /proc to become available, discarding 
 samples */
 +has_procfs = access(/proc/vmstat, F_OK) == 0;

  sample_stop = gettime_ns();

 diff --git a/src/bootchart/store.c b/src/bootchart/store.c
 index e071983..cedcba8 100644
 --- a/src/bootchart/store.c
 +++ b/src/bootchart/store.c
 @@ -57,27 +57,22 @@ double gettime_ns(void) {
  return (n.tv_sec + (n.tv_nsec / 10.0));
  }

 -void log_uptime(void) {
 -_cleanup_fclose_ FILE *f = NULL;
 -char str[32];
 -double uptime;
 -
 -f = fopen(/proc/uptime, re);
 -
 -if (!f)
 -return;
 -if (!fscanf(f, %s %*s, str))
 -return;
 -
 -uptime = strtod(str, NULL);
 +static double gettime_up(void) {
 +struct timespec n;

 -log_start = gettime_ns();
 +clock_gettime(CLOCK_BOOTTIME, n);
 +return (n.tv_sec + (n.tv_nsec / 10.0));
You can use NSEC_PER_SEC from src/shared/time-util.h instead of 10.0.

 +}

 -/* start graph at kernel boot time */
 +void log_uptime(void) {
  if (arg_relative)
 -graph_start = log_start;
 -else
 +graph_start = log_start = gettime_ns();
 +else {
 +double uptime = gettime_up();
 +
 +log_start = gettime_ns();
  graph_start = log_start - uptime;
 +}
  }

  static char *bufgetline(char *buf) {
 --
 1.9.3

 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org

[systemd-devel] [PATCH 1/2] bootchart: don't parse /proc/uptime, use CLOCK_BOOTTIME

2014-07-31 Thread Karel Zak
* systemd-bootchart always parses /proc/uptime, although the
  information is unnecessary when --rel specified

* use /proc/uptime is overkill, since Linux 2.6.39 we have
  clock_gettime(CLOCK_BOOTTIME, ...). The backend on kernel side is
  get_monotonic_boottime() in both cases.

* main() uses if (graph_start = 0.0) to detect that /proc is
  available.

  This is fragile solution as graph_start is always smaller than zero
  on all systems after suspend/resume (e.g. laptops), because in this
  case the system uptime includes suspend time and uptime is always
  greater number than monotonic time. For example right now difference
  between uptime and monotonic time is 37 hours on my laptop.

  Note that main() calls log_uptime() (to parse /proc/uptime) for each
  sample when it believes that /proc is not available. So on my laptop
  systemd-boochars spends all live with /proc/uptime parsing +
  nanosleep(), try

strace  /usr/lib/systemd/systemd-bootchart

  to see the never ending loop.

  This patch uses access(/proc/vmstat, F_OK) to detect procfs.
---
 man/systemd-bootchart.xml |  4 +++-
 src/bootchart/bootchart.c | 11 +++
 src/bootchart/store.c | 29 -
 3 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/man/systemd-bootchart.xml b/man/systemd-bootchart.xml
index e19bbc1..150ca48 100644
--- a/man/systemd-bootchart.xml
+++ b/man/systemd-bootchart.xml
@@ -131,7 +131,9 @@
 not graph the time elapsed since boot
 and before systemd-bootchart was
 started, as it may result in extremely
-large graphs.  /para/listitem
+large graphs. The time elapsed since boot
+might also include any time that the system
+was suspended./para/listitem
 /varlistentry
 /variablelist
 /refsect1
diff --git a/src/bootchart/bootchart.c b/src/bootchart/bootchart.c
index cbfc28d..909ef46 100644
--- a/src/bootchart/bootchart.c
+++ b/src/bootchart/bootchart.c
@@ -310,6 +310,7 @@ int main(int argc, char *argv[]) {
 time_t t = 0;
 int r;
 struct rlimit rlim;
+bool has_procfs = false;
 
 parse_conf();
 
@@ -349,6 +350,8 @@ int main(int argc, char *argv[]) {
 
 log_uptime();
 
+has_procfs = access(/proc/vmstat, F_OK) == 0;
+
 LIST_HEAD_INIT(head);
 
 /* main program loop */
@@ -385,11 +388,11 @@ int main(int argc, char *argv[]) {
 parse_env_file(/usr/lib/os-release, NEWLINE, 
PRETTY_NAME, build, NULL);
 }
 
-/* wait for /proc to become available, discarding samples */
-if (graph_start = 0.0)
-log_uptime();
-else
+if (has_procfs)
 log_sample(samples, sampledata);
+else
+/* wait for /proc to become available, discarding 
samples */
+has_procfs = access(/proc/vmstat, F_OK) == 0;
 
 sample_stop = gettime_ns();
 
diff --git a/src/bootchart/store.c b/src/bootchart/store.c
index e071983..cedcba8 100644
--- a/src/bootchart/store.c
+++ b/src/bootchart/store.c
@@ -57,27 +57,22 @@ double gettime_ns(void) {
 return (n.tv_sec + (n.tv_nsec / 10.0));
 }
 
-void log_uptime(void) {
-_cleanup_fclose_ FILE *f = NULL;
-char str[32];
-double uptime;
-
-f = fopen(/proc/uptime, re);
-
-if (!f)
-return;
-if (!fscanf(f, %s %*s, str))
-return;
-
-uptime = strtod(str, NULL);
+static double gettime_up(void) {
+struct timespec n;
 
-log_start = gettime_ns();
+clock_gettime(CLOCK_BOOTTIME, n);
+return (n.tv_sec + (n.tv_nsec / 10.0));
+}
 
-/* start graph at kernel boot time */
+void log_uptime(void) {
 if (arg_relative)
-graph_start = log_start;
-else
+graph_start = log_start = gettime_ns();
+else {
+double uptime = gettime_up();
+
+log_start = gettime_ns();
 graph_start = log_start - uptime;
+}
 }
 
 static char *bufgetline(char *buf) {
-- 
1.9.3

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel