Re: [PATCH] blk-throttle: Do not trim slices if there is remainder after division

[Vivek Goyal <vgoyal@redhat.com>]

On Thu, Aug 25, 2011 at 04:32:25PM -0400, Vivek Goyal wrote:
> Throttling code divides a second into smaller slices of 100ms each and
> then decides how many IOPs are allowed in that 100ms. Once IO has been
> dispatched, these slices are reaped off from effective slice.
> 
> Division by a factor of 10, can lead to error if input IOPS rate is
> not a mulitple of 10. As remainder is discarded. So if input rate is
> 69 IOPS, then we effectively get only 60 IOPS.
> 
> Hence do not trim slice at every 100ms period if input rate is not
> a multiple of 10. Instead wait for full second to complete and 
> then trim 10 slices at one go.
> 
> Reported-by: Ryan Harper <ryanh@us.ibm.com>
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>

Hi Jens,

Do you have any concerns with this patch. If not, can you please cosider
applying it.

Thanks
Vivek

> ---
>  block/blk-throttle.c |   15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6/block/blk-throttle.c
> ===================================================================
> --- linux-2.6.orig/block/blk-throttle.c	2011-08-25 16:27:55.869757580 -0400
> +++ linux-2.6/block/blk-throttle.c	2011-08-25 16:28:34.866572646 -0400
> @@ -534,7 +534,7 @@ throtl_slice_used(struct throtl_data *td
>  static inline void
>  throtl_trim_slice(struct throtl_data *td, struct throtl_grp *tg, bool rw)
>  {
> -	unsigned long nr_slices, time_elapsed, io_trim;
> +	unsigned long nr_slices, time_elapsed, io_trim, io_remainder;
>  	u64 bytes_trim, tmp;
>  
>  	BUG_ON(time_before(tg->slice_end[rw], tg->slice_start[rw]));
> @@ -569,6 +569,19 @@ throtl_trim_slice(struct throtl_data *td
>  
>  	io_trim = (tg->iops[rw] * throtl_slice * nr_slices)/HZ;
>  
> +	/*
> +	 * Due to division operation of input rate, we can lose some accuracy
> +	 * as raminder is discarded. For example with iops=69, we will dispatch
> +	 * 6 ios but then trim slice after 100ms and never extend slice enough
> +	 * so that over a period of 1 second 69 IOs are dispatched. Instead we
> +	 * dispatch 6 IO at each slice interval and trim the slice. So go
> +	 * ahead with trim operation only when there is no remainder. That
> +	 */
> +	io_remainder = (tg->iops[rw] * throtl_slice * nr_slices)%HZ;
> +
> +	if (io_remainder)
> +		return;
> +
>  	if (!bytes_trim && !io_trim)
>  		return;
>