5:24 p.m.
Add a helper function get_real_size() to make it easier to add sanity
checking that mutex calls don't fail.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
I'm a bit unsure why truncate.c needs a lock anyway. I guess it's
because we are storing 'size' and 'real_size' as globals rather than
per-connection values in the handle, and we're worrying about parallel
connections where we don't want one thread reading inconsistent values
while another thread is in the middle of truncate_get_size()? At any
rate, as long as we don't have dynamic resize in the NBD protocol,
size can't change within the confines of a single connection. And even
if we DO assume that the underlying plugin reports different sizes for
different connections, our use of a global does not play well (if
client A sees size 1k, then client B sees size 2k, that changes client
A's use of 'real_size' to be something different than client A was
expecting).
So, I think we have to move 'size' and 'real_size' into the
per-connection handle, at which point, can't we just set them once
during truncate_prepare by moving the existing guts of
truncate_get_size there, and then letting truncate_get_size just
return h->size while all other truncate_* functions just access
h->real_size without a lock?
I guess I should look to see what other global filter state is better
tracked per-filter rather than globally?
---
filters/truncate/truncate.c | 41 ++++++++++++-------------------------
1 file changed, 13 insertions(+), 28 deletions(-)
diff --git a/filters/truncate/truncate.c b/filters/truncate/truncate.c
index 5f3370d..38745da 100644
--- a/filters/truncate/truncate.c
+++ b/filters/truncate/truncate.c
@@ -63,6 +63,13 @@ static uint64_t size;
/* This lock protects the real_size and size fields. */
static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
+static uint64_t
+get_real_size (void)
+{
+ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
+ return real_size;
+}
+
static int
parse_round_param (const char *key, const char *value, unsigned *ret)
{
@@ -147,7 +154,7 @@ truncate_get_size (struct nbdkit_next_ops *next_ops, void *nxdata,
if (r == -1)
return -1;
- pthread_mutex_lock (&lock);
+ ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock);
real_size = size = r;
@@ -163,8 +170,6 @@ truncate_get_size (struct nbdkit_next_ops *next_ops, void *nxdata,
size = ROUND_DOWN (size, round_down);
ret = size;
- pthread_mutex_unlock (&lock);
-
return ret;
}
@@ -176,11 +181,7 @@ truncate_pread (struct nbdkit_next_ops *next_ops, void *nxdata,
{
int r;
uint32_t n;
- uint64_t real_size_copy;
-
- pthread_mutex_lock (&lock);
- real_size_copy = real_size;
- pthread_mutex_unlock (&lock);
+ uint64_t real_size_copy = get_real_size ();
if (offset < real_size_copy) {
if (offset + count <= real_size_copy)
@@ -209,11 +210,7 @@ truncate_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata,
{
int r;
uint32_t n;
- uint64_t real_size_copy;
-
- pthread_mutex_lock (&lock);
- real_size_copy = real_size;
- pthread_mutex_unlock (&lock);
+ uint64_t real_size_copy = get_real_size ();
if (offset < real_size_copy) {
if (offset + count <= real_size_copy)
@@ -246,11 +243,7 @@ truncate_trim (struct nbdkit_next_ops *next_ops, void *nxdata,
uint32_t flags, int *err)
{
uint32_t n;
- uint64_t real_size_copy;
-
- pthread_mutex_lock (&lock);
- real_size_copy = real_size;
- pthread_mutex_unlock (&lock);
+ uint64_t real_size_copy = get_real_size ();
if (offset < real_size_copy) {
if (offset + count <= real_size_copy)
@@ -269,11 +262,7 @@ truncate_zero (struct nbdkit_next_ops *next_ops, void *nxdata,
uint32_t flags, int *err)
{
uint32_t n;
- uint64_t real_size_copy;
-
- pthread_mutex_lock (&lock);
- real_size_copy = real_size;
- pthread_mutex_unlock (&lock);
+ uint64_t real_size_copy = get_real_size ();
if (offset < real_size_copy) {
if (offset + count <= real_size_copy)
@@ -292,14 +281,10 @@ truncate_extents (struct nbdkit_next_ops *next_ops, void *nxdata,
uint32_t flags, struct nbdkit_extents *extents, int *err)
{
uint32_t n;
- uint64_t real_size_copy;
+ uint64_t real_size_copy = get_real_size ();
CLEANUP_EXTENTS_FREE struct nbdkit_extents *extents2 = NULL;
size_t i;
- pthread_mutex_lock (&lock);
- real_size_copy = real_size;
- pthread_mutex_unlock (&lock);
-
/* If the entire request is beyond the end of the underlying plugin
* then this is the easy case: return a hole up to the end of the
* file.
--
2.20.1