tdb: consolidate tdb allocation code - re-use dead records at hash top.

When in tdb_store we re-use a dead record reactivated from the
target hash chain itself, we currently leave it in its place in
the chain. When we re-use a dead record from a different chain or
from the freelist instead, we insert it at the beginning of the
target chain.

This patch changes the behaviour to always newly store a
record at the beginning of the hash chain. This removes
a special case and hence simplifies the allocation code.
On the other hand side, it introduces two additioal tdb_ofs_write
calls for the in-chain-case.

Note the subtelty of the patch that by moving the case of the candidate
record's chain as new case "i=0" into the for loop, we also reverse the
order of the two steps in the for-loop body (non blocking freelist alloc
and searching for dead record in a chain) in order to keep the overall
order of execution identical.

Signed-off-by: Michael Adam <obnox@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

Autobuild-User(master): Michael Adam <obnox@samba.org>
Autobuild-Date(master): Wed Apr  9 10:37:08 CEST 2014 on sn-devel-104

2 files changed, 13 insertions, 43 deletions

diff --git a/lib/tdb/common/freelist.c b/lib/tdb/common/freelist.c
index 6f8f812d416..2aeeb1c3832 100644
--- a/lib/tdb/common/freelist.c
+++ b/lib/tdb/common/freelist.c
@@ -404,22 +404,10 @@ tdb_off_t tdb_allocate(struct tdb_context *tdb, int hash, tdb_len_t length,
 	 * hash chains, something which is pretty bad normally)
 	 */
 
-	for (i=1; i<tdb->hash_size; i++) {
+	for (i=0; i<tdb->hash_size; i++) {
 
 		int list;
 
-		if (tdb_lock_nonblock(tdb, -1, F_WRLCK) == 0) {
-			/*
-			 * Under the freelist lock take the chance to give
-			 * back our dead records.
-			 */
-			tdb_purge_dead(tdb, hash);
-
-			ret = tdb_allocate_from_freelist(tdb, length, rec);
-			tdb_unlock(tdb, -1, F_WRLCK);
-			return ret;
-		}
-
 		list = BUCKET(hash+i);
 
 		if (tdb_lock_nonblock(tdb, list, F_WRLCK) == 0) {
@@ -432,6 +420,18 @@ tdb_off_t tdb_allocate(struct tdb_context *tdb, int hash, tdb_len_t length,
 				return ret;
 			}
 		}
+
+		if (tdb_lock_nonblock(tdb, -1, F_WRLCK) == 0) {
+			/*
+			 * Under the freelist lock take the chance to give
+			 * back our dead records.
+			 */
+			tdb_purge_dead(tdb, hash);
+
+			ret = tdb_allocate_from_freelist(tdb, length, rec);
+			tdb_unlock(tdb, -1, F_WRLCK);
+			return ret;
+		}
 	}
 
 blocking_freelist_allocate:
diff --git a/lib/tdb/common/tdb.c b/lib/tdb/common/tdb.c
index ba1c98edbe6..ebd4ffe3e01 100644
--- a/lib/tdb/common/tdb.c
+++ b/lib/tdb/common/tdb.c
@@ -520,36 +520,6 @@ static int _tdb_store(struct tdb_context *tdb, TDB_DATA key,
 	if (flag != TDB_INSERT)
 		tdb_delete_hash(tdb, key, hash);
 
-	if (tdb->max_dead_records != 0) {
-		tdb_off_t last_ptr;
-		/*
-		 * Allow for some dead records per hash chain, look if we can
-		 * find one that can hold the new record. We need enough space
-		 * for key, data and tailer. If we find one, we don't have to
-		 * consult the central freelist.
-		 */
-		rec_ptr = tdb_find_dead(tdb, hash, &rec,
-					key.dsize + dbuf.dsize,
-					&last_ptr);
-
-		if (rec_ptr != 0) {
-			rec.key_len = key.dsize;
-			rec.data_len = dbuf.dsize;
-			rec.full_hash = hash;
-			rec.magic = TDB_MAGIC;
-			if (tdb_rec_write(tdb, rec_ptr, &rec) == -1
-			    || tdb->methods->tdb_write(
-				    tdb, rec_ptr + sizeof(rec),
-				    key.dptr, key.dsize) == -1
-			    || tdb->methods->tdb_write(
-				    tdb, rec_ptr + sizeof(rec) + key.dsize,
-				    dbuf.dptr, dbuf.dsize) == -1) {
-				goto fail;
-			}
-			goto done;
-		}
-	}
-
 	/* we have to allocate some space */
 	rec_ptr = tdb_allocate(tdb, hash, key.dsize + dbuf.dsize, &rec);