Patchwork [11/14] buffered_file: don't flush on put buffer

login
register
mail settings
Submitter Juan Quintela
Date Sept. 21, 2012, 2:08 p.m.
Message ID <1348236500-2565-12-git-send-email-quintela@redhat.com>
Download mbox | patch
Permalink /patch/185777/
State New
Headers show

Comments

Juan Quintela - Sept. 21, 2012, 2:08 p.m.
We call buffered_put_buffer with iothread held, and buffered_flush() does
synchronous writes.  We only want to do the synchronous writes outside.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 buffered_file.c | 6 ------
 1 file changed, 6 deletions(-)
Paolo Bonzini - Sept. 21, 2012, 3:34 p.m.
Il 21/09/2012 16:08, Juan Quintela ha scritto:
> We call buffered_put_buffer with iothread held, and buffered_flush() does
> synchronous writes.  We only want to do the synchronous writes outside.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  buffered_file.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/buffered_file.c b/buffered_file.c
> index f4a788d..929b911 100644
> --- a/buffered_file.c
> +++ b/buffered_file.c
> @@ -107,12 +107,6 @@ static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
>          buffered_append(s, buf, size);
>      }
> 
> -    error = buffered_flush(s);
> -    if (error < 0) {
> -        DPRINTF("buffered flush error. bailing: %s\n", strerror(-error));
> -        return error;
> -    }
> -
>      return size;
>  }
> 

This means that the buffer can grow to up to s->xfer_limit bytes.
Perhaps you want to make the granularity (currently fixed to 100ms) a
bit smaller, like 20-30 ms, if the bandwidth limit grows above say 100
MB/sec?  This can be done on top of this series however.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Eric Blake - Sept. 21, 2012, 9:12 p.m.
On 09/21/2012 09:34 AM, Paolo Bonzini wrote:
> Il 21/09/2012 16:08, Juan Quintela ha scritto:
>> We call buffered_put_buffer with iothread held, and buffered_flush() does
>> synchronous writes.  We only want to do the synchronous writes outside.
>>

> 
> This means that the buffer can grow to up to s->xfer_limit bytes.
> Perhaps you want to make the granularity (currently fixed to 100ms) a
> bit smaller, like 20-30 ms, if the bandwidth limit grows above say 100
> MB/sec?  This can be done on top of this series however.

Note that libvirt intentionally tries to raise the bandwidth as high as
possible in cases where the user did not request a particular bandwidth.

Patch

diff --git a/buffered_file.c b/buffered_file.c
index f4a788d..929b911 100644
--- a/buffered_file.c
+++ b/buffered_file.c
@@ -107,12 +107,6 @@  static int buffered_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, in
         buffered_append(s, buf, size);
     }

-    error = buffered_flush(s);
-    if (error < 0) {
-        DPRINTF("buffered flush error. bailing: %s\n", strerror(-error));
-        return error;
-    }
-
     return size;
 }