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

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

[Vivek Goyal]

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

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

[Vivek Goyal]

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