Oops more testing was required....

[Rogier Wolff]

On Wed, Jun 18, 2003 at 09:09:59PM +1000, Martin Pool wrote:
> On 17 Jun 2003, Rogier Wolff <R.E.Wolff at BitWizard.nl> wrote:
> > 
> > Oops. Missed one line in the last patch....
> 
> Thankyou.  That looks good.
> 
> If we're going to make this more accurate it might be worthwhile to
> actually look at how long we really did sleep for, and use that to
> adjust time_to_sleep rather than resetting to zero.
> 
> Also I'd prefer the variable be called micros_to_sleep or
> us_to_sleep.  Small point I know.

OK. Agreed. 

If I'm going to do "gettimeofday" calls, I might just as well keep a
variable that keeps the "time until we've used our quotum". 

Whenever we're ready to "sleep", we'll get the current time, and only
sleep for the difference.

The trouble is, that if we end up using 5 seconds worth of CPU time, 
we should not suddenly try to catch up for those whole 5 seconds.... 

I like to set bwlimit to about 90% of my link capacity. This causes
rsync not to fill the link: filling the link causes my provider to
drop my packets. Also rsync won't fill my queues leading to higher
latencies on interactive traffic.

But if we allow bursting of those 5 seconds that we didn't have any
data, we'll fill the link to the brim for 50 seconds! (if there
suddenly is a nice supply of fresh data) This is undesirable. So, I'm
still thinking on how to make this more accurate, but prevent big
bursts. I'm considering not allowing more than bwlimit of
backlog. (c.f. leaky bucket bandwidth limiting in the packet filtering
world........)

			Roger. 

> 
> > diff -ur rsync-2.5.6.orig/io.c rsync-2.5.6/io.c
> > +++ rsync-2.5.6/io.c	Tue Jun 17 23:43:49 2003
> > @@ -416,10 +416,19 @@
> >   * use a bit less bandwidth than specified, because it doesn't make up
> >   * for slow periods.  But arguably this is a feature.  In addition, we
> >   * ought to take the time used to write the data into account.
> > + *
> > + * During some phases of big transfers (file XXX is uptodate) this is
> > + * called with a small bytes_written every time. As the kernel has to
> > + * round small waits up to guarantee that we actually wait at least
> > + * the requested number of microseconds, this can become grossly
> > + * inaccurate. We therefore keep a cumulating number of microseconds
> > + * to wait, and only actually perform the sleep when the rouding
> > + * becomes insignificant. (less than 10%) -- REW.
> >   **/
> >  static void sleep_for_bwlimit(int bytes_written)
> >  {
> >  	struct timeval tv;
> > +	static int time_to_sleep = 0; 
> >  
> >  	if (!bwlimit)
> >  		return;
> > @@ -427,9 +436,13 @@
> >  	assert(bytes_written > 0);
> >  	assert(bwlimit > 0);
> >  	
> > -	tv.tv_usec = bytes_written * 1000 / bwlimit;
> > -	tv.tv_sec  = tv.tv_usec / 1000000;
> > -	tv.tv_usec = tv.tv_usec % 1000000;
> > +	time_to_sleep += bytes_written * 1000 / bwlimit; 
> > +
> > +	if (time_to_sleep < 100000) return;
> > +
> > +	tv.tv_sec  = time_to_sleep / 1000000;
> > +	tv.tv_usec = time_to_sleep % 1000000;
> > +	time_to_sleep = 0; 
> >  
> >  	select(0, NULL, NULL, NULL, &tv);
> >  }
> 
> -- 
> Martin 

-- 
** R.E.Wolff at BitWizard.nl ** http://www.BitWizard.nl/ ** +31-15-2600998 **
*-- BitWizard writes Linux device drivers for any device you may have! --*
* The Worlds Ecosystem is a stable system. Stable systems may experience *
* excursions from the stable situation. We are currently in such an      * 
* excursion: The stable situation does not include humans. ***************