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>

diff --git a/lib/ntdb/free.c b/lib/ntdb/free.c
index 971436f5a30b63bb67719ce1d767eaca1604a784..470c3765d90fb22213862a0adf77d9c7acd9c5fe 100644 (file)
--- 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 a54f1a661c86d65be162a49a7e3e4950facc4603..6749d282d91247e9064db61338ff256cac08edfb 100644 (file)
--- 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 ddbf7d6e9e180802d6a71588e8875c3dd26616a3..6c75915899bfeb881e5837567f5ae0f8bf914b04 100644 (file)
--- 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 56c97afe4331bb1db9a72f3c0df9724ebaf2bcbd..abec1172362df51357dbaf7bb4d41c95d28c82c2 100644 (file)
--- 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 90b782d303b7bb1decb09728acf07914e0051bc9..5efd2e0d92818d78620edf7f7551b5539a6da644 100644 (file)
--- 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;