dovecot: mmaping works again properly. Changed to use private mm...

dovecot at dovecot.org dovecot at dovecot.org
Mon Jul 2 23:46:31 EEST 2007


details:   http://hg.dovecot.org/dovecot/rev/1b70ae186611
changeset: 5867:1b70ae186611
user:      Timo Sirainen <tss at iki.fi>
date:      Mon Jul 02 23:46:22 2007 +0300
description:
mmaping works again properly. Changed to use private mmaps which are
directly modified. The file is kept locked the whole time while it's being
mmaped, so multi-process updates may be slower than necessary.

diffstat:

9 files changed, 32 insertions(+), 82 deletions(-)
src/lib-index/mail-index-map.c            |   10 ++--
src/lib-index/mail-index-private.h        |    2 
src/lib-index/mail-index-sync-keywords.c  |    2 
src/lib-index/mail-index-sync-update.c    |   68 ++++++++---------------------
src/lib-index/mail-index-sync.c           |    6 --
src/lib-index/mail-index-view-sync.c      |   13 -----
src/lib-index/mail-index-view.c           |    6 +-
src/lib-index/mail-index.c                |    4 -
src/lib-index/mail-transaction-log-file.c |    3 -

diffs (truncated from 322 to 300 lines):

diff -r 092b9d3b7f97 -r 1b70ae186611 src/lib-index/mail-index-map.c
--- a/src/lib-index/mail-index-map.c	Mon Jul 02 23:24:34 2007 +0300
+++ b/src/lib-index/mail-index-map.c	Mon Jul 02 23:46:22 2007 +0300
@@ -375,8 +375,8 @@ static int mail_index_mmap(struct mail_i
 		return -1;
 	}
 
-	map->mmap_base =
-		mmap(NULL, file_size, PROT_READ, MAP_SHARED, index->fd, 0);
+	map->mmap_base = mmap(NULL, file_size, PROT_READ | PROT_WRITE,
+			      MAP_PRIVATE, index->fd, 0);
 	if (map->mmap_base == MAP_FAILED) {
 		map->mmap_base = NULL;
 		mail_index_set_syscall_error(index, "mmap()");
@@ -792,7 +792,11 @@ int mail_index_map_lock(struct mail_inde
 	if (map->lock_id != 0 || MAIL_INDEX_MAP_IS_IN_MEMORY(map))
 		return 0;
 
-	return mail_index_lock_shared(map->index, &map->lock_id);
+	if (mail_index_lock_shared(map->index, &map->lock_id) < 0)
+		return -1;
+
+	mail_index_map_copy_hdr(map, map->mmap_base);
+	return 0;
 }
 
 void mail_index_map_unlock(struct mail_index_map *map)
diff -r 092b9d3b7f97 -r 1b70ae186611 src/lib-index/mail-index-private.h
--- a/src/lib-index/mail-index-private.h	Mon Jul 02 23:24:34 2007 +0300
+++ b/src/lib-index/mail-index-private.h	Mon Jul 02 23:46:22 2007 +0300
@@ -13,7 +13,7 @@ struct mail_index_sync_map_ctx;
 struct mail_index_sync_map_ctx;
 
 /* How large index files to mmap() instead of reading to memory. */
-#define MAIL_INDEX_MMAP_MIN_SIZE (1024*256)
+#define MAIL_INDEX_MMAP_MIN_SIZE (1024*64)
 /* How many seconds to wait a lock for index file. */
 #define MAIL_INDEX_LOCK_SECS 120
 /* How many times to retry opening index files if read/fstat returns ESTALE.
diff -r 092b9d3b7f97 -r 1b70ae186611 src/lib-index/mail-index-sync-keywords.c
--- a/src/lib-index/mail-index-sync-keywords.c	Mon Jul 02 23:24:34 2007 +0300
+++ b/src/lib-index/mail-index-sync-keywords.c	Mon Jul 02 23:46:22 2007 +0300
@@ -214,7 +214,6 @@ keywords_update_records(struct mail_inde
 	if (seq1 == 0)
 		return 1;
 
-	mail_index_sync_move_to_private(ctx);
 	mail_index_sync_write_seq_update(ctx, seq1, seq2);
 
 	data_offset = keyword_idx / CHAR_BIT;
@@ -333,7 +332,6 @@ mail_index_sync_keywords_reset(struct ma
 		if (seq1 == 0)
 			continue;
 
-		mail_index_sync_move_to_private(ctx);
 		mail_index_sync_write_seq_update(ctx, seq1, seq2);
 		for (seq1--; seq1 < seq2; seq1++) {
 			rec = MAIL_INDEX_MAP_IDX(map, seq1);
diff -r 092b9d3b7f97 -r 1b70ae186611 src/lib-index/mail-index-sync-update.c
--- a/src/lib-index/mail-index-sync-update.c	Mon Jul 02 23:24:34 2007 +0300
+++ b/src/lib-index/mail-index-sync-update.c	Mon Jul 02 23:46:22 2007 +0300
@@ -44,31 +44,6 @@ mail_index_sync_update_log_offset(struct
 	map->hdr.log_file_head_offset = prev_offset;
 }
 
-#if 0 // FIXME: can we / do we want to support this?
-static int
-mail_index_map_msync(struct mail_index *index, struct mail_index_map *map)
-{
-	if (MAIL_INDEX_MAP_IS_IN_MEMORY(map)) {
-		buffer_write(map->hdr_copy_buf, 0, &map->hdr, sizeof(map->hdr));
-		return 0;
-	}
-
-	map->mmap_used_size = map->hdr.header_size +
-		map->records_count * map->hdr.record_size;
-
-	memcpy(map->mmap_base, &map->hdr,
-	       I_MIN(map->hdr.base_header_size, sizeof(map->hdr)));
-	memcpy(PTR_OFFSET(map->mmap_base, map->hdr.base_header_size),
-	       CONST_PTR_OFFSET(map->hdr_base, map->hdr.base_header_size),
-	       map->hdr.header_size - map->hdr.base_header_size);
-	if (msync(map->mmap_base, map->mmap_used_size, MS_SYNC) < 0) {
-		mail_index_set_syscall_error(index, "msync()");
-		return -1;
-	}
-	return 0;
-}
-#endif
-
 static void mail_index_sync_replace_map(struct mail_index_sync_map_ctx *ctx,
 					struct mail_index_map *map)
 {
@@ -77,13 +52,6 @@ static void mail_index_sync_replace_map(
 	i_assert(view->map != map);
 
 	mail_index_sync_update_log_offset(ctx, view->map, FALSE);
-#if 0 // FIXME
-	/* we could have already updated some of the records, so make sure
-	   that other views (in possibly other processes) will see this map's
-	   header in a valid state.  */
-	(void)mail_index_map_msync(view->index, view->map);
-#endif
-
 	mail_index_unmap(&view->map);
 	view->map = map;
 
@@ -95,19 +63,27 @@ void mail_index_sync_move_to_private(str
 {
 	struct mail_index_map *map = ctx->view->map;
 
-	if (map->refcount == 1) {
-		if (!MAIL_INDEX_MAP_IS_IN_MEMORY(map))
-			mail_index_map_move_to_memory(map);
-	} else {
+	i_assert(MAIL_INDEX_MAP_IS_IN_MEMORY(map) || map->lock_id != 0);
+
+	if (map->refcount > 1) {
 		map = mail_index_map_clone(map);
 		mail_index_sync_replace_map(ctx, map);
 	}
 }
 
+static void
+mail_index_sync_move_to_private_memory(struct mail_index_sync_map_ctx *ctx)
+{
+	mail_index_sync_move_to_private(ctx);
+
+	if (!MAIL_INDEX_MAP_IS_IN_MEMORY(ctx->view->map))
+		mail_index_map_move_to_memory(ctx->view->map);
+}
+
 struct mail_index_map *
 mail_index_sync_get_atomic_map(struct mail_index_sync_map_ctx *ctx)
 {
-	mail_index_sync_move_to_private(ctx);
+	mail_index_sync_move_to_private_memory(ctx);
 	ctx->view->map->write_atomic = TRUE;
 	return ctx->view->map;
 }
@@ -286,8 +262,6 @@ void mail_index_sync_write_seq_update(st
 {
 	struct mail_index_map *map = ctx->view->map;
 
-	i_assert(MAIL_INDEX_MAP_IS_IN_MEMORY(map));
-
 	if (map->write_seq_first == 0 ||
 	    map->write_seq_first > seq1)
 		map->write_seq_first = seq1;
@@ -313,7 +287,7 @@ static int sync_append(const struct mail
 	/* move to memory. the mapping is written when unlocking so we don't
 	   waste time re-mmap()ing multiple times or waste space growing index
 	   file too large */
-	mail_index_sync_move_to_private(ctx);
+	mail_index_sync_move_to_private_memory(ctx);
 	map = view->map;
 
 	/* don't rely on buffer->used being at the correct position.
@@ -358,7 +332,6 @@ static int sync_flag_update(const struct
 	if (seq1 == 0)
 		return 1;
 
-	mail_index_sync_move_to_private(ctx);
 	mail_index_sync_write_seq_update(ctx, seq1, seq2);
 
 	hdr = &view->map->hdr;
@@ -824,13 +797,12 @@ int mail_index_sync_map(struct mail_inde
 	   reading. */
 	map->hdr.log_file_tail_offset = index->log->head->max_tail_offset;
 
-	if (map->write_base_header) {
-		i_assert(MAIL_INDEX_MAP_IS_IN_MEMORY(map));
-		buffer_write(map->hdr_copy_buf, 0, &map->hdr, sizeof(map->hdr));
-	}
-
-	/*FIXME:if (mail_index_map_msync(index, map) < 0)
-		ret = -1;*/
+	buffer_write(map->hdr_copy_buf, 0, &map->hdr, sizeof(map->hdr));
+	if (!MAIL_INDEX_MAP_IS_IN_MEMORY(map)) {
+		memcpy(map->mmap_base, map->hdr_copy_buf->data,
+		       map->hdr_copy_buf->used);
+	}
+
 	if (sync_map_ctx.errors) {
 		/* avoid the same syncing errors the next time */
 		mail_index_write(index, FALSE);
diff -r 092b9d3b7f97 -r 1b70ae186611 src/lib-index/mail-index-sync.c
--- a/src/lib-index/mail-index-sync.c	Mon Jul 02 23:24:34 2007 +0300
+++ b/src/lib-index/mail-index-sync.c	Mon Jul 02 23:46:22 2007 +0300
@@ -351,13 +351,11 @@ int mail_index_sync_begin(struct mail_in
 	   mail_index_sync_commit(). */
 	if ((ret = mail_index_map(index, MAIL_INDEX_SYNC_HANDLER_HEAD)) <= 0) {
 		if (ret == 0 || mail_index_fsck(index) <= 0) {
-			mail_index_map_unlock(index->map);
 			mail_transaction_log_sync_unlock(index->log);
 			return -1;
 		}
 		/* let's try again */
 		if (mail_index_map(index, MAIL_INDEX_SYNC_HANDLER_HEAD) <= 0) {
-			mail_index_map_unlock(index->map);
 			mail_transaction_log_sync_unlock(index->log);
 			return -1;
 		}
@@ -366,7 +364,6 @@ int mail_index_sync_begin(struct mail_in
 
 	if (!mail_index_need_sync(index, hdr, flags,
 				  log_file_seq, log_file_offset)) {
-		mail_index_map_unlock(index->map);
 		mail_transaction_log_sync_unlock(index->log);
 		return 0;
 	}
@@ -379,7 +376,6 @@ int mail_index_sync_begin(struct mail_in
 			"broken sync positions in index file %s",
 			index->filepath);
 		if (mail_index_fsck(index) <= 0) {
-			mail_index_map_unlock(index->map);
 			mail_transaction_log_sync_unlock(index->log);
 			return -1;
 		}
@@ -404,7 +400,6 @@ int mail_index_sync_begin(struct mail_in
 		   to skip over it. fix the problem with fsck and try again. */
 		mail_index_sync_rollback(&ctx);
 		if (mail_index_fsck(index) <= 0) {
-			mail_index_map_unlock(index->map);
 			mail_transaction_log_sync_unlock(index->log);
 			return -1;
 		}
@@ -574,7 +569,6 @@ static void mail_index_sync_end(struct m
 
 	*_ctx = NULL;
 
-	mail_index_map_unlock(ctx->index->map);
 	mail_transaction_log_sync_unlock(ctx->index->log);
 
 	mail_index_view_close(&ctx->view);
diff -r 092b9d3b7f97 -r 1b70ae186611 src/lib-index/mail-index-view-sync.c
--- a/src/lib-index/mail-index-view-sync.c	Mon Jul 02 23:24:34 2007 +0300
+++ b/src/lib-index/mail-index-view-sync.c	Mon Jul 02 23:46:22 2007 +0300
@@ -299,19 +299,6 @@ int mail_index_view_sync_begin(struct ma
 			/* Using non-head mapping. We have to apply
 			   transactions to it to get latest changes into it. */
 			ctx->sync_map_update = TRUE;
-		}
-
-		/* Unless map was synced at the exact same position as
-		   view, the message flags can't be reliably used to
-		   update flag counters. note that map->hdr may contain
-		   old information if another process updated the
-		   index file since. */
-		if (view->map->mmap_base != NULL) {
-			// FIXME: locking should do this..?..
-			const struct mail_index_header *hdr;
-
-			hdr = view->map->mmap_base;
-			view->map->hdr = *hdr;
 		}
 
 		if (view->map->refcount > 1) {
diff -r 092b9d3b7f97 -r 1b70ae186611 src/lib-index/mail-index-view.c
--- a/src/lib-index/mail-index-view.c	Mon Jul 02 23:24:34 2007 +0300
+++ b/src/lib-index/mail-index-view.c	Mon Jul 02 23:46:22 2007 +0300
@@ -76,8 +76,10 @@ void mail_index_view_unlock(struct mail_
 	mail_index_view_check_nextuid(view);
 #endif
 
-	mail_index_map_unlock(view->map);
-	mail_index_map_unlock(view->index->map);
+	/* currently this is a no-op. if we unlock any maps, they might get
+	   changed and then it's unspecified what parts of the memory mapping
+	   are up-to-date. we could also copy the map to memory always, but
+	   that kinds of defeats the purpose of mmaps. */
 }
 
 bool mail_index_view_is_inconsistent(struct mail_index_view *view)
diff -r 092b9d3b7f97 -r 1b70ae186611 src/lib-index/mail-index.c
--- a/src/lib-index/mail-index.c	Mon Jul 02 23:24:34 2007 +0300
+++ b/src/lib-index/mail-index.c	Mon Jul 02 23:46:22 2007 +0300
@@ -360,7 +360,6 @@ mail_index_try_open(struct mail_index *i
 
 	i_assert(index->map == NULL || index->map->lock_id == 0);
 	ret = mail_index_map(index, MAIL_INDEX_SYNC_HANDLER_HEAD);
-	mail_index_map_unlock(index->map);
 	if (ret == 0) {
 		/* it's corrupted - recreate it */
 		if (index->fd != -1) {
@@ -582,7 +581,6 @@ int mail_index_reopen_if_changed(struct 
 
 int mail_index_refresh(struct mail_index *index)
 {
-	bool locked = index->map->lock_id != 0;
 	int ret;
 
 	if (MAIL_INDEX_IS_IN_MEMORY(index))
@@ -595,8 +593,6 @@ int mail_index_refresh(struct mail_index
 	}
 
 	ret = mail_index_map(index, MAIL_INDEX_SYNC_HANDLER_HEAD);
-	if (!locked)
-		mail_index_map_unlock(index->map);
 	return ret <= 0 ? -1 : 0;


More information about the dovecot-cvs mailing list