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

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

[Michael Neuling]

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

Signed-off-by: Michael Neuling <mikey@neuling.org>
---
 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) {

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

[Andrew Morton]

On Fri, 10 Aug 2007 18:24:45 +1000 Michael Neuling <mikey@neuling.org> wrote:

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

What issues?

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

[Michael Neuling]

> > 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

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

[Balbir Singh]

Michael Neuling wrote:
> Fix a couple of minor issues with the getdelays.c example code. 
> 
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---
>  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

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

[Michael Neuling]

In message <46C2A505.7040008@linux.vnet.ibm.com> you wrote:
> Michael Neuling wrote:
> > Fix a couple of minor issues with the getdelays.c example code. 
> > 
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
> > ---
> >  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

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

[Balbir Singh]

<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

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

[Michael Neuling]

> 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

[PATCH] Documentation: fix getdelays.c printf bug

[Michael Neuling]

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 <mikey@ozlabs.org>
---
 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,

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

[Balbir Singh]

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 <mikey@ozlabs.org>
> ---
>  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 <balbir@linux.vnet.ibm.com>

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