Make the implicit lock explicit, and hold it around blk_* operations. This allows us to relax the thread model for the filter to NBDKIT_THREAD_MODEL_PARALLEL. --- filters/cache/blk.h | 7 ++++++ filters/cache/cache.c | 57 +++++++++++++++++++++++++++++++------------ 2 files changed, 49 insertions(+), 15 deletions(-) diff --git a/filters/cache/blk.h b/filters/cache/blk.h index 24bf6a1..ab9134e 100644 --- a/filters/cache/blk.h +++ b/filters/cache/blk.h @@ -40,6 +40,13 @@ extern int blk_init (void); /* Close the cache, free the bitmap. */ extern void blk_free (void); +/*---------------------------------------------------------------------- + * ** NOTE ** + * + * An exclusive lock must be held when you call any function below + * this line. + */ + /* Allocate or resize the cache file and bitmap. */ extern int blk_set_size (uint64_t new_size); diff --git a/filters/cache/cache.c b/filters/cache/cache.c index 0d006bc..580e4f5 100644 --- a/filters/cache/cache.c +++ b/filters/cache/cache.c @@ -46,6 +46,8 @@ #include <sys/ioctl.h> #include <assert.h> +#include <pthread.h> + #ifdef HAVE_ALLOCA_H #include <alloca.h> #endif @@ -55,8 +57,12 @@ #include "cache.h" #include "blk.h" -/* XXX See design comment in filters/cow/cow.c. */ -#define THREAD_MODEL NBDKIT_THREAD_MODEL_SERIALIZE_ALL_REQUESTS +#define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL + +/* In order to handle parallel requests safely, this lock must be held + * when calling any blk_* functions. + */ +static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; enum cache_mode cache_mode = CACHE_MODE_WRITEBACK; bool cache_on_read = false; @@ -132,6 +138,7 @@ cache_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle) { int64_t size; + int r; size = next_ops->get_size (nxdata); if (size == -1) @@ -139,7 +146,10 @@ cache_get_size (struct nbdkit_next_ops *next_ops, void *nxdata, nbdkit_debug ("cache: underlying file size: %" PRIi64, size); - if (blk_set_size (size)) + pthread_mutex_lock (&lock); + r = blk_set_size (size); + pthread_mutex_unlock (&lock); + if (r == -1) return -1; return size; @@ -179,6 +189,7 @@ cache_pread (struct nbdkit_next_ops *next_ops, void *nxdata, while (count > 0) { uint64_t blknum, blkoffs, n; + int r; blknum = offset / BLKSIZE; /* block number */ blkoffs = offset % BLKSIZE; /* offset within the block */ @@ -186,7 +197,10 @@ cache_pread (struct nbdkit_next_ops *next_ops, void *nxdata, if (n > count) n = count; - if (blk_read (next_ops, nxdata, blknum, block, err) == -1) { + pthread_mutex_lock (&lock); + r = blk_read (next_ops, nxdata, blknum, block, err); + pthread_mutex_unlock (&lock); + if (r == -1) { free (block); return -1; } @@ -225,6 +239,7 @@ cache_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata, } while (count > 0) { uint64_t blknum, blkoffs, n; + int r; blknum = offset / BLKSIZE; /* block number */ blkoffs = offset % BLKSIZE; /* offset within the block */ @@ -232,13 +247,17 @@ cache_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata, if (n > count) n = count; - /* Do a read-modify-write operation on the current block. */ - if (blk_read (next_ops, nxdata, blknum, block, err) == -1){ - free (block); - return -1; + /* Do a read-modify-write operation on the current block. + * Hold the lock over the whole operation. + */ + pthread_mutex_lock (&lock); + r = blk_read (next_ops, nxdata, blknum, block, err); + if (r != -1) { + memcpy (&block[blkoffs], buf, n); + r = blk_write (next_ops, nxdata, blknum, block, flags, err); } - memcpy (&block[blkoffs], buf, n); - if (blk_write (next_ops, nxdata, blknum, block, flags, err) == -1) { + pthread_mutex_unlock (&lock); + if (r == -1) { free (block); return -1; } @@ -278,6 +297,7 @@ cache_zero (struct nbdkit_next_ops *next_ops, void *nxdata, } while (count > 0) { uint64_t blknum, blkoffs, n; + int r; blknum = offset / BLKSIZE; /* block number */ blkoffs = offset % BLKSIZE; /* offset within the block */ @@ -285,12 +305,17 @@ cache_zero (struct nbdkit_next_ops *next_ops, void *nxdata, if (n > count) n = count; - if (blk_read (next_ops, nxdata, blknum, block, err) == -1) { - free (block); - return -1; + /* Do a read-modify-write operation on the current block. + * Hold the lock over the whole operation. + */ + pthread_mutex_lock (&lock); + r = blk_read (next_ops, nxdata, blknum, block, err); + if (r != -1) { + memset (&block[blkoffs], 0, n); + r = blk_write (next_ops, nxdata, blknum, block, flags, err); } - memset (&block[blkoffs], 0, n); - if (blk_write (next_ops, nxdata, blknum, block, flags, err) == -1) { + pthread_mutex_unlock (&lock); + if (r == -1) { free (block); return -1; } @@ -342,7 +367,9 @@ cache_flush (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, * to be sure. Also we still need to issue the flush to the * underlying storage. */ + pthread_mutex_lock (&lock); for_each_dirty_block (flush_dirty_block, &data); + pthread_mutex_unlock (&lock); free (data.block); /* Now issue a flush request to the underlying storage. */ -- 2.19.2