]> git.ozlabs.org Git - ccan/blobdiff - ccan/tdb2/free.c
tdb2: don't start again when we coalesce a record.
[ccan] / ccan / tdb2 / free.c
index 9ea8efd1134542b1dc0a92a1eb2d0d43364210bc..cd9a332abe425407cf27b5e35a49db912e3a92b3 100644 (file)
@@ -103,55 +103,96 @@ static tdb_off_t find_free_head(struct tdb_context *tdb,
                                    bucket, TDB_FREE_BUCKETS);
 }
 
+static void check_list(struct tdb_context *tdb, tdb_off_t b_off)
+{
+#ifdef CCAN_TDB2_DEBUG
+       tdb_off_t off, prev = 0, first;
+       struct tdb_free_record r;
+
+       first = off = tdb_read_off(tdb, b_off);
+       while (off != 0) {
+               tdb_read_convert(tdb, off, &r, sizeof(r));
+               if (frec_magic(&r) != TDB_FREE_MAGIC)
+                       abort();
+               if (prev && frec_prev(&r) != prev)
+                       abort();
+               prev = off;
+               off = r.next;
+       }
+
+       if (first) {
+               tdb_read_convert(tdb, first, &r, sizeof(r));
+               if (frec_prev(&r) != prev)
+                       abort();
+       }
+#endif
+}
+
 /* Remove from free bucket. */
 static enum TDB_ERROR remove_from_list(struct tdb_context *tdb,
                                       tdb_off_t b_off, tdb_off_t r_off,
                                       const struct tdb_free_record *r)
 {
-       tdb_off_t off;
+       tdb_off_t off, prev_next, head;
        enum TDB_ERROR ecode;
 
-       /* Front of list? */
-       if (frec_prev(r) == 0) {
-               off = b_off;
-       } else {
-               off = frec_prev(r) + offsetof(struct tdb_free_record, next);
-       }
-
-#ifdef CCAN_TDB2_DEBUG
-       if (tdb_read_off(tdb, off) != r_off) {
-               return tdb_logerr(tdb, TDB_ERR_CORRUPT, TDB_LOG_ERROR,
-                                 "remove_from_list:"
-                                 " %llu bad prev in list %llu",
-                                 (long long)r_off, (long long)b_off);
-       }
-#endif
+       /* Is this only element in list?  Zero out bucket, and we're done. */
+       if (frec_prev(r) == r_off)
+               return tdb_write_off(tdb, b_off, 0);
 
-       /* r->prev->next = r->next */
-       ecode = tdb_write_off(tdb, off, r->next);
-       if (ecode != TDB_SUCCESS) {
-               return ecode;
-       }
+       /* off = &r->prev->next */
+       off = frec_prev(r) + offsetof(struct tdb_free_record, next);
 
-       if (r->next != 0) {
-               off = r->next + offsetof(struct tdb_free_record,magic_and_prev);
-               /* r->next->prev = r->prev */
+       /* Get prev->next */
+       prev_next = tdb_read_off(tdb, off);
+       if (TDB_OFF_IS_ERR(prev_next))
+               return prev_next;
 
+       /* If prev->next == 0, we were head: update bucket to point to next. */
+       if (prev_next == 0) {
 #ifdef CCAN_TDB2_DEBUG
-               if (tdb_read_off(tdb, off) & TDB_OFF_MASK != r_off) {
+               if (tdb_read_off(tdb, b_off) != r_off) {
                        return tdb_logerr(tdb, TDB_ERR_CORRUPT, TDB_LOG_ERROR,
                                          "remove_from_list:"
-                                         " %llu bad list %llu",
-                                         (long long)r_off, (long long)b_off);
+                                         " %llu head %llu on list %llu",
+                                         (long long)r_off,
+                                         (long long)tdb_read_off(tdb, b_off),
+                                         (long long)b_off);
                }
 #endif
-
-               ecode = tdb_write_off(tdb, off, r->magic_and_prev);
-               if (ecode != TDB_SUCCESS) {
+               ecode = tdb_write_off(tdb, b_off, r->next);
+               if (ecode != TDB_SUCCESS)
+                       return ecode;
+       } else {
+               /* r->prev->next = r->next */
+               ecode = tdb_write_off(tdb, off, r->next);
+               if (ecode != TDB_SUCCESS)
                        return ecode;
-               }
        }
-       return TDB_SUCCESS;
+
+       /* If we were the tail, off = &head->prev. */
+       if (r->next == 0) {
+               head = tdb_read_off(tdb, b_off);
+               if (TDB_OFF_IS_ERR(head))
+                       return head;
+               off = head + offsetof(struct tdb_free_record, magic_and_prev);
+       } else {
+               /* off = &r->next->prev */
+               off = r->next + offsetof(struct tdb_free_record,
+                                        magic_and_prev);
+       }
+
+#ifdef CCAN_TDB2_DEBUG
+       /* *off == r */
+       if ((tdb_read_off(tdb, off) & TDB_OFF_MASK) != r_off) {
+               return tdb_logerr(tdb, TDB_ERR_CORRUPT, TDB_LOG_ERROR,
+                                 "remove_from_list:"
+                                 " %llu bad prev in list %llu",
+                                 (long long)r_off, (long long)b_off);
+       }
+#endif
+       /* r->next->prev = r->prev */
+       return tdb_write_off(tdb, off, r->magic_and_prev);
 }
 
 /* Enqueue in this free bucket. */
@@ -162,13 +203,12 @@ static enum TDB_ERROR enqueue_in_free(struct tdb_context *tdb,
 {
        struct tdb_free_record new;
        enum TDB_ERROR ecode;
+       tdb_off_t prev;
        uint64_t magic = (TDB_FREE_MAGIC << (64 - TDB_OFF_UPPER_STEAL));
 
        /* We only need to set ftable_and_len; rest is set in enqueue_in_free */
        new.ftable_and_len = ((uint64_t)tdb->ftable << (64 - TDB_OFF_UPPER_STEAL))
                | len;
-       /* prev = 0. */
-       new.magic_and_prev = magic;
 
        /* new->next = head. */
        new.next = tdb_read_off(tdb, b_off);
@@ -176,19 +216,22 @@ static enum TDB_ERROR enqueue_in_free(struct tdb_context *tdb,
                return new.next;
        }
 
-       if (new.next) {
-#ifdef CCAN_TDB2_DEBUG
-               if (tdb_read_off(tdb,
-                                new.next + offsetof(struct tdb_free_record,
-                                                    magic_and_prev))
-                   != magic) {
+       /* First element?  Prev points to ourselves. */
+       if (!new.next) {
+               new.magic_and_prev = (magic | off);
+       } else {
+               /* new->prev = next->prev */
+               prev = tdb_read_off(tdb,
+                                   new.next + offsetof(struct tdb_free_record,
+                                                       magic_and_prev));
+               new.magic_and_prev = prev;
+               if (frec_magic(&new) != TDB_FREE_MAGIC) {
                        return tdb_logerr(tdb, TDB_ERR_CORRUPT, TDB_LOG_ERROR,
                                          "enqueue_in_free: %llu bad head"
                                          " prev %llu",
                                          (long long)new.next,
-                                         (long long)b_off);
+                                         (long long)prev);
                }
-#endif
                /* next->prev = new. */
                ecode = tdb_write_off(tdb, new.next
                                      + offsetof(struct tdb_free_record,
@@ -197,6 +240,20 @@ static enum TDB_ERROR enqueue_in_free(struct tdb_context *tdb,
                if (ecode != TDB_SUCCESS) {
                        return ecode;
                }
+
+#ifdef CCAN_TDB2_DEBUG
+               prev = tdb_read_off(tdb, frec_prev(&new)
+                                   + offsetof(struct tdb_free_record, next));
+               if (prev != 0) {
+                       return tdb_logerr(tdb, TDB_ERR_CORRUPT, TDB_LOG_ERROR,
+                                         "enqueue_in_free:"
+                                         " %llu bad tail next ptr %llu",
+                                         (long long)frec_prev(&new)
+                                         + offsetof(struct tdb_free_record,
+                                                    next),
+                                         (long long)prev);
+               }
+#endif
        }
        /* head = new */
        ecode = tdb_write_off(tdb, b_off, off);
@@ -209,7 +266,8 @@ static enum TDB_ERROR enqueue_in_free(struct tdb_context *tdb,
 
 /* List need not be locked. */
 enum TDB_ERROR add_free_record(struct tdb_context *tdb,
-                              tdb_off_t off, tdb_len_t len_with_header)
+                              tdb_off_t off, tdb_len_t len_with_header,
+                              enum tdb_lock_flags waitflag)
 {
        tdb_off_t b_off;
        tdb_len_t len;
@@ -220,12 +278,13 @@ enum TDB_ERROR add_free_record(struct tdb_context *tdb,
        len = len_with_header - sizeof(struct tdb_used_record);
 
        b_off = bucket_off(tdb->ftable_off, size_to_bucket(len));
-       ecode = tdb_lock_free_bucket(tdb, b_off, TDB_LOCK_WAIT);
+       ecode = tdb_lock_free_bucket(tdb, b_off, waitflag);
        if (ecode != TDB_SUCCESS) {
                return ecode;
        }
 
        ecode = enqueue_in_free(tdb, b_off, off, len);
+       check_list(tdb, b_off);
        tdb_unlock_free_bucket(tdb, b_off);
        return ecode;
 }
@@ -275,19 +334,22 @@ static tdb_off_t ftable_offset(struct tdb_context *tdb, unsigned int ftable)
        return off;
 }
 
-/* Note: we unlock the current bucket if we coalesce or fail. */
-static tdb_bool_err coalesce(struct tdb_context *tdb,
-                            tdb_off_t off, tdb_off_t b_off,
-                            tdb_len_t data_len)
+/* Note: we unlock the current bucket if fail (-ve), or coalesce (-ve) and
+ * need to blatt either of the *protect records (which is set to an error). */
+static tdb_len_t coalesce(struct tdb_context *tdb,
+                         tdb_off_t off, tdb_off_t b_off,
+                         tdb_len_t data_len,
+                         tdb_off_t *protect1,
+                         tdb_off_t *protect2)
 {
        tdb_off_t end;
        struct tdb_free_record rec;
        enum TDB_ERROR ecode;
 
-       add_stat(tdb, alloc_coalesce_tried, 1);
+       tdb->stats.alloc_coalesce_tried++;
        end = off + sizeof(struct tdb_used_record) + data_len;
 
-       while (end < tdb->map_size) {
+       while (end < tdb->file->map_size) {
                const struct tdb_free_record *r;
                tdb_off_t nb_off;
                unsigned ftable, bucket;
@@ -318,7 +380,7 @@ static tdb_bool_err coalesce(struct tdb_context *tdb,
                /* We may be violating lock order here, so best effort. */
                if (tdb_lock_free_bucket(tdb, nb_off, TDB_LOCK_NOWAIT)
                    != TDB_SUCCESS) {
-                       add_stat(tdb, alloc_coalesce_lockfail, 1);
+                       tdb->stats.alloc_coalesce_lockfail++;
                        break;
                }
 
@@ -330,19 +392,24 @@ static tdb_bool_err coalesce(struct tdb_context *tdb,
                }
 
                if (unlikely(frec_magic(&rec) != TDB_FREE_MAGIC)) {
-                       add_stat(tdb, alloc_coalesce_race, 1);
+                       tdb->stats.alloc_coalesce_race++;
                        tdb_unlock_free_bucket(tdb, nb_off);
                        break;
                }
 
                if (unlikely(frec_ftable(&rec) != ftable)
                    || unlikely(size_to_bucket(frec_len(&rec)) != bucket)) {
-                       add_stat(tdb, alloc_coalesce_race, 1);
+                       tdb->stats.alloc_coalesce_race++;
                        tdb_unlock_free_bucket(tdb, nb_off);
                        break;
                }
 
+               /* Did we just mess up a record you were hoping to use? */
+               if (end == *protect1 || end == *protect2)
+                       *protect1 = TDB_ERR_NOEXIST;
+
                ecode = remove_from_list(tdb, nb_off, end, &rec);
+               check_list(tdb, nb_off);
                if (ecode != TDB_SUCCESS) {
                        tdb_unlock_free_bucket(tdb, nb_off);
                        goto err;
@@ -350,12 +417,16 @@ static tdb_bool_err coalesce(struct tdb_context *tdb,
 
                end += sizeof(struct tdb_used_record) + frec_len(&rec);
                tdb_unlock_free_bucket(tdb, nb_off);
-               add_stat(tdb, alloc_coalesce_num_merged, 1);
+               tdb->stats.alloc_coalesce_num_merged++;
        }
 
        /* Didn't find any adjacent free? */
        if (end == off + sizeof(struct tdb_used_record) + data_len)
-               return false;
+               return 0;
+
+       /* Before we expand, check this isn't one you wanted protected? */
+       if (off == *protect1 || off == *protect2)
+               *protect1 = TDB_ERR_EXISTS;
 
        /* OK, expand initial record */
        ecode = tdb_read_convert(tdb, off, &rec, sizeof(rec));
@@ -371,29 +442,44 @@ static tdb_bool_err coalesce(struct tdb_context *tdb,
        }
 
        ecode = remove_from_list(tdb, b_off, off, &rec);
+       check_list(tdb, b_off);
        if (ecode != TDB_SUCCESS) {
                goto err;
        }
 
-       /* We have to drop this to avoid deadlocks, so make sure record
-        * doesn't get coalesced by someone else! */
-       rec.ftable_and_len = (TDB_FTABLE_NONE << (64 - TDB_OFF_UPPER_STEAL))
-               | (end - off - sizeof(struct tdb_used_record));
-       ecode = tdb_write_off(tdb, off + offsetof(struct tdb_free_record,
-                                                 ftable_and_len),
-                             rec.ftable_and_len);
+       /* Try locking violation first... */
+       ecode = add_free_record(tdb, off, end - off, TDB_LOCK_NOWAIT);
        if (ecode != TDB_SUCCESS) {
-               goto err;
-       }
+               /* Need to drop lock.  Can't rely on anything stable. */
+               *protect1 = TDB_ERR_CORRUPT;
+
+               /* We have to drop this to avoid deadlocks, so make sure record
+                * doesn't get coalesced by someone else! */
+               rec.ftable_and_len = (TDB_FTABLE_NONE
+                                     << (64 - TDB_OFF_UPPER_STEAL))
+                       | (end - off - sizeof(struct tdb_used_record));
+               ecode = tdb_write_off(tdb,
+                                     off + offsetof(struct tdb_free_record,
+                                                    ftable_and_len),
+                                     rec.ftable_and_len);
+               if (ecode != TDB_SUCCESS) {
+                       goto err;
+               }
 
-       add_stat(tdb, alloc_coalesce_succeeded, 1);
-       tdb_unlock_free_bucket(tdb, b_off);
+               tdb->stats.alloc_coalesce_succeeded++;
+               tdb_unlock_free_bucket(tdb, b_off);
 
-       ecode = add_free_record(tdb, off, end - off);
-       if (ecode != TDB_SUCCESS) {
-               return ecode;
+               ecode = add_free_record(tdb, off, end - off, TDB_LOCK_WAIT);
+               if (ecode != TDB_SUCCESS) {
+                       return ecode;
+               }
+       } else if (TDB_OFF_IS_ERR(*protect1)) {
+               /* For simplicity, we always drop lock if they can't continue */
+               tdb_unlock_free_bucket(tdb, b_off);
        }
-       return true;
+
+       /* Return usable length. */
+       return end - off - sizeof(struct tdb_used_record);
 
 err:
        /* To unify error paths, we *always* unlock bucket on error. */
@@ -413,10 +499,11 @@ static tdb_off_t lock_and_alloc(struct tdb_context *tdb,
        tdb_off_t off, b_off,best_off;
        struct tdb_free_record best = { 0 };
        double multiplier;
+       bool coalesce_after_best = false; /* Damn GCC warning! */
        size_t size = adjust_size(keylen, datalen);
        enum TDB_ERROR ecode;
 
-       add_stat(tdb, allocs, 1);
+       tdb->stats.allocs++;
 again:
        b_off = bucket_off(ftable_off, bucket);
 
@@ -446,9 +533,8 @@ again:
 
        while (off) {
                const struct tdb_free_record *r;
-               tdb_len_t len;
+               tdb_len_t len, coal;
                tdb_off_t next;
-               int coal;
 
                r = tdb_access_read(tdb, off, sizeof(*r), true);
                if (TDB_PTR_IS_ERR(r)) {
@@ -457,21 +543,22 @@ again:
                }
 
                if (frec_magic(r) != TDB_FREE_MAGIC) {
-                       tdb_access_release(tdb, r);
                        ecode = tdb_logerr(tdb, TDB_ERR_CORRUPT, TDB_LOG_ERROR,
                                           "lock_and_alloc:"
                                           " %llu non-free 0x%llx",
                                           (long long)off,
                                           (long long)r->magic_and_prev);
+                       tdb_access_release(tdb, r);
                        goto unlock_err;
                }
 
                if (frec_len(r) >= size && frec_len(r) < frec_len(&best)) {
                        best_off = off;
                        best = *r;
+                       coalesce_after_best = false;
                }
 
-               if (frec_len(&best) < size * multiplier && best_off) {
+               if (frec_len(&best) <= size * multiplier && best_off) {
                        tdb_access_release(tdb, r);
                        break;
                }
@@ -483,15 +570,17 @@ again:
                tdb_access_release(tdb, r);
 
                /* Since we're going slow anyway, try coalescing here. */
-               coal = coalesce(tdb, off, b_off, len);
-               if (coal == 1) {
-                       /* This has unlocked list, restart. */
-                       goto again;
-               }
-               if (coal < 0) {
+               coal = coalesce(tdb, off, b_off, len, &best_off, &next);
+               if (TDB_OFF_IS_ERR(coal)) {
                        /* This has already unlocked on error. */
                        return coal;
                }
+               if (TDB_OFF_IS_ERR(best_off)) {
+                       /* This has unlocked list, restart. */
+                       goto again;
+               }
+               if (coal > 0)
+                       coalesce_after_best = true;
                off = next;
        }
 
@@ -500,8 +589,17 @@ again:
                struct tdb_used_record rec;
                size_t leftover;
 
+               /* If we coalesced, we might have change prev/next ptrs. */
+               if (coalesce_after_best) {
+                       ecode = tdb_read_convert(tdb, best_off, &best,
+                                                sizeof(best));
+                       if (ecode != TDB_SUCCESS)
+                               goto unlock_err;
+               }
+
                /* We're happy with this size: take it. */
                ecode = remove_from_list(tdb, b_off, best_off, &best);
+               check_list(tdb, b_off);
                if (ecode != TDB_SUCCESS) {
                        goto unlock_err;
                }
@@ -535,11 +633,11 @@ again:
                /* Bucket of leftover will be <= current bucket, so nested
                 * locking is allowed. */
                if (leftover) {
-                       add_stat(tdb, alloc_leftover, 1);
+                       tdb->stats.alloc_leftover++;
                        ecode = add_free_record(tdb,
                                                best_off + sizeof(rec)
                                                + frec_len(&best) - leftover,
-                                               leftover);
+                                               leftover, TDB_LOCK_WAIT);
                        if (ecode != TDB_SUCCESS) {
                                best_off = ecode;
                        }
@@ -588,9 +686,9 @@ static tdb_off_t get_free(struct tdb_context *tdb,
                                return off;
                        if (off != 0) {
                                if (b == start_b)
-                                       add_stat(tdb, alloc_bucket_exact, 1);
+                                       tdb->stats.alloc_bucket_exact++;
                                if (b == TDB_FREE_BUCKETS - 1)
-                                       add_stat(tdb, alloc_bucket_max, 1);
+                                       tdb->stats.alloc_bucket_max++;
                                /* Worked?  Stay using this list. */
                                tdb->ftable_off = ftable_off;
                                tdb->ftable = ftable;
@@ -652,13 +750,10 @@ enum TDB_ERROR set_header(struct tdb_context *tdb,
 /* Expand the database. */
 static enum TDB_ERROR tdb_expand(struct tdb_context *tdb, tdb_len_t size)
 {
-       uint64_t old_size;
+       uint64_t old_size, rec_size, map_size;
        tdb_len_t wanted;
        enum TDB_ERROR ecode;
 
-       /* We need room for the record header too. */
-       wanted = sizeof(struct tdb_used_record) + size;
-
        /* Need to hold a hash lock to expand DB: transactions rely on it. */
        if (!(tdb->flags & TDB_NOLOCK)
            && !tdb->file->allrecord_lock.count && !tdb_has_hash_locks(tdb)) {
@@ -666,14 +761,6 @@ static enum TDB_ERROR tdb_expand(struct tdb_context *tdb, tdb_len_t size)
                                  "tdb_expand: must hold lock during expand");
        }
 
-       /* always make room for at least 100 more records, and at
-           least 25% more space. */
-       if (size * TDB_EXTENSION_FACTOR > tdb->map_size / 4)
-               wanted = size * TDB_EXTENSION_FACTOR;
-       else
-               wanted = tdb->map_size / 4;
-       wanted = adjust_size(0, wanted);
-
        /* Only one person can expand file at a time. */
        ecode = tdb_lock_expand(tdb, F_WRLCK);
        if (ecode != TDB_SUCCESS) {
@@ -681,13 +768,39 @@ static enum TDB_ERROR tdb_expand(struct tdb_context *tdb, tdb_len_t size)
        }
 
        /* Someone else may have expanded the file, so retry. */
-       old_size = tdb->map_size;
-       tdb->methods->oob(tdb, tdb->map_size + 1, true);
-       if (tdb->map_size != old_size) {
+       old_size = tdb->file->map_size;
+       tdb->methods->oob(tdb, tdb->file->map_size + 1, true);
+       if (tdb->file->map_size != old_size) {
                tdb_unlock_expand(tdb, F_WRLCK);
                return TDB_SUCCESS;
        }
 
+       /* limit size in order to avoid using up huge amounts of memory for
+        * in memory tdbs if an oddball huge record creeps in */
+       if (size > 100 * 1024) {
+               rec_size = size * 2;
+       } else {
+               rec_size = size * 100;
+       }
+
+       /* always make room for at least rec_size more records, and at
+          least 25% more space. if the DB is smaller than 100MiB,
+          otherwise grow it by 10% only. */
+       if (old_size > 100 * 1024 * 1024) {
+               map_size = old_size / 10;
+       } else {
+               map_size = old_size / 4;
+       }
+
+       if (map_size > rec_size) {
+               wanted = map_size;
+       } else {
+               wanted = rec_size;
+       }
+
+       /* We need room for the record header too. */
+       wanted = adjust_size(0, sizeof(struct tdb_used_record) + wanted);
+
        ecode = tdb->methods->expand_file(tdb, wanted);
        if (ecode != TDB_SUCCESS) {
                tdb_unlock_expand(tdb, F_WRLCK);
@@ -697,8 +810,8 @@ static enum TDB_ERROR tdb_expand(struct tdb_context *tdb, tdb_len_t size)
        /* We need to drop this lock before adding free record. */
        tdb_unlock_expand(tdb, F_WRLCK);
 
-       add_stat(tdb, expands, 1);
-       return add_free_record(tdb, old_size, wanted);
+       tdb->stats.expands++;
+       return add_free_record(tdb, old_size, wanted, TDB_LOCK_WAIT);
 }
 
 /* This won't fail: it will expand the database if it has to. */