* [Qemu-devel] [PATCH 0/2] block/nfs optimizations
@ 2017-02-17 16:38 Peter Lieven
2017-02-17 16:39 ` [Qemu-devel] [PATCH 1/2] block/nfs: convert to preadv / pwritev Peter Lieven
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Peter Lieven @ 2017-02-17 16:38 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, mreitz, jcody, Peter Lieven
Peter Lieven (2):
block/nfs: convert to preadv / pwritev
block/nfs: try to avoid the bounce buffer in pwritev
block/nfs.c | 50 ++++++++++++++++++++++++++++----------------------
1 file changed, 28 insertions(+), 22 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 1/2] block/nfs: convert to preadv / pwritev
2017-02-17 16:38 [Qemu-devel] [PATCH 0/2] block/nfs optimizations Peter Lieven
@ 2017-02-17 16:39 ` Peter Lieven
2017-02-17 21:33 ` Jeff Cody
2017-02-17 16:39 ` [Qemu-devel] [PATCH 2/2] block/nfs: try to avoid the bounce buffer in pwritev Peter Lieven
2017-02-24 3:50 ` [Qemu-devel] [PATCH 0/2] block/nfs optimizations Jeff Cody
2 siblings, 1 reply; 10+ messages in thread
From: Peter Lieven @ 2017-02-17 16:39 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, mreitz, jcody, Peter Lieven
Signed-off-by: Peter Lieven <pl@kamp.de>
---
block/nfs.c | 33 +++++++++++++++------------------
1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/block/nfs.c b/block/nfs.c
index 689eaa7..ab5dcc2 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -256,9 +256,9 @@ nfs_co_generic_cb(int ret, struct nfs_context *nfs, void *data,
nfs_co_generic_bh_cb, task);
}
-static int coroutine_fn nfs_co_readv(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors,
- QEMUIOVector *iov)
+static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, uint64_t offset,
+ uint64_t bytes, QEMUIOVector *iov,
+ int flags)
{
NFSClient *client = bs->opaque;
NFSRPC task;
@@ -267,9 +267,7 @@ static int coroutine_fn nfs_co_readv(BlockDriverState *bs,
task.iov = iov;
if (nfs_pread_async(client->context, client->fh,
- sector_num * BDRV_SECTOR_SIZE,
- nb_sectors * BDRV_SECTOR_SIZE,
- nfs_co_generic_cb, &task) != 0) {
+ offset, bytes, nfs_co_generic_cb, &task) != 0) {
return -ENOMEM;
}
@@ -290,9 +288,9 @@ static int coroutine_fn nfs_co_readv(BlockDriverState *bs,
return 0;
}
-static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors,
- QEMUIOVector *iov)
+static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
+ uint64_t bytes, QEMUIOVector *iov,
+ int flags)
{
NFSClient *client = bs->opaque;
NFSRPC task;
@@ -300,17 +298,16 @@ static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
nfs_co_init_task(bs, &task);
- buf = g_try_malloc(nb_sectors * BDRV_SECTOR_SIZE);
- if (nb_sectors && buf == NULL) {
+ buf = g_try_malloc(bytes);
+ if (bytes && buf == NULL) {
return -ENOMEM;
}
- qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE);
+ qemu_iovec_to_buf(iov, 0, buf, bytes);
if (nfs_pwrite_async(client->context, client->fh,
- sector_num * BDRV_SECTOR_SIZE,
- nb_sectors * BDRV_SECTOR_SIZE,
- buf, nfs_co_generic_cb, &task) != 0) {
+ offset, bytes, buf,
+ nfs_co_generic_cb, &task) != 0) {
g_free(buf);
return -ENOMEM;
}
@@ -322,7 +319,7 @@ static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
g_free(buf);
- if (task.ret != nb_sectors * BDRV_SECTOR_SIZE) {
+ if (task.ret != bytes) {
return task.ret < 0 ? task.ret : -EIO;
}
@@ -856,8 +853,8 @@ static BlockDriver bdrv_nfs = {
.bdrv_create = nfs_file_create,
.bdrv_reopen_prepare = nfs_reopen_prepare,
- .bdrv_co_readv = nfs_co_readv,
- .bdrv_co_writev = nfs_co_writev,
+ .bdrv_co_preadv = nfs_co_preadv,
+ .bdrv_co_pwritev = nfs_co_pwritev,
.bdrv_co_flush_to_disk = nfs_co_flush,
.bdrv_detach_aio_context = nfs_detach_aio_context,
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 2/2] block/nfs: try to avoid the bounce buffer in pwritev
2017-02-17 16:38 [Qemu-devel] [PATCH 0/2] block/nfs optimizations Peter Lieven
2017-02-17 16:39 ` [Qemu-devel] [PATCH 1/2] block/nfs: convert to preadv / pwritev Peter Lieven
@ 2017-02-17 16:39 ` Peter Lieven
2017-02-17 21:37 ` Jeff Cody
2017-02-24 3:50 ` [Qemu-devel] [PATCH 0/2] block/nfs optimizations Jeff Cody
2 siblings, 1 reply; 10+ messages in thread
From: Peter Lieven @ 2017-02-17 16:39 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf, mreitz, jcody, Peter Lieven
if the passed qiov contains exactly one iov we can
pass the buffer directly.
Signed-off-by: Peter Lieven <pl@kamp.de>
---
block/nfs.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/block/nfs.c b/block/nfs.c
index ab5dcc2..bb4b75f 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -295,20 +295,27 @@ static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
NFSClient *client = bs->opaque;
NFSRPC task;
char *buf = NULL;
+ bool my_buffer = false;
nfs_co_init_task(bs, &task);
- buf = g_try_malloc(bytes);
- if (bytes && buf == NULL) {
- return -ENOMEM;
+ if (iov->niov != 1) {
+ buf = g_try_malloc(bytes);
+ if (bytes && buf == NULL) {
+ return -ENOMEM;
+ }
+ qemu_iovec_to_buf(iov, 0, buf, bytes);
+ my_buffer = true;
+ } else {
+ buf = iov->iov[0].iov_base;
}
- qemu_iovec_to_buf(iov, 0, buf, bytes);
-
if (nfs_pwrite_async(client->context, client->fh,
offset, bytes, buf,
nfs_co_generic_cb, &task) != 0) {
- g_free(buf);
+ if (my_buffer) {
+ g_free(buf);
+ }
return -ENOMEM;
}
@@ -317,7 +324,9 @@ static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
qemu_coroutine_yield();
}
- g_free(buf);
+ if (my_buffer) {
+ g_free(buf);
+ }
if (task.ret != bytes) {
return task.ret < 0 ? task.ret : -EIO;
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] block/nfs: convert to preadv / pwritev
2017-02-17 16:39 ` [Qemu-devel] [PATCH 1/2] block/nfs: convert to preadv / pwritev Peter Lieven
@ 2017-02-17 21:33 ` Jeff Cody
0 siblings, 0 replies; 10+ messages in thread
From: Jeff Cody @ 2017-02-17 21:33 UTC (permalink / raw)
To: Peter Lieven; +Cc: qemu-devel, qemu-block, kwolf, mreitz
On Fri, Feb 17, 2017 at 05:39:00PM +0100, Peter Lieven wrote:
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
> block/nfs.c | 33 +++++++++++++++------------------
> 1 file changed, 15 insertions(+), 18 deletions(-)
>
> diff --git a/block/nfs.c b/block/nfs.c
> index 689eaa7..ab5dcc2 100644
> --- a/block/nfs.c
> +++ b/block/nfs.c
> @@ -256,9 +256,9 @@ nfs_co_generic_cb(int ret, struct nfs_context *nfs, void *data,
> nfs_co_generic_bh_cb, task);
> }
>
> -static int coroutine_fn nfs_co_readv(BlockDriverState *bs,
> - int64_t sector_num, int nb_sectors,
> - QEMUIOVector *iov)
> +static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, uint64_t offset,
> + uint64_t bytes, QEMUIOVector *iov,
> + int flags)
> {
> NFSClient *client = bs->opaque;
> NFSRPC task;
> @@ -267,9 +267,7 @@ static int coroutine_fn nfs_co_readv(BlockDriverState *bs,
> task.iov = iov;
>
> if (nfs_pread_async(client->context, client->fh,
> - sector_num * BDRV_SECTOR_SIZE,
> - nb_sectors * BDRV_SECTOR_SIZE,
> - nfs_co_generic_cb, &task) != 0) {
> + offset, bytes, nfs_co_generic_cb, &task) != 0) {
> return -ENOMEM;
> }
>
> @@ -290,9 +288,9 @@ static int coroutine_fn nfs_co_readv(BlockDriverState *bs,
> return 0;
> }
>
> -static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
> - int64_t sector_num, int nb_sectors,
> - QEMUIOVector *iov)
> +static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
> + uint64_t bytes, QEMUIOVector *iov,
> + int flags)
> {
> NFSClient *client = bs->opaque;
> NFSRPC task;
> @@ -300,17 +298,16 @@ static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
>
> nfs_co_init_task(bs, &task);
>
> - buf = g_try_malloc(nb_sectors * BDRV_SECTOR_SIZE);
> - if (nb_sectors && buf == NULL) {
> + buf = g_try_malloc(bytes);
> + if (bytes && buf == NULL) {
> return -ENOMEM;
> }
>
> - qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE);
> + qemu_iovec_to_buf(iov, 0, buf, bytes);
>
> if (nfs_pwrite_async(client->context, client->fh,
> - sector_num * BDRV_SECTOR_SIZE,
> - nb_sectors * BDRV_SECTOR_SIZE,
> - buf, nfs_co_generic_cb, &task) != 0) {
> + offset, bytes, buf,
> + nfs_co_generic_cb, &task) != 0) {
> g_free(buf);
> return -ENOMEM;
> }
> @@ -322,7 +319,7 @@ static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
>
> g_free(buf);
>
> - if (task.ret != nb_sectors * BDRV_SECTOR_SIZE) {
> + if (task.ret != bytes) {
> return task.ret < 0 ? task.ret : -EIO;
> }
>
> @@ -856,8 +853,8 @@ static BlockDriver bdrv_nfs = {
> .bdrv_create = nfs_file_create,
> .bdrv_reopen_prepare = nfs_reopen_prepare,
>
> - .bdrv_co_readv = nfs_co_readv,
> - .bdrv_co_writev = nfs_co_writev,
> + .bdrv_co_preadv = nfs_co_preadv,
> + .bdrv_co_pwritev = nfs_co_pwritev,
> .bdrv_co_flush_to_disk = nfs_co_flush,
>
> .bdrv_detach_aio_context = nfs_detach_aio_context,
> --
> 1.9.1
>
Reviewed-by: Jeff Cody <jcody@redhat.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] block/nfs: try to avoid the bounce buffer in pwritev
2017-02-17 16:39 ` [Qemu-devel] [PATCH 2/2] block/nfs: try to avoid the bounce buffer in pwritev Peter Lieven
@ 2017-02-17 21:37 ` Jeff Cody
2017-02-17 21:42 ` Eric Blake
0 siblings, 1 reply; 10+ messages in thread
From: Jeff Cody @ 2017-02-17 21:37 UTC (permalink / raw)
To: Peter Lieven; +Cc: qemu-devel, qemu-block, kwolf, mreitz
On Fri, Feb 17, 2017 at 05:39:01PM +0100, Peter Lieven wrote:
> if the passed qiov contains exactly one iov we can
> pass the buffer directly.
>
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
> block/nfs.c | 23 ++++++++++++++++-------
> 1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/block/nfs.c b/block/nfs.c
> index ab5dcc2..bb4b75f 100644
> --- a/block/nfs.c
> +++ b/block/nfs.c
> @@ -295,20 +295,27 @@ static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
> NFSClient *client = bs->opaque;
> NFSRPC task;
> char *buf = NULL;
> + bool my_buffer = false;
g_free() is a nop if buf is NULL, so there is no need for the my_buffer
variable & check.
>
> nfs_co_init_task(bs, &task);
>
> - buf = g_try_malloc(bytes);
> - if (bytes && buf == NULL) {
> - return -ENOMEM;
> + if (iov->niov != 1) {
> + buf = g_try_malloc(bytes);
> + if (bytes && buf == NULL) {
> + return -ENOMEM;
> + }
> + qemu_iovec_to_buf(iov, 0, buf, bytes);
> + my_buffer = true;
> + } else {
> + buf = iov->iov[0].iov_base;
> }
>
> - qemu_iovec_to_buf(iov, 0, buf, bytes);
> -
> if (nfs_pwrite_async(client->context, client->fh,
> offset, bytes, buf,
> nfs_co_generic_cb, &task) != 0) {
> - g_free(buf);
> + if (my_buffer) {
> + g_free(buf);
> + }
> return -ENOMEM;
> }
>
> @@ -317,7 +324,9 @@ static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
> qemu_coroutine_yield();
> }
>
> - g_free(buf);
> + if (my_buffer) {
> + g_free(buf);
> + }
>
> if (task.ret != bytes) {
> return task.ret < 0 ? task.ret : -EIO;
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] block/nfs: try to avoid the bounce buffer in pwritev
2017-02-17 21:37 ` Jeff Cody
@ 2017-02-17 21:42 ` Eric Blake
2017-02-17 21:51 ` Jeff Cody
0 siblings, 1 reply; 10+ messages in thread
From: Eric Blake @ 2017-02-17 21:42 UTC (permalink / raw)
To: Jeff Cody, Peter Lieven; +Cc: kwolf, qemu-devel, qemu-block, mreitz
[-- Attachment #1: Type: text/plain, Size: 1708 bytes --]
On 02/17/2017 03:37 PM, Jeff Cody wrote:
> On Fri, Feb 17, 2017 at 05:39:01PM +0100, Peter Lieven wrote:
>> if the passed qiov contains exactly one iov we can
>> pass the buffer directly.
>>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>> block/nfs.c | 23 ++++++++++++++++-------
>> 1 file changed, 16 insertions(+), 7 deletions(-)
>>
>> diff --git a/block/nfs.c b/block/nfs.c
>> index ab5dcc2..bb4b75f 100644
>> --- a/block/nfs.c
>> +++ b/block/nfs.c
>> @@ -295,20 +295,27 @@ static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
>> NFSClient *client = bs->opaque;
>> NFSRPC task;
>> char *buf = NULL;
>> + bool my_buffer = false;
>
> g_free() is a nop if buf is NULL, so there is no need for the my_buffer
> variable & check.
Umm, yes there is:
>> + if (iov->niov != 1) {
>> + buf = g_try_malloc(bytes);
>> + if (bytes && buf == NULL) {
>> + return -ENOMEM;
>> + }
>> + qemu_iovec_to_buf(iov, 0, buf, bytes);
>> + my_buffer = true;
>> + } else {
>> + buf = iov->iov[0].iov_base;
If we took the else branch, then we definitely do not want to be calling
g_free(buf).
>> }
>>
>> - qemu_iovec_to_buf(iov, 0, buf, bytes);
>> -
>> if (nfs_pwrite_async(client->context, client->fh,
>> offset, bytes, buf,
>> nfs_co_generic_cb, &task) != 0) {
>> - g_free(buf);
>> + if (my_buffer) {
>> + g_free(buf);
>> + }
It looks correct as-is to me.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] block/nfs: try to avoid the bounce buffer in pwritev
2017-02-17 21:42 ` Eric Blake
@ 2017-02-17 21:51 ` Jeff Cody
2017-02-22 12:47 ` Kevin Wolf
0 siblings, 1 reply; 10+ messages in thread
From: Jeff Cody @ 2017-02-17 21:51 UTC (permalink / raw)
To: Eric Blake; +Cc: Peter Lieven, kwolf, qemu-devel, qemu-block, mreitz
On Fri, Feb 17, 2017 at 03:42:52PM -0600, Eric Blake wrote:
> On 02/17/2017 03:37 PM, Jeff Cody wrote:
> > On Fri, Feb 17, 2017 at 05:39:01PM +0100, Peter Lieven wrote:
> >> if the passed qiov contains exactly one iov we can
> >> pass the buffer directly.
> >>
> >> Signed-off-by: Peter Lieven <pl@kamp.de>
> >> ---
> >> block/nfs.c | 23 ++++++++++++++++-------
> >> 1 file changed, 16 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/block/nfs.c b/block/nfs.c
> >> index ab5dcc2..bb4b75f 100644
> >> --- a/block/nfs.c
> >> +++ b/block/nfs.c
> >> @@ -295,20 +295,27 @@ static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
> >> NFSClient *client = bs->opaque;
> >> NFSRPC task;
> >> char *buf = NULL;
> >> + bool my_buffer = false;
> >
> > g_free() is a nop if buf is NULL, so there is no need for the my_buffer
> > variable & check.
>
> Umm, yes there is:
>
> >> + if (iov->niov != 1) {
> >> + buf = g_try_malloc(bytes);
> >> + if (bytes && buf == NULL) {
> >> + return -ENOMEM;
> >> + }
> >> + qemu_iovec_to_buf(iov, 0, buf, bytes);
> >> + my_buffer = true;
> >> + } else {
> >> + buf = iov->iov[0].iov_base;
>
> If we took the else branch, then we definitely do not want to be calling
> g_free(buf).
Doh!
>
> >> }
> >>
> >> - qemu_iovec_to_buf(iov, 0, buf, bytes);
> >> -
> >> if (nfs_pwrite_async(client->context, client->fh,
> >> offset, bytes, buf,
> >> nfs_co_generic_cb, &task) != 0) {
> >> - g_free(buf);
> >> + if (my_buffer) {
> >> + g_free(buf);
> >> + }
>
> It looks correct as-is to me.
Indeed.
Reviewed-by: Jeff Cody <jcody@redhat.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] block/nfs: try to avoid the bounce buffer in pwritev
2017-02-17 21:51 ` Jeff Cody
@ 2017-02-22 12:47 ` Kevin Wolf
2017-02-22 12:48 ` Jeff Cody
0 siblings, 1 reply; 10+ messages in thread
From: Kevin Wolf @ 2017-02-22 12:47 UTC (permalink / raw)
To: Jeff Cody; +Cc: Eric Blake, Peter Lieven, qemu-devel, qemu-block, mreitz
Am 17.02.2017 um 22:51 hat Jeff Cody geschrieben:
> On Fri, Feb 17, 2017 at 03:42:52PM -0600, Eric Blake wrote:
> > On 02/17/2017 03:37 PM, Jeff Cody wrote:
> > > On Fri, Feb 17, 2017 at 05:39:01PM +0100, Peter Lieven wrote:
> > >> if the passed qiov contains exactly one iov we can
> > >> pass the buffer directly.
> > >>
> > >> Signed-off-by: Peter Lieven <pl@kamp.de>
> > >> ---
> > >> block/nfs.c | 23 ++++++++++++++++-------
> > >> 1 file changed, 16 insertions(+), 7 deletions(-)
> > >>
> > >> diff --git a/block/nfs.c b/block/nfs.c
> > >> index ab5dcc2..bb4b75f 100644
> > >> --- a/block/nfs.c
> > >> +++ b/block/nfs.c
> > >> @@ -295,20 +295,27 @@ static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
> > >> NFSClient *client = bs->opaque;
> > >> NFSRPC task;
> > >> char *buf = NULL;
> > >> + bool my_buffer = false;
> > >
> > > g_free() is a nop if buf is NULL, so there is no need for the my_buffer
> > > variable & check.
> >
> > Umm, yes there is:
> >
> > >> + if (iov->niov != 1) {
> > >> + buf = g_try_malloc(bytes);
> > >> + if (bytes && buf == NULL) {
> > >> + return -ENOMEM;
> > >> + }
> > >> + qemu_iovec_to_buf(iov, 0, buf, bytes);
> > >> + my_buffer = true;
> > >> + } else {
> > >> + buf = iov->iov[0].iov_base;
> >
> > If we took the else branch, then we definitely do not want to be calling
> > g_free(buf).
>
> Doh!
>
> >
> > >> }
> > >>
> > >> - qemu_iovec_to_buf(iov, 0, buf, bytes);
> > >> -
> > >> if (nfs_pwrite_async(client->context, client->fh,
> > >> offset, bytes, buf,
> > >> nfs_co_generic_cb, &task) != 0) {
> > >> - g_free(buf);
> > >> + if (my_buffer) {
> > >> + g_free(buf);
> > >> + }
> >
> > It looks correct as-is to me.
>
> Indeed.
>
> Reviewed-by: Jeff Cody <jcody@redhat.com>
You gave R-b for both patches, but did not merge it - who is supposed to
do that?
Kevin
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] block/nfs: try to avoid the bounce buffer in pwritev
2017-02-22 12:47 ` Kevin Wolf
@ 2017-02-22 12:48 ` Jeff Cody
0 siblings, 0 replies; 10+ messages in thread
From: Jeff Cody @ 2017-02-22 12:48 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Eric Blake, Peter Lieven, qemu-devel, qemu-block, mreitz
On Wed, Feb 22, 2017 at 01:47:10PM +0100, Kevin Wolf wrote:
> Am 17.02.2017 um 22:51 hat Jeff Cody geschrieben:
> > On Fri, Feb 17, 2017 at 03:42:52PM -0600, Eric Blake wrote:
> > > On 02/17/2017 03:37 PM, Jeff Cody wrote:
> > > > On Fri, Feb 17, 2017 at 05:39:01PM +0100, Peter Lieven wrote:
> > > >> if the passed qiov contains exactly one iov we can
> > > >> pass the buffer directly.
> > > >>
> > > >> Signed-off-by: Peter Lieven <pl@kamp.de>
> > > >> ---
> > > >> block/nfs.c | 23 ++++++++++++++++-------
> > > >> 1 file changed, 16 insertions(+), 7 deletions(-)
> > > >>
> > > >> diff --git a/block/nfs.c b/block/nfs.c
> > > >> index ab5dcc2..bb4b75f 100644
> > > >> --- a/block/nfs.c
> > > >> +++ b/block/nfs.c
> > > >> @@ -295,20 +295,27 @@ static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
> > > >> NFSClient *client = bs->opaque;
> > > >> NFSRPC task;
> > > >> char *buf = NULL;
> > > >> + bool my_buffer = false;
> > > >
> > > > g_free() is a nop if buf is NULL, so there is no need for the my_buffer
> > > > variable & check.
> > >
> > > Umm, yes there is:
> > >
> > > >> + if (iov->niov != 1) {
> > > >> + buf = g_try_malloc(bytes);
> > > >> + if (bytes && buf == NULL) {
> > > >> + return -ENOMEM;
> > > >> + }
> > > >> + qemu_iovec_to_buf(iov, 0, buf, bytes);
> > > >> + my_buffer = true;
> > > >> + } else {
> > > >> + buf = iov->iov[0].iov_base;
> > >
> > > If we took the else branch, then we definitely do not want to be calling
> > > g_free(buf).
> >
> > Doh!
> >
> > >
> > > >> }
> > > >>
> > > >> - qemu_iovec_to_buf(iov, 0, buf, bytes);
> > > >> -
> > > >> if (nfs_pwrite_async(client->context, client->fh,
> > > >> offset, bytes, buf,
> > > >> nfs_co_generic_cb, &task) != 0) {
> > > >> - g_free(buf);
> > > >> + if (my_buffer) {
> > > >> + g_free(buf);
> > > >> + }
> > >
> > > It looks correct as-is to me.
> >
> > Indeed.
> >
> > Reviewed-by: Jeff Cody <jcody@redhat.com>
>
> You gave R-b for both patches, but did not merge it - who is supposed to
> do that?
>
I am going to do it in my next round, I was just waiting to see if there
were any other comments.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] block/nfs optimizations
2017-02-17 16:38 [Qemu-devel] [PATCH 0/2] block/nfs optimizations Peter Lieven
2017-02-17 16:39 ` [Qemu-devel] [PATCH 1/2] block/nfs: convert to preadv / pwritev Peter Lieven
2017-02-17 16:39 ` [Qemu-devel] [PATCH 2/2] block/nfs: try to avoid the bounce buffer in pwritev Peter Lieven
@ 2017-02-24 3:50 ` Jeff Cody
2 siblings, 0 replies; 10+ messages in thread
From: Jeff Cody @ 2017-02-24 3:50 UTC (permalink / raw)
To: Peter Lieven; +Cc: qemu-devel, qemu-block, kwolf, mreitz
On Fri, Feb 17, 2017 at 05:38:59PM +0100, Peter Lieven wrote:
> Peter Lieven (2):
> block/nfs: convert to preadv / pwritev
> block/nfs: try to avoid the bounce buffer in pwritev
>
> block/nfs.c | 50 ++++++++++++++++++++++++++++----------------------
> 1 file changed, 28 insertions(+), 22 deletions(-)
>
> --
> 1.9.1
>
Thanks,
Applied to my block branch:
git://github.com/codyprime/qemu-kvm-jtc.git block
-Jeff
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-02-24 3:50 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-17 16:38 [Qemu-devel] [PATCH 0/2] block/nfs optimizations Peter Lieven
2017-02-17 16:39 ` [Qemu-devel] [PATCH 1/2] block/nfs: convert to preadv / pwritev Peter Lieven
2017-02-17 21:33 ` Jeff Cody
2017-02-17 16:39 ` [Qemu-devel] [PATCH 2/2] block/nfs: try to avoid the bounce buffer in pwritev Peter Lieven
2017-02-17 21:37 ` Jeff Cody
2017-02-17 21:42 ` Eric Blake
2017-02-17 21:51 ` Jeff Cody
2017-02-22 12:47 ` Kevin Wolf
2017-02-22 12:48 ` Jeff Cody
2017-02-24 3:50 ` [Qemu-devel] [PATCH 0/2] block/nfs optimizations Jeff Cody
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).