Re: [LTP] [PATCH] pipeio - number of writes overflow

[CAI Q. <ca...@cc...>]

Hi,



----- Original Message ----
> From: Jiri Palecek <jpa...@we...>
> To: ca...@cc...
> Cc: ltp...@li...
> Sent: Sunday, November 16, 2008 7:13:39 PM
> Subject: Re: [LTP] [PATCH] pipeio - number of writes overflow
> 
> On Saturday 15 November 2008 07:42:59 CAI Qian wrote:
> > Hi,
> >
> > --- On Wed, 11/12/08, Subrata Modak wrote:
> > > On Wed, 2008-11-12 at 00:41 +0100, Jiří Paleček wrote:
> > > > the computation of the number of writes in pipeio can
> > > > overflow, eg. if you
> > > > want to run more than 4 GB through the pipe. The
> > > > attached patch fixes that.
> > >
> > > Thanks for this.
> >
> > It can still overflow after applying the patch.
> 
> Yes; but not if the result is in range.
> 
> > ../pipeio -s 5000 -i 2000000000 -c 5
> >
> > pipeio      0  INFO  :  adjusting i/o size to 4096, and # of writes to
> > -1853561046 pipeio      1  PASS  :  1 PASS 1 pipe reads complete, read size
> > = 4096, named pipe, non-blking, pipeio      0  INFO  :  adjusting i/o size
> > to 4096, and # of writes to -1853561046 pipeio      1  FAIL  :  child pipe
> > open(tpipe.4735, 04001) failed: No such file or directory pipeio      2 
> > FAIL  :  Test failed
> > pipeio      0  INFO  :  adjusting i/o size to 4096, and # of writes to
> > -1853561046 pipeio      1  FAIL  :  child pipe open(tpipe.4735, 04001)
> > failed: No such file or directory pipeio      2  FAIL  :  Test failed
> > pipeio      0  INFO  :  adjusting i/o size to 4096, and # of writes to
> > -1853561046 pipeio      1  FAIL  :  child pipe open(tpipe.4735, 04001)
> > failed: No such file or directory pipeio      2  FAIL  :  Test failed
> > pipeio      0  INFO  :  adjusting i/o size to 4096, and # of writes to
> > -1853561046 pipeio      1  FAIL  :  child pipe open(tpipe.4735, 04001)
> > failed: No such file or directory pipeio      2  FAIL  :  Test failed
> >
> > The following modification made the overflow goes away.
> >
> > @@ -409,7 +409,7 @@
> >       */
> >      if ( size > PIPE_BUF && num_wrters > 1 ) {
> >          if ( ! loop ) {
> > -        j=((long long)num_writes*size+PIPE_BUF-1)/PIPE_BUF;
> > +        j=(int)((long long)num_writes*size+PIPE_BUF-1)/PIPE_BUF;
> >          num_writes=j;
> >              tst_resm (TINFO, "adjusting i/o size to %d, and # of writes to
> > %d", PIPE_BUF, num_writes);
> >
> > pipeio      0  INFO  :  adjusting i/o size to 4096, and # of writes to
> > 321322 pipeio      0  INFO  :  adjusting i/o size to 4096, and # of writes
> > to 321322 ....
> 
> No, it doesn't. Your change basically reverts my, but the problem persists. 
> You didn't want to run 300k writes (per writer) through the pipe, but 
> 2441406250 writes which is more than INT_MAX (on 32bit systems), so you got 
> the negative number (the failure is caused by a different thing, namely, the 
> parent not waiting for children). It would probably be appropriate to fix this 
> by making j unsigned, but I'm not sure if someone wanting more than 4G writes 
> would not emerge, closely followed by someone wanting more than 16P writes :-)
> 

That makes sense. It is also possible to have a check with INT_MAX in the test case for the overflowing beforehand to make the test more robust.

Cai Qian

> Regards
>     Jiri Palecek