From 0a34f342c3facace0767ff08f05532c9f161e305 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 22 Jun 2012 09:44:40 +0930 Subject: 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 --- lib/ntdb/free.c | 3 -- lib/ntdb/io.c | 80 ++++++++++++++++++++++++++++++++++++++++-------------- lib/ntdb/ntdb.c | 10 +++++-- lib/ntdb/open.c | 7 +++-- lib/ntdb/private.h | 19 ++++++++++--- 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 -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; -- cgit