2007-08-10 08:25:16

by Michael Neuling

[permalink] [raw]
Subject: [PATCH] Documentation: fix getdelays.c example -l option and segv

Fix a couple of minor issues with the getdelays.c example code.

Signed-off-by: Michael Neuling <[email protected]>
---
Documentation/accounting/getdelays.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)

Index: linux-2.6-ozlabs/Documentation/accounting/getdelays.c
===================================================================
--- linux-2.6-ozlabs.orig/Documentation/accounting/getdelays.c
+++ linux-2.6-ozlabs/Documentation/accounting/getdelays.c
@@ -196,7 +196,7 @@ void print_delayacct(struct taskstats *t
"IO %15s%15s\n"
" %15llu%15llu\n"
"MEM %15s%15s\n"
- " %15llu%15llu\n"
+ " %15llu%15llu\n",
"count", "real total", "virtual total", "delay total",
t->cpu_count, t->cpu_run_real_total, t->cpu_run_virtual_total,
t->cpu_delay_total,
@@ -335,17 +335,17 @@ int main(int argc, char *argv[])
}
}

- if (tid) {
- rc = send_cmd(nl_sd, id, mypid, TASKSTATS_CMD_GET,
- cmd_type, &tid, sizeof(__u32));
- PRINTF("Sent pid/tgid, retval %d\n", rc);
- if (rc < 0) {
- fprintf(stderr, "error sending tid/tgid cmd\n");
- goto done;
+ do {
+ if (tid) {
+ rc = send_cmd(nl_sd, id, mypid, TASKSTATS_CMD_GET,
+ cmd_type, &tid, sizeof(__u32));
+ PRINTF("Sent pid/tgid, retval %d\n", rc);
+ if (rc < 0) {
+ fprintf(stderr, "error sending tid/tgid cmd\n");
+ goto done;
+ }
}
- }

- do {
int i;

rep_len = recv(nl_sd, &msg, sizeof(msg), 0);
@@ -430,6 +430,7 @@ int main(int argc, char *argv[])
}
na = (struct nlattr *) (GENLMSG_DATA(&msg) + len);
}
+ sleep(2);
} while (loop);
done:
if (maskset) {





2007-08-10 08:34:07

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] Documentation: fix getdelays.c example -l option and segv

On Fri, 10 Aug 2007 18:24:45 +1000 Michael Neuling <[email protected]> wrote:

> Fix a couple of minor issues with the getdelays.c example code.

What issues?

2007-08-10 08:53:59

by Michael Neuling

[permalink] [raw]
Subject: Re: [PATCH] Documentation: fix getdelays.c example -l option and segv

> > Fix a couple of minor issues with the getdelays.c example code.
>
> What issues?

-l option (loop) doesn't work and it seg faults in print_delayacct.

Mikey

2007-08-15 07:02:50

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH] Documentation: fix getdelays.c example -l option and segv

Michael Neuling wrote:
> Fix a couple of minor issues with the getdelays.c example code.
>
> Signed-off-by: Michael Neuling <[email protected]>
> ---
> Documentation/accounting/getdelays.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> Index: linux-2.6-ozlabs/Documentation/accounting/getdelays.c
> ===================================================================
> --- linux-2.6-ozlabs.orig/Documentation/accounting/getdelays.c
> +++ linux-2.6-ozlabs/Documentation/accounting/getdelays.c
> @@ -196,7 +196,7 @@ void print_delayacct(struct taskstats *t
> "IO %15s%15s\n"
> " %15llu%15llu\n"
> "MEM %15s%15s\n"
> - " %15llu%15llu\n"
> + " %15llu%15llu\n",

This change looks good!

> "count", "real total", "virtual total", "delay total",
> t->cpu_count, t->cpu_run_real_total, t->cpu_run_virtual_total,
> t->cpu_delay_total,
> @@ -335,17 +335,17 @@ int main(int argc, char *argv[])
> }
> }
>
> - if (tid) {
> - rc = send_cmd(nl_sd, id, mypid, TASKSTATS_CMD_GET,
> - cmd_type, &tid, sizeof(__u32));
> - PRINTF("Sent pid/tgid, retval %d\n", rc);
> - if (rc < 0) {
> - fprintf(stderr, "error sending tid/tgid cmd\n");
> - goto done;
> + do {
> + if (tid) {
> + rc = send_cmd(nl_sd, id, mypid, TASKSTATS_CMD_GET,
> + cmd_type, &tid, sizeof(__u32));
> + PRINTF("Sent pid/tgid, retval %d\n", rc);
> + if (rc < 0) {
> + fprintf(stderr, "error sending tid/tgid cmd\n");
> + goto done;
> + }
> }
> - }
>
> - do {
> int i;
>
> rep_len = recv(nl_sd, &msg, sizeof(msg), 0);
> @@ -430,6 +430,7 @@ int main(int argc, char *argv[])
> }
> na = (struct nlattr *) (GENLMSG_DATA(&msg) + len);
> }
> + sleep(2);

Is this really required? a sleep() in the code. Why do we do multiple send_cmd()'s
in the do loop? I'll test and get back.

> } while (loop);
> done:
> if (maskset) {
>
>
>
>


--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL

2007-08-15 08:14:31

by Michael Neuling

[permalink] [raw]
Subject: Re: [PATCH] Documentation: fix getdelays.c example -l option and segv

In message <[email protected]> you wrote:
> Michael Neuling wrote:
> > Fix a couple of minor issues with the getdelays.c example code.
> >
> > Signed-off-by: Michael Neuling <[email protected]>
> > ---
> > Documentation/accounting/getdelays.c | 21 +++++++++++----------
> > 1 file changed, 11 insertions(+), 10 deletions(-)
> >
> > Index: linux-2.6-ozlabs/Documentation/accounting/getdelays.c
> > ===================================================================
> > --- linux-2.6-ozlabs.orig/Documentation/accounting/getdelays.c
> > +++ linux-2.6-ozlabs/Documentation/accounting/getdelays.c
> > @@ -196,7 +196,7 @@ void print_delayacct(struct taskstats *t
> > "IO %15s%15s\n"
> > " %15llu%15llu\n"
> > "MEM %15s%15s\n"
> > - " %15llu%15llu\n"
> > + " %15llu%15llu\n",
>
> This change looks good!

Looks like it was randomly removed in b663a79c191508f27cd885224b592a878c0ba0f6.

>
> > "count", "real total", "virtual total", "delay total",
> > t->cpu_count, t->cpu_run_real_total, t->cpu_run_virtual_total,
> > t->cpu_delay_total,
> > @@ -335,17 +335,17 @@ int main(int argc, char *argv[])
> > }
> > }
> >
> > - if (tid) {
> > - rc = send_cmd(nl_sd, id, mypid, TASKSTATS_CMD_GET,
> > - cmd_type, &tid, sizeof(__u32));
> > - PRINTF("Sent pid/tgid, retval %d\n", rc);
> > - if (rc < 0) {
> > - fprintf(stderr, "error sending tid/tgid cmd\n");
> > - goto done;
> > + do {
> > + if (tid) {
> > + rc = send_cmd(nl_sd, id, mypid, TASKSTATS_CMD_GET,
> > + cmd_type, &tid, sizeof(__u32));
> > + PRINTF("Sent pid/tgid, retval %d\n", rc);
> > + if (rc < 0) {
> > + fprintf(stderr, "error sending tid/tgid cmd\n")
;
> > + goto done;
> > + }
> > }
> > - }
> >
> > - do {
> > int i;
> >
> > rep_len = recv(nl_sd, &msg, sizeof(msg), 0);
> > @@ -430,6 +430,7 @@ int main(int argc, char *argv[])
> > }
> > na = (struct nlattr *) (GENLMSG_DATA(&msg) + len);
> > }
> > + sleep(2);
>
> Is this really required? a sleep() in the code. Why do we do multiple
> send_cmd()'s in the do loop? I'll test and get back.

I needed to send another command to receive more data. Without the per
loop send_cmd, I never got any more stats.

Should we receive more data without the additional send_cmd? I've been
running with:
./getdelays -d -p 1 -l

Mikey


2007-08-15 11:21:12

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH] Documentation: fix getdelays.c example -l option and segv

<snip>

>> Is this really required? a sleep() in the code. Why do we do multiple
>> send_cmd()'s in the do loop? I'll test and get back.
>
> I needed to send another command to receive more data. Without the per
> loop send_cmd, I never got any more stats.
>
> Should we receive more data without the additional send_cmd? I've been
> running with:
> ./getdelays -d -p 1 -l
>

Interesting, we never used -l that way. We used -l to get data for all
exiting tasks

./getdelays -d -l

For the usage you've mentioned, I'd use

while :
do
sleep 2
./getdelays -p 1 -d
done

I guess your changes change getdelays and since there are other ways
to obtaining the same data, I'd remove the last bit of changes made
to send the TGID often to get data.

> Mikey
>
>


--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL

2007-08-15 12:00:36

by Michael Neuling

[permalink] [raw]
Subject: Re: [PATCH] Documentation: fix getdelays.c example -l option and segv

> Interesting, we never used -l that way. We used -l to get data for all
> exiting tasks
>
> ./getdelays -d -l
>
> For the usage you've mentioned, I'd use
>
> while :
> do
> sleep 2
> ./getdelays -p 1 -d
> done
>
> I guess your changes change getdelays and since there are other ways
> to obtaining the same data, I'd remove the last bit of changes made
> to send the TGID often to get data.

Ok. I'll resubmit with just the first fix.

Mikey

2007-08-15 12:08:48

by Michael Neuling

[permalink] [raw]
Subject: [PATCH] Documentation: fix getdelays.c printf bug

b663a79c191508f27cd885224b592a878c0ba0f6 incorrectly removed a comma
from a printf statement. This causes corruption in the output printing
or a seg fault.

Signed-off-by: Michael Neuling <[email protected]>
---
Documentation/accounting/getdelays.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

akpm: please replace
documentation-fix-getdelaysc-example-l-option-and-segv.patch with this.

Index: linux-2.6-ozlabs/Documentation/accounting/getdelays.c
===================================================================
--- linux-2.6-ozlabs.orig/Documentation/accounting/getdelays.c
+++ linux-2.6-ozlabs/Documentation/accounting/getdelays.c
@@ -196,7 +196,7 @@ void print_delayacct(struct taskstats *t
"IO %15s%15s\n"
" %15llu%15llu\n"
"MEM %15s%15s\n"
- " %15llu%15llu\n"
+ " %15llu%15llu\n",
"count", "real total", "virtual total", "delay total",
t->cpu_count, t->cpu_run_real_total, t->cpu_run_virtual_total,
t->cpu_delay_total,

2007-08-15 13:16:21

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH] Documentation: fix getdelays.c printf bug

Michael Neuling wrote:
> b663a79c191508f27cd885224b592a878c0ba0f6 incorrectly removed a comma
> from a printf statement. This causes corruption in the output printing
> or a seg fault.
>
> Signed-off-by: Michael Neuling <[email protected]>
> ---
> Documentation/accounting/getdelays.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> akpm: please replace
> documentation-fix-getdelaysc-example-l-option-and-segv.patch with this.
>
> Index: linux-2.6-ozlabs/Documentation/accounting/getdelays.c
> ===================================================================
> --- linux-2.6-ozlabs.orig/Documentation/accounting/getdelays.c
> +++ linux-2.6-ozlabs/Documentation/accounting/getdelays.c
> @@ -196,7 +196,7 @@ void print_delayacct(struct taskstats *t
> "IO %15s%15s\n"
> " %15llu%15llu\n"
> "MEM %15s%15s\n"
> - " %15llu%15llu\n"
> + " %15llu%15llu\n",
> "count", "real total", "virtual total", "delay total",
> t->cpu_count, t->cpu_run_real_total, t->cpu_run_virtual_total,
> t->cpu_delay_total,

This is exactly what Maxim had sent as well.

Acked-by: Balbir Singh <[email protected]>

--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL