* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.