On Sat, May 11, 2019 at 03:30:04PM -0500, Eric Blake wrote:
Although the time spent in memcpy/memset probably pales in
comparison
to time spent in socket I/O, it's still worth worth reducing the
number of times we have to utilize a bounce buffer when we already
have aligned data.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
filters/cache/cache.c | 60 ++++++++++++++++++++++++++++---------------
1 file changed, 39 insertions(+), 21 deletions(-)
diff --git a/filters/cache/cache.c b/filters/cache/cache.c
index 19ce555..98786b5 100644
--- a/filters/cache/cache.c
+++ b/filters/cache/cache.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2018 Red Hat Inc.
+ * Copyright (C) 2018-2019 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -58,6 +58,7 @@
#include "cache.h"
#include "blk.h"
#include "reclaim.h"
+#include "isaligned.h"
#define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL
@@ -233,11 +234,13 @@ cache_pread (struct nbdkit_next_ops *next_ops, void *nxdata,
CLEANUP_FREE uint8_t *block = NULL;
assert (!flags);
- block = malloc (blksize);
- if (block == NULL) {
- *err = errno;
- nbdkit_error ("malloc: %m");
- return -1;
+ if (!IS_ALIGNED (count | offset, blksize)) {
+ block = malloc (blksize);
+ if (block == NULL) {
+ *err = errno;
+ nbdkit_error ("malloc: %m");
+ return -1;
+ }
}
/* XXX This breaks up large read requests into smaller ones, which
@@ -258,12 +261,14 @@ cache_pread (struct nbdkit_next_ops *next_ops, void *nxdata,
{
ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
- r = blk_read (next_ops, nxdata, blknum, block, err);
+ r = blk_read (next_ops, nxdata, blknum,
+ blkoffs || n < blksize ? block : buf, err);
}
if (r == -1)
return -1;
- memcpy (buf, &block[blkoffs], n);
+ if (blkoffs || n < blksize)
+ memcpy (buf, &block[blkoffs], n);
I've stared at this patch for a while and I don't really understand
it.
One suggestion though. The patch might be better (or at least might
be more obviously correct) if the function defined a boolean at the
top which was used to tell if the bounce buffer was in use, something
like:
const bool use_bounce_buffer = /* condition */;
if (use_bounce_buffer) {
block = malloc (blksize);
...
}
r = blk_read (..., use_bounce_buffer ? block : buf, err);
etc.
Rich.
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
Read my programming and virtualization blog:
http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW