ntdb: enhancement to allow direct access to the ntdb map during expansion.

This means keeping the old mmap around when we expand the database.
We could revert to read/write, except for platforms with incoherent
mmap (ie. OpenBSD), where we need to use mmap for all accesses.

Thus we keep a linked list of old maps, and unmap them when the last access
finally goes away.

This is required if we want ntdb_parse_record() callbacks to be able
to expand the database.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

5 files changed, 86 insertions, 33 deletions

diff --git a/lib/ntdb/free.c b/lib/ntdb/free.c
index 971436f5a3..470c3765d9 100644
--- a/lib/ntdb/free.c
+++ b/lib/ntdb/free.c
@@ -958,9 +958,6 @@ ntdb_off_t alloc(struct ntdb_context *ntdb, size_t keylen, size_t datalen,
 {
 	ntdb_off_t off;
 
-	/* We can't hold pointers during this: we could unmap! */
-	assert(!ntdb->direct_access);
-
 	for (;;) {
 		enum NTDB_ERROR ecode;
 		off = get_free(ntdb, keylen, datalen, growing, magic);
diff --git a/lib/ntdb/io.c b/lib/ntdb/io.c
index a54f1a661c..6749d282d9 100644
--- a/lib/ntdb/io.c
+++ b/lib/ntdb/io.c
@@ -28,17 +28,48 @@
 #include "private.h"
 #include <ccan/likely/likely.h>
 
-void ntdb_munmap(struct ntdb_file *file)
+static void free_old_mmaps(struct ntdb_context *ntdb)
 {
-	if (file->fd == -1)
-		return;
+	struct ntdb_old_mmap *i;
 
-	if (file->map_ptr) {
-		munmap(file->map_ptr, file->map_size);
-		file->map_ptr = NULL;
+	assert(ntdb->file->direct_count == 0);
+
+	while ((i = ntdb->file->old_mmaps) != NULL) {
+		ntdb->file->old_mmaps = i->next;
+		munmap(i->map_ptr, i->map_size);
+		ntdb->free_fn(i, ntdb->alloc_data);
 	}
 }
 
+enum NTDB_ERROR ntdb_munmap(struct ntdb_context *ntdb)
+{
+	if (ntdb->file->fd == -1) {
+		return NTDB_SUCCESS;
+	}
+
+	if (!ntdb->file->map_ptr) {
+		return NTDB_SUCCESS;
+	}
+
+	/* We can't unmap now if there are accessors. */
+	if (ntdb->file->direct_count) {
+		struct ntdb_old_mmap *old
+			= ntdb->alloc_fn(ntdb, sizeof(*old), ntdb->alloc_data);
+		if (!old) {
+			return ntdb_logerr(ntdb, NTDB_ERR_OOM, NTDB_LOG_ERROR,
+					   "ntdb_munmap alloc failed");
+		}
+		old->next = ntdb->file->old_mmaps;
+		old->map_ptr = ntdb->file->map_ptr;
+		old->map_size = ntdb->file->map_size;
+		ntdb->file->old_mmaps = old;
+	} else {
+		munmap(ntdb->file->map_ptr, ntdb->file->map_size);
+		ntdb->file->map_ptr = NULL;
+	}
+	return NTDB_SUCCESS;
+}
+
 enum NTDB_ERROR ntdb_mmap(struct ntdb_context *ntdb)
 {
 	int mmap_flags;
@@ -98,11 +129,6 @@ static enum NTDB_ERROR ntdb_normal_oob(struct ntdb_context *ntdb,
 	struct stat st;
 	enum NTDB_ERROR ecode;
 
-	/* We can't hold pointers during this: we could unmap! */
-	assert(!ntdb->direct_access
-	       || (ntdb->flags & NTDB_NOLOCK)
-	       || ntdb_has_expansion_lock(ntdb));
-
 	if (len + off < len) {
 		if (probe)
 			return NTDB_SUCCESS;
@@ -149,7 +175,10 @@ static enum NTDB_ERROR ntdb_normal_oob(struct ntdb_context *ntdb,
 	}
 
 	/* Unmap, update size, remap */
-	ntdb_munmap(ntdb->file);
+	ecode = ntdb_munmap(ntdb);
+	if (ecode) {
+		return ecode;
+	}
 
 	ntdb->file->map_size = st.st_size;
 	return ntdb_mmap(ntdb);
@@ -418,7 +447,10 @@ static enum NTDB_ERROR ntdb_expand_file(struct ntdb_context *ntdb,
 	} else {
 		/* Unmap before trying to write; old NTDB claimed OpenBSD had
 		 * problem with this otherwise. */
-		ntdb_munmap(ntdb->file);
+		ecode = ntdb_munmap(ntdb);
+		if (ecode) {
+			return ecode;
+		}
 
 		/* If this fails, we try to fill anyway. */
 		if (ftruncate(ntdb->file->fd, ntdb->file->map_size + addition))
@@ -461,8 +493,9 @@ const void *ntdb_access_read(struct ntdb_context *ntdb,
 		if (convert) {
 			ntdb_convert(ntdb, (void *)ret, len);
 		}
-	} else
-		ntdb->direct_access++;
+	} else {
+		ntdb->file->direct_count++;
+	}
 
 	return ret;
 }
@@ -500,9 +533,9 @@ void *ntdb_access_write(struct ntdb_context *ntdb,
 		ret = hdr + 1;
 		if (convert)
 			ntdb_convert(ntdb, (void *)ret, len);
-	} else
-		ntdb->direct_access++;
-
+	} else {
+		ntdb->file->direct_count++;
+	}
 	return ret;
 }
 
@@ -525,8 +558,11 @@ void ntdb_access_release(struct ntdb_context *ntdb, const void *p)
 		hdr = *hp;
 		*hp = hdr->next;
 		ntdb->free_fn(hdr, ntdb->alloc_data);
-	} else
-		ntdb->direct_access--;
+	} else {
+		if (--ntdb->file->direct_count == 0) {
+			free_old_mmaps(ntdb);
+		}
+	}
 }
 
 enum NTDB_ERROR ntdb_access_commit(struct ntdb_context *ntdb, void *p)
@@ -543,7 +579,9 @@ enum NTDB_ERROR ntdb_access_commit(struct ntdb_context *ntdb, void *p)
 		*hp = hdr->next;
 		ntdb->free_fn(hdr, ntdb->alloc_data);
 	} else {
-		ntdb->direct_access--;
+		if (--ntdb->file->direct_count == 0) {
+			free_old_mmaps(ntdb);
+		}
 		ecode = NTDB_SUCCESS;
 	}
 
diff --git a/lib/ntdb/ntdb.c b/lib/ntdb/ntdb.c
index ddbf7d6e9e..6c75915899 100644
--- a/lib/ntdb/ntdb.c
+++ b/lib/ntdb/ntdb.c
@@ -234,7 +234,7 @@ out:
 }
 
 _PUBLIC_ enum NTDB_ERROR ntdb_fetch(struct ntdb_context *ntdb, NTDB_DATA key,
-			 NTDB_DATA *data)
+				    NTDB_DATA *data)
 {
 	ntdb_off_t off;
 	struct ntdb_used_record rec;
@@ -353,9 +353,15 @@ _PUBLIC_ void ntdb_add_flag(struct ntdb_context *ntdb, unsigned flag)
 		ntdb->flags |= NTDB_NOLOCK;
 		break;
 	case NTDB_NOMMAP:
+		if (ntdb->file->direct_count) {
+			ntdb_logerr(ntdb, NTDB_ERR_EINVAL, NTDB_LOG_USE_ERROR,
+				    "ntdb_add_flag: Can't get NTDB_NOMMAP from"
+				    " ntdb_parse_record!");
+			return;
+		}
 		ntdb->flags |= NTDB_NOMMAP;
 #ifndef HAVE_INCOHERENT_MMAP
-		ntdb_munmap(ntdb->file);
+		ntdb_munmap(ntdb);
 #endif
 		break;
 	case NTDB_NOSYNC:
diff --git a/lib/ntdb/open.c b/lib/ntdb/open.c
index 56c97afe43..abec117236 100644
--- a/lib/ntdb/open.c
+++ b/lib/ntdb/open.c
@@ -99,7 +99,6 @@ static void ntdb_context_init(struct ntdb_context *ntdb)
 {
 	/* Initialize the NTDB fields here */
 	ntdb_io_init(ntdb);
-	ntdb->direct_access = 0;
 	ntdb->transaction = NULL;
 	ntdb->access = NULL;
 }
@@ -259,6 +258,8 @@ static enum NTDB_ERROR ntdb_new_file(struct ntdb_context *ntdb)
 	ntdb->file->allrecord_lock.count = 0;
 	ntdb->file->refcnt = 1;
 	ntdb->file->map_ptr = NULL;
+	ntdb->file->direct_count = 0;
+	ntdb->file->old_mmaps = NULL;
 	return NTDB_SUCCESS;
 }
 
@@ -841,7 +842,7 @@ fail_errno:
 					ntdb->free_fn(ntdb->file->map_ptr,
 						      ntdb->alloc_data);
 				} else
-					ntdb_munmap(ntdb->file);
+					ntdb_munmap(ntdb);
 			}
 			if (ntdb->file->fd != -1 && close(ntdb->file->fd) != 0)
 				ntdb_logerr(ntdb, NTDB_ERR_IO, NTDB_LOG_ERROR,
@@ -875,7 +876,7 @@ _PUBLIC_ int ntdb_close(struct ntdb_context *ntdb)
 				ntdb->free_fn(ntdb->file->map_ptr,
 					      ntdb->alloc_data);
 			} else {
-				ntdb_munmap(ntdb->file);
+				ntdb_munmap(ntdb);
 			}
 		}
 		ret = close(ntdb->file->fd);
diff --git a/lib/ntdb/private.h b/lib/ntdb/private.h
index 90b782d303..5efd2e0d92 100644
--- a/lib/ntdb/private.h
+++ b/lib/ntdb/private.h
@@ -301,6 +301,14 @@ struct ntdb_access_hdr {
 	bool convert;
 };
 
+/* mmaps we are keeping around because they are still direct accessed */
+struct ntdb_old_mmap {
+	struct ntdb_old_mmap *next;
+
+	void *map_ptr;
+	ntdb_len_t map_size;
+};
+
 struct ntdb_file {
 	/* How many are sharing us? */
 	unsigned int refcnt;
@@ -314,6 +322,12 @@ struct ntdb_file {
 	/* The file descriptor (-1 for NTDB_INTERNAL). */
 	int fd;
 
+	/* How many are accessing directly? */
+	unsigned int direct_count;
+
+	/* Old maps, still direct accessed. */
+	struct ntdb_old_mmap *old_mmaps;
+
 	/* Lock information */
 	pid_t locker;
 	struct ntdb_lock allrecord_lock;
@@ -429,7 +443,7 @@ void ntdb_io_init(struct ntdb_context *ntdb);
 void *ntdb_convert(const struct ntdb_context *ntdb, void *buf, ntdb_len_t size);
 
 /* Unmap and try to map the ntdb. */
-void ntdb_munmap(struct ntdb_file *file);
+enum NTDB_ERROR ntdb_munmap(struct ntdb_context *ntdb);
 enum NTDB_ERROR ntdb_mmap(struct ntdb_context *ntdb);
 
 /* Either alloc a copy, or give direct access.  Release frees or noop. */
@@ -576,9 +590,6 @@ struct ntdb_context {
 	enum NTDB_ERROR (*openhook)(int fd, void *data);
 	void *openhook_data;
 
-	/* Are we accessing directly? (debugging check). */
-	int direct_access;
-
 	/* Set if we are in a transaction. */
 	struct ntdb_transaction *transaction;