[Qemu-devel] [PATCH] qemu-img: optimize is_allocated_sectors_min

[Qemu-devel] [PATCH] qemu-img: optimize is_allocated_sectors_min

[Peter Lieven]

the current implementation always splits requests if a buffer
begins or ends with zeroes independent of the length of the
zero area. Change this to really only split off zero areas
that have at least a length of 'min' bytes.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 qemu-img.c | 38 ++++++++++++--------------------------
 1 file changed, 12 insertions(+), 26 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 5df66fe..046696f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1010,45 +1010,31 @@ static int is_allocated_sectors(const uint8_t *buf, int n, int *pnum)
 }
 
 /*
- * Like is_allocated_sectors, but if the buffer starts with a used sector,
- * up to 'min' consecutive sectors containing zeros are ignored. This avoids
- * breaking up write requests for only small sparse areas.
+ * Like is_allocated_sectors, but only at least 'min' consecutive sectors
+ * containing zeros are considered unallocated. This avoids breaking up write
+ * requests for only small sparse areas.
  */
 static int is_allocated_sectors_min(const uint8_t *buf, int n, int *pnum,
-    int min)
+                                    int min)
 {
-    int ret;
-    int num_checked, num_used;
-
-    if (n < min) {
-        min = n;
-    }
-
-    ret = is_allocated_sectors(buf, n, pnum);
-    if (!ret) {
-        return ret;
-    }
-
-    num_used = *pnum;
-    buf += BDRV_SECTOR_SIZE * *pnum;
-    n -= *pnum;
-    num_checked = num_used;
+    int ret, num_checked = 0, num_used = 0;
 
     while (n > 0) {
         ret = is_allocated_sectors(buf, n, pnum);
-
         buf += BDRV_SECTOR_SIZE * *pnum;
         n -= *pnum;
         num_checked += *pnum;
-        if (ret) {
-            num_used = num_checked;
-        } else if (*pnum >= min) {
+        if (!ret && *pnum >= min) {
             break;
         }
+        num_used = num_checked;
     }
 
-    *pnum = num_used;
-    return 1;
+    if (num_used) {
+        *pnum = num_used;
+    }
+
+    return !!num_used;
 }
 
 /*
-- 
1.9.1

Re: [Qemu-devel] [PATCH] qemu-img: optimize is_allocated_sectors_min

[Peter Lieven]

Please ignore this one The code was suboptimal. Will send v2.

Peter

Am 19.01.2017 um 16:56 schrieb Peter Lieven:
> the current implementation always splits requests if a buffer
> begins or ends with zeroes independent of the length of the
> zero area. Change this to really only split off zero areas
> that have at least a length of 'min' bytes.
>
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>   qemu-img.c | 38 ++++++++++++--------------------------
>   1 file changed, 12 insertions(+), 26 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 5df66fe..046696f 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1010,45 +1010,31 @@ static int is_allocated_sectors(const uint8_t *buf, int n, int *pnum)
>   }
>   
>   /*
> - * Like is_allocated_sectors, but if the buffer starts with a used sector,
> - * up to 'min' consecutive sectors containing zeros are ignored. This avoids
> - * breaking up write requests for only small sparse areas.
> + * Like is_allocated_sectors, but only at least 'min' consecutive sectors
> + * containing zeros are considered unallocated. This avoids breaking up write
> + * requests for only small sparse areas.
>    */
>   static int is_allocated_sectors_min(const uint8_t *buf, int n, int *pnum,
> -    int min)
> +                                    int min)
>   {
> -    int ret;
> -    int num_checked, num_used;
> -
> -    if (n < min) {
> -        min = n;
> -    }
> -
> -    ret = is_allocated_sectors(buf, n, pnum);
> -    if (!ret) {
> -        return ret;
> -    }
> -
> -    num_used = *pnum;
> -    buf += BDRV_SECTOR_SIZE * *pnum;
> -    n -= *pnum;
> -    num_checked = num_used;
> +    int ret, num_checked = 0, num_used = 0;
>   
>       while (n > 0) {
>           ret = is_allocated_sectors(buf, n, pnum);
> -
>           buf += BDRV_SECTOR_SIZE * *pnum;
>           n -= *pnum;
>           num_checked += *pnum;
> -        if (ret) {
> -            num_used = num_checked;
> -        } else if (*pnum >= min) {
> +        if (!ret && *pnum >= min) {
>               break;
>           }
> +        num_used = num_checked;
>       }
>   
> -    *pnum = num_used;
> -    return 1;
> +    if (num_used) {
> +        *pnum = num_used;
> +    }
> +
> +    return !!num_used;
>   }
>   
>   /*