Re: [systemd-devel] [PATCH] cgtop: add options to format memory, IO usage in raw bytes

2015-05-27 Thread Zbigniew Jędrzejewski-Szmek 
On Fri, May 22, 2015 at 05:44:47PM -0500, Charles Duffy wrote:
> From: Charles Duffy 
> 
> ---
>  src/cgtop/cgtop.c | 64 
> ---
>  1 file changed, 51 insertions(+), 13 deletions(-)
> 
> diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c
> index a390cf3..45c8d6f 100644
> --- a/src/cgtop/cgtop.c
> +++ b/src/cgtop/cgtop.c
> @@ -77,6 +77,15 @@ static enum {
>  CPU_TIME,
>  } arg_cpu_type = CPU_PERCENT;
>  
> +enum ByteRepresentation {
> +BYTES_HUMAN,
> +BYTES_RAW,
> +};
> +typedef enum ByteRepresentation ByteRepresentation;
> +
> +static ByteRepresentation arg_memory_type = BYTES_HUMAN;
> +static ByteRepresentation arg_io_type = BYTES_HUMAN;
> +
>  static void group_free(Group *g) {
>  assert(g);
>  
> @@ -96,6 +105,16 @@ static void group_hashmap_free(Hashmap *h) {
>  hashmap_free(h);
>  }
>  
> +static char *cond_format_bytes(char *buf, size_t l, off_t t, 
> ByteRepresentation r, bool is_valid) {
This should return const char *.

> +if (!is_valid)
> +return (char*)"   -";
Width formatting is already done in the caller of this function.
So just "-" should be enough.

> +if (r == BYTES_HUMAN)
> +return format_bytes(buf, l, t);
> +snprintf(buf, l, "%lu8", t);
What is "8" doing in the format? Also off_t is signed. I think "%zd" is
the right format.

> +buf[l-1] = 0;
snprintf terminates with NUL on its own, always, so this line is not needed.

Zbyszek

> +return buf;
> +}
> +
>  static int process(const char *controller, const char *path, Hashmap *a, 
> Hashmap *b, unsigned iteration) {
>  Group *g;
>  int r;
> @@ -532,18 +551,9 @@ static int display(Hashmap *a) {
>  } else
>  printf(" %*s", maxtcpu, format_timespan(buffer, 
> sizeof(buffer), (nsec_t) (g->cpu_usage / NSEC_PER_USEC), 0));
>  
> -if (g->memory_valid)
> -printf(" %8s", format_bytes(buffer, sizeof(buffer), 
> g->memory));
> -else
> -fputs("-", stdout);
> -
> -if (g->io_valid) {
> -printf(" %8s",
> -   format_bytes(buffer, sizeof(buffer), 
> g->io_input_bps));
> -printf(" %8s",
> -   format_bytes(buffer, sizeof(buffer), 
> g->io_output_bps));
> -} else
> -fputs("--", stdout);
> +printf(" %8s", cond_format_bytes(buffer, sizeof(buffer), 
> g->memory, arg_memory_type, g->memory_valid));
> +printf(" %8s", cond_format_bytes(buffer, sizeof(buffer), 
> g->io_input_bps, arg_io_type, g->io_valid));
> +printf(" %8s", cond_format_bytes(buffer, sizeof(buffer), 
> g->io_output_bps, arg_io_type, g->io_valid));
>  
>  putchar('\n');
>  }
> @@ -562,6 +572,8 @@ static void help(void) {
> "  -m  Order by memory load\n"
> "  -i  Order by IO load\n"
> " --cpu[=TYPE] Show CPU usage as time or percentage 
> (default)\n"
> +   " --memory[=TYPE]  Show memory usage as bytes or human 
> (default)\n"
> +   " --io[=TYPE]  Show memory usage as bytes or human 
> (default)\n"
So, I know that Umut suggested this, but I'm not sure that this is a good
idea. When would you want different units for memory and io? One switch
should be enough.

Zbyszek

> "  -d --delay=DELAYDelay between updates\n"
> "  -n --iterations=N   Run for N iterations before exiting\n"
> "  -b --batch  Run in batch mode, accepting no 
> input\n"
> @@ -574,7 +586,9 @@ static int parse_argv(int argc, char *argv[]) {
>  enum {
>  ARG_VERSION = 0x100,
>  ARG_DEPTH,
> -ARG_CPU_TYPE
> +ARG_CPU_TYPE,
> +ARG_MEM_TYPE,
> +ARG_IO_TYPE,
>  };
>  
>  static const struct option options[] = {
> @@ -585,6 +599,8 @@ static int parse_argv(int argc, char *argv[]) {
>  { "batch",  no_argument,   NULL, 'b' },
>  { "depth",  required_argument, NULL, ARG_DEPTH   },
>  { "cpu",optional_argument, NULL, ARG_CPU_TYPE},
> +{ "memory", optional_argument, NULL, ARG_MEM_TYPE},
> +{ "io", optional_argument, NULL, ARG_IO_TYPE },
>  {}
>  };
>  
> @@ -618,6 +634,28 @@ static int parse_argv(int argc, char *argv[]) {
>  }
>  break;
>  
> +case ARG_MEM_TYPE:
> +if (optarg) {
> +if (strcmp(optarg, "human") == 0)
> +

Re: [systemd-devel] [PATCH] cgtop: add options to format memory, IO usage in raw bytes

2015-05-27 Thread systemd github import bot 
Patchset imported to github.
Pull request:


--
Generated by https://github.com/haraldh/mail2git
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] cgtop: add options to format memory, IO usage in raw bytes

2015-05-27 Thread Charles Duffy 
From: Charles Duffy 

---
 src/cgtop/cgtop.c | 64 ---
 1 file changed, 51 insertions(+), 13 deletions(-)

diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c
index a390cf3..45c8d6f 100644
--- a/src/cgtop/cgtop.c
+++ b/src/cgtop/cgtop.c
@@ -77,6 +77,15 @@ static enum {
 CPU_TIME,
 } arg_cpu_type = CPU_PERCENT;
 
+enum ByteRepresentation {
+BYTES_HUMAN,
+BYTES_RAW,
+};
+typedef enum ByteRepresentation ByteRepresentation;
+
+static ByteRepresentation arg_memory_type = BYTES_HUMAN;
+static ByteRepresentation arg_io_type = BYTES_HUMAN;
+
 static void group_free(Group *g) {
 assert(g);
 
@@ -96,6 +105,16 @@ static void group_hashmap_free(Hashmap *h) {
 hashmap_free(h);
 }
 
+static char *cond_format_bytes(char *buf, size_t l, off_t t, 
ByteRepresentation r, bool is_valid) {
+if (!is_valid)
+return (char*)"   -";
+if (r == BYTES_HUMAN)
+return format_bytes(buf, l, t);
+snprintf(buf, l, "%lu8", t);
+buf[l-1] = 0;
+return buf;
+}
+
 static int process(const char *controller, const char *path, Hashmap *a, 
Hashmap *b, unsigned iteration) {
 Group *g;
 int r;
@@ -532,18 +551,9 @@ static int display(Hashmap *a) {
 } else
 printf(" %*s", maxtcpu, format_timespan(buffer, 
sizeof(buffer), (nsec_t) (g->cpu_usage / NSEC_PER_USEC), 0));
 
-if (g->memory_valid)
-printf(" %8s", format_bytes(buffer, sizeof(buffer), 
g->memory));
-else
-fputs("-", stdout);
-
-if (g->io_valid) {
-printf(" %8s",
-   format_bytes(buffer, sizeof(buffer), 
g->io_input_bps));
-printf(" %8s",
-   format_bytes(buffer, sizeof(buffer), 
g->io_output_bps));
-} else
-fputs("--", stdout);
+printf(" %8s", cond_format_bytes(buffer, sizeof(buffer), 
g->memory, arg_memory_type, g->memory_valid));
+printf(" %8s", cond_format_bytes(buffer, sizeof(buffer), 
g->io_input_bps, arg_io_type, g->io_valid));
+printf(" %8s", cond_format_bytes(buffer, sizeof(buffer), 
g->io_output_bps, arg_io_type, g->io_valid));
 
 putchar('\n');
 }
@@ -562,6 +572,8 @@ static void help(void) {
"  -m  Order by memory load\n"
"  -i  Order by IO load\n"
" --cpu[=TYPE] Show CPU usage as time or percentage 
(default)\n"
+   " --memory[=TYPE]  Show memory usage as bytes or human 
(default)\n"
+   " --io[=TYPE]  Show memory usage as bytes or human 
(default)\n"
"  -d --delay=DELAYDelay between updates\n"
"  -n --iterations=N   Run for N iterations before exiting\n"
"  -b --batch  Run in batch mode, accepting no input\n"
@@ -574,7 +586,9 @@ static int parse_argv(int argc, char *argv[]) {
 enum {
 ARG_VERSION = 0x100,
 ARG_DEPTH,
-ARG_CPU_TYPE
+ARG_CPU_TYPE,
+ARG_MEM_TYPE,
+ARG_IO_TYPE,
 };
 
 static const struct option options[] = {
@@ -585,6 +599,8 @@ static int parse_argv(int argc, char *argv[]) {
 { "batch",  no_argument,   NULL, 'b' },
 { "depth",  required_argument, NULL, ARG_DEPTH   },
 { "cpu",optional_argument, NULL, ARG_CPU_TYPE},
+{ "memory", optional_argument, NULL, ARG_MEM_TYPE},
+{ "io", optional_argument, NULL, ARG_IO_TYPE },
 {}
 };
 
@@ -618,6 +634,28 @@ static int parse_argv(int argc, char *argv[]) {
 }
 break;
 
+case ARG_MEM_TYPE:
+if (optarg) {
+if (strcmp(optarg, "human") == 0)
+arg_memory_type = BYTES_HUMAN;
+else if (strcmp(optarg, "bytes") == 0)
+arg_memory_type = BYTES_RAW;
+else
+return -EINVAL;
+}
+break;
+
+case ARG_IO_TYPE:
+if (optarg) {
+if (strcmp(optarg, "human") == 0)
+arg_io_type = BYTES_HUMAN;
+else if (strcmp(optarg, "bytes") == 0)
+arg_io_type = BYTES_RAW;
+else
+return -EINV