* [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
* 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
* [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 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.