Skip to content

C
ccan
Commit b2c0ebc6 authored by Rusty Russell's avatar Rusty Russell
Browse files
Options

tdb: locking cleanup 

Explicitly split lock and unlock, for future replacement with user-supplied functions.
Use explicit flags enum for marking, blocking and probing.
Indicate upon unlock whether we had a read or write lock (POSIX doesn't care, but
wrappers might, and it's nice to be explicit).
parent 450a9a4c
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 239 additions and 120 deletions
ccan/tdb/io.c
View file @ b2c0ebc6
......@@ -460,7 +460,8 @@ static const struct tdb_methods io_methods = {
tdb_next_hash_chain,
tdb_oob,
tdb_expand_file,
tdb_brlock
tdb_brlock,
tdb_brunlock
};
/*
......
ccan/tdb/lock.c
View file @ b2c0ebc6
This diff is collapsed.
ccan/tdb/open.c
View file @ b2c0ebc6
......@@ -226,7 +226,7 @@ struct tdb_context *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
fcntl(tdb->fd, F_SETFD, v | FD_CLOEXEC);
/* ensure there is only one process initialising at once */
if (tdb->methods->tdb_brlock(tdb, GLOBAL_LOCK, F_WRLCK, F_SETLKW, 0, 1) == -1) {
if (tdb->methods->brlock(tdb, F_WRLCK, GLOBAL_LOCK, 1, TDB_LOCK_WAIT) == -1) {
TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_open_ex: failed to get global lock on %s: %s\n",
name, strerror(errno)));
goto fail; /* errno set by tdb_brlock */
......@@ -235,7 +235,7 @@ struct tdb_context *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
/* we need to zero database if we are the only one with it open */
if ((tdb_flags & TDB_CLEAR_IF_FIRST) &&
(!tdb->read_only) &&
(locked = (tdb->methods->tdb_brlock(tdb, ACTIVE_LOCK, F_WRLCK, F_SETLK, 0, 1) == 0))) {
(locked = (tdb->methods->brlock(tdb, F_WRLCK, ACTIVE_LOCK, 1, TDB_LOCK_NOWAIT|TDB_LOCK_PROBE) == 0))) {
open_flags |= O_CREAT;
if (ftruncate(tdb->fd, 0) == -1) {
TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_open_ex: "
......@@ -298,7 +298,7 @@ struct tdb_context *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
tdb->inode = st.st_ino;
tdb_mmap(tdb);
if (locked) {
if (tdb->methods->tdb_brlock(tdb, ACTIVE_LOCK, F_UNLCK, F_SETLK, 0, 1) == -1) {
if (tdb->methods->brunlock(tdb, F_WRLCK, ACTIVE_LOCK, 1) == -1) {
TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_open_ex: "
"failed to take ACTIVE_LOCK on %s: %s\n",
name, strerror(errno)));
......@@ -313,7 +313,7 @@ struct tdb_context *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
if (tdb_flags & TDB_CLEAR_IF_FIRST) {
/* leave this lock in place to indicate it's in use */
if (tdb->methods->tdb_brlock(tdb, ACTIVE_LOCK, F_RDLCK, F_SETLKW, 0, 1) == -1)
if (tdb->methods->brlock(tdb, F_RDLCK, ACTIVE_LOCK, 1, TDB_LOCK_WAIT) == -1)
goto fail;
}
......@@ -341,7 +341,7 @@ struct tdb_context *tdb_open_ex(const char *name, int hash_size, int tdb_flags,
/* Internal (memory-only) databases skip all the code above to
* do with disk files, and resume here by releasing their
* global lock and hooking into the active list. */
if (tdb->methods->tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0, 1) == -1)
if (tdb->methods->brunlock(tdb, F_WRLCK, GLOBAL_LOCK, 1) == -1)
goto fail;
tdb->next = tdbs;
tdbs = tdb;
......@@ -391,10 +391,10 @@ int tdb_close(struct tdb_context *tdb)
struct tdb_context **i;
int ret = 0;
tdb_trace(tdb, "tdb_close");
if (tdb->transaction) {
_tdb_transaction_cancel(tdb);
tdb_transaction_cancel(tdb);
}
tdb_trace(tdb, "tdb_close");
if (tdb->map_ptr) {
if (tdb->flags & TDB_INTERNAL)
......@@ -468,7 +468,7 @@ int tdb_reopen(struct tdb_context *tdb)
goto fail;
}
if ((tdb->flags & TDB_CLEAR_IF_FIRST) &&
(tdb->methods->tdb_brlock(tdb, ACTIVE_LOCK, F_RDLCK, F_SETLKW, 0, 1) == -1)) {
(tdb->methods->brlock(tdb, F_RDLCK, ACTIVE_LOCK, 1, TDB_LOCK_WAIT) == -1)) {
TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_reopen: failed to obtain active lock\n"));
goto fail;
}
......
ccan/tdb/tdb.c
View file @ b2c0ebc6
......@@ -59,13 +59,14 @@ static void tdb_increment_seqnum(struct tdb_context *tdb)
return;
}
if (tdb_brlock(tdb, TDB_SEQNUM_OFS, F_WRLCK, F_SETLKW, 1, 1) != 0) {
if (tdb_brlock(tdb, F_WRLCK, TDB_SEQNUM_OFS, 1,
TDB_LOCK_WAIT|TDB_LOCK_PROBE) != 0) {
return;
}
tdb_increment_seqnum_nonblock(tdb);
tdb_brlock(tdb, TDB_SEQNUM_OFS, F_UNLCK, F_SETLKW, 1, 1);
tdb_brunlock(tdb, F_WRLCK, TDB_SEQNUM_OFS, 1);
}
static int tdb_key_compare(TDB_DATA key, TDB_DATA data, void *private_data)
......
ccan/tdb/tdb_private.h
View file @ b2c0ebc6
......@@ -182,6 +182,15 @@ struct tdb_traverse_lock {
int lock_rw;
};
enum tdb_lock_flags {
/* WAIT == F_SETLKW, NOWAIT == F_SETLK */
TDB_LOCK_NOWAIT = 0,
TDB_LOCK_WAIT = 1,
/* If set, don't log an error on failure. */
TDB_LOCK_PROBE = 2,
/* If set, don't actually lock at all. */
TDB_LOCK_MARK_ONLY = 4,
};
struct tdb_methods {
int (*tdb_read)(struct tdb_context *, tdb_off_t , void *, tdb_len_t , int );
......@@ -189,7 +198,8 @@ struct tdb_methods {
void (*next_hash_chain)(struct tdb_context *, uint32_t *);
int (*tdb_oob)(struct tdb_context *, tdb_off_t , int );
int (*tdb_expand_file)(struct tdb_context *, tdb_off_t , tdb_off_t );
int (*tdb_brlock)(struct tdb_context *, tdb_off_t , int, int, int, size_t);
int (*brlock)(struct tdb_context *, int, tdb_off_t, size_t, enum tdb_lock_flags);
int (*brunlock)(struct tdb_context *, int, tdb_off_t, size_t);
};
struct tdb_context {
......@@ -235,9 +245,13 @@ void tdb_mmap(struct tdb_context *tdb);
int tdb_lock(struct tdb_context *tdb, int list, int ltype);
int tdb_lock_nonblock(struct tdb_context *tdb, int list, int ltype);
int tdb_unlock(struct tdb_context *tdb, int list, int ltype);
int tdb_brlock(struct tdb_context *tdb, tdb_off_t offset, int rw_type, int lck_type, int probe, size_t len);
int tdb_brlock(struct tdb_context *tdb,
int rw_type, tdb_off_t offset, size_t len,
enum tdb_lock_flags flags);
int tdb_brunlock(struct tdb_context *tdb,
int rw_type, tdb_off_t offset, size_t len);
int tdb_transaction_lock(struct tdb_context *tdb, int ltype);
int tdb_transaction_unlock(struct tdb_context *tdb);
int tdb_transaction_unlock(struct tdb_context *tdb, int ltype);
int tdb_brlock_upgrade(struct tdb_context *tdb, tdb_off_t offset, size_t len);
int tdb_write_lock_record(struct tdb_context *tdb, tdb_off_t off);
int tdb_write_unlock_record(struct tdb_context *tdb, tdb_off_t off);
......@@ -250,7 +264,7 @@ int tdb_ofs_read(struct tdb_context *tdb, tdb_off_t offset, tdb_off_t *d);
int tdb_ofs_write(struct tdb_context *tdb, tdb_off_t offset, tdb_off_t *d);
int tdb_lock_record(struct tdb_context *tdb, tdb_off_t off);
int tdb_unlock_record(struct tdb_context *tdb, tdb_off_t off);
int _tdb_transaction_cancel(struct tdb_context *tdb);
int _tdb_transaction_cancel(struct tdb_context *tdb, int ltype);
int tdb_rec_read(struct tdb_context *tdb, tdb_off_t offset, struct list_struct *rec);
int tdb_rec_write(struct tdb_context *tdb, tdb_off_t offset, struct list_struct *rec);
int tdb_do_delete(struct tdb_context *tdb, tdb_off_t rec_ptr, struct list_struct *rec);
......
ccan/tdb/test/run-3G-file.c
View file @ b2c0ebc6
......@@ -50,7 +50,8 @@ static const struct tdb_methods large_io_methods = {
tdb_next_hash_chain,
tdb_oob,
tdb_expand_file_sparse,
tdb_brlock
tdb_brlock,
tdb_brunlock
};
static int test_traverse(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data,
......
ccan/tdb/transaction.c
View file @ b2c0ebc6
......@@ -411,8 +411,15 @@ static int transaction_expand_file(struct tdb_context *tdb, tdb_off_t size,
/*
brlock during a transaction - ignore them
*/
static int transaction_brlock(struct tdb_context *tdb, tdb_off_t offset,
int rw_type, int lck_type, int probe, size_t len)
static int transaction_brlock(struct tdb_context *tdb,
int rw_type, tdb_off_t offset, size_t len,
enum tdb_lock_flags flags)
{
return 0;
}
static int transaction_brunlock(struct tdb_context *tdb,
int rw_type, tdb_off_t offset, size_t len)
{
return 0;
}
......@@ -423,7 +430,8 @@ static const struct tdb_methods transaction_methods = {
transaction_next_hash_chain,
transaction_oob,
transaction_expand_file,
transaction_brlock
transaction_brlock,
transaction_brunlock
};
/*
......@@ -455,7 +463,8 @@ static int transaction_sync(struct tdb_context *tdb, tdb_off_t offset, tdb_len_t
return 0;
}
int _tdb_transaction_cancel(struct tdb_context *tdb)
/* ltype is F_WRLCK after prepare. */
int _tdb_transaction_cancel(struct tdb_context *tdb, int ltype)
{
int i, ret = 0;
......@@ -494,15 +503,16 @@ int _tdb_transaction_cancel(struct tdb_context *tdb)
/* remove any global lock created during the transaction */
if (tdb->global_lock.count != 0) {
tdb_brlock(tdb, FREELIST_TOP, F_UNLCK, F_SETLKW, 0, 4*tdb->header.hash_size);
tdb_brunlock(tdb, tdb->global_lock.ltype,
FREELIST_TOP, 4*tdb->header.hash_size);
tdb->global_lock.count = 0;
}
/* remove any locks created during the transaction */
if (tdb->num_locks != 0) {
for (i=0;i<tdb->num_lockrecs;i++) {
tdb_brlock(tdb,FREELIST_TOP+4*tdb->lockrecs[i].list,
F_UNLCK,F_SETLKW, 0, 1);
tdb_brunlock(tdb, tdb->lockrecs[i].ltype,
FREELIST_TOP+4*tdb->lockrecs[i].list, 1);
}
tdb->num_locks = 0;
tdb->num_lockrecs = 0;
......@@ -512,8 +522,8 @@ int _tdb_transaction_cancel(struct tdb_context *tdb)
/* restore the normal io methods */
tdb->methods = tdb->transaction->io_methods;
tdb_brlock(tdb, FREELIST_TOP, F_UNLCK, F_SETLKW, 0, 0);
tdb_transaction_unlock(tdb);
tdb_brunlock(tdb, ltype, FREELIST_TOP, 0);
tdb_transaction_unlock(tdb, F_WRLCK);
SAFE_FREE(tdb->transaction->hash_heads);
SAFE_FREE(tdb->transaction);
......@@ -585,7 +595,7 @@ int tdb_transaction_start(struct tdb_context *tdb)
/* get a read lock from the freelist to the end of file. This
is upgraded to a write lock during the commit */
if (tdb_brlock(tdb, FREELIST_TOP, F_RDLCK, F_SETLKW, 0, 0) == -1) {
if (tdb_brlock(tdb, F_RDLCK, FREELIST_TOP, 0, TDB_LOCK_WAIT) == -1) {
TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_transaction_start: failed to get hash locks\n"));
tdb->ecode = TDB_ERR_LOCK;
goto fail;
......@@ -621,8 +631,8 @@ int tdb_transaction_start(struct tdb_context *tdb)
return 0;
fail:
tdb_brlock(tdb, FREELIST_TOP, F_UNLCK, F_SETLKW, 0, 0);
tdb_transaction_unlock(tdb);
tdb_brunlock(tdb, F_RDLCK, FREELIST_TOP, 0);
tdb_transaction_unlock(tdb, F_WRLCK);
SAFE_FREE(tdb->transaction->blocks);
SAFE_FREE(tdb->transaction->hash_heads);
SAFE_FREE(tdb->transaction);
......@@ -634,9 +644,12 @@ fail:
cancel the current transaction
*/
int tdb_transaction_cancel(struct tdb_context *tdb)
{
{
int ltype = F_RDLCK;
tdb_trace(tdb, "tdb_transaction_cancel");
return _tdb_transaction_cancel(tdb);
if (tdb->transaction && tdb->transaction->prepared)
ltype = F_WRLCK;
return _tdb_transaction_cancel(tdb, ltype);
}
/*
......@@ -896,14 +909,14 @@ static int _tdb_transaction_prepare_commit(struct tdb_context *tdb)
if (tdb->transaction->prepared) {
tdb->ecode = TDB_ERR_EINVAL;
_tdb_transaction_cancel(tdb);
_tdb_transaction_cancel(tdb, F_WRLCK);
TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_transaction_prepare_commit: transaction already prepared\n"));
return -1;
}
if (tdb->transaction->transaction_error) {
tdb->ecode = TDB_ERR_IO;
_tdb_transaction_cancel(tdb);
_tdb_transaction_cancel(tdb, F_RDLCK);
TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_transaction_prepare_commit: transaction error pending\n"));
return -1;
}
......@@ -931,7 +944,7 @@ static int _tdb_transaction_prepare_commit(struct tdb_context *tdb)
if (tdb->num_locks || tdb->global_lock.count) {
tdb->ecode = TDB_ERR_LOCK;
TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_transaction_prepare_commit: locks pending on commit\n"));
_tdb_transaction_cancel(tdb);
_tdb_transaction_cancel(tdb, F_RDLCK);
return -1;
}
......@@ -939,16 +952,16 @@ static int _tdb_transaction_prepare_commit(struct tdb_context *tdb)
if (tdb_brlock_upgrade(tdb, FREELIST_TOP, 0) == -1) {
TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_transaction_prepare_commit: failed to upgrade hash locks\n"));
tdb->ecode = TDB_ERR_LOCK;
_tdb_transaction_cancel(tdb);
_tdb_transaction_cancel(tdb, F_RDLCK);
return -1;
}
/* get the global lock - this prevents new users attaching to the database
during the commit */
if (tdb_brlock(tdb, GLOBAL_LOCK, F_WRLCK, F_SETLKW, 0, 1) == -1) {
if (tdb_brlock(tdb, F_WRLCK, GLOBAL_LOCK, 1, TDB_LOCK_WAIT) == -1) {
TDB_LOG((tdb, TDB_DEBUG_ERROR, "tdb_transaction_prepare_commit: failed to get global lock\n"));
tdb->ecode = TDB_ERR_LOCK;
_tdb_transaction_cancel(tdb);
_tdb_transaction_cancel(tdb, F_WRLCK);
return -1;
}
......@@ -956,8 +969,8 @@ static int _tdb_transaction_prepare_commit(struct tdb_context *tdb)
/* write the recovery data to the end of the file */
if (transaction_setup_recovery(tdb, &tdb->transaction->magic_offset) == -1) {
TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_transaction_prepare_commit: failed to setup recovery data\n"));
tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0, 1);
_tdb_transaction_cancel(tdb);
tdb_brunlock(tdb, F_WRLCK, GLOBAL_LOCK, 0);
_tdb_transaction_cancel(tdb, F_WRLCK);
return -1;
}
}
......@@ -971,8 +984,8 @@ static int _tdb_transaction_prepare_commit(struct tdb_context *tdb)
tdb->transaction->old_map_size) == -1) {
tdb->ecode = TDB_ERR_IO;
TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_transaction_prepare_commit: expansion failed\n"));
tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0, 1);
_tdb_transaction_cancel(tdb);
tdb_brunlock(tdb, F_WRLCK, GLOBAL_LOCK, 0);
_tdb_transaction_cancel(tdb, F_WRLCK);
return -1;
}
tdb->map_size = tdb->transaction->old_map_size;
......@@ -1030,7 +1043,7 @@ int tdb_transaction_commit(struct tdb_context *tdb)
/* check for a null transaction */
if (tdb->transaction->blocks == NULL) {
_tdb_transaction_cancel(tdb);
_tdb_transaction_cancel(tdb, F_RDLCK);
return 0;
}
......@@ -1066,8 +1079,8 @@ int tdb_transaction_commit(struct tdb_context *tdb)
tdb->methods = methods;
tdb_transaction_recover(tdb);
_tdb_transaction_cancel(tdb);
tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0, 1);
_tdb_transaction_cancel(tdb, F_WRLCK);
tdb_brunlock(tdb, F_WRLCK, GLOBAL_LOCK, 0);
TDB_LOG((tdb, TDB_DEBUG_FATAL, "tdb_transaction_commit: write failed\n"));
return -1;
......@@ -1083,7 +1096,7 @@ int tdb_transaction_commit(struct tdb_context *tdb)
return -1;
}
tdb_brlock(tdb, GLOBAL_LOCK, F_UNLCK, F_SETLKW, 0, 1);
tdb_brunlock(tdb, F_WRLCK, GLOBAL_LOCK, 1);
/*
TODO: maybe write to some dummy hdr field, or write to magic
......@@ -1104,7 +1117,7 @@ int tdb_transaction_commit(struct tdb_context *tdb)
/* use a transaction cancel to free memory and remove the
transaction locks */
_tdb_transaction_cancel(tdb);
_tdb_transaction_cancel(tdb, F_WRLCK);
if (need_repack) {
return tdb_repack(tdb);
......
ccan/tdb/traverse.c
View file @ b2c0ebc6
......@@ -229,7 +229,7 @@ int tdb_traverse_read(struct tdb_context *tdb,
ret = _tdb_traverse(tdb, fn, private_data, &tl);
tdb->traverse_read--;
tdb_transaction_unlock(tdb);
tdb_transaction_unlock(tdb, F_RDLCK);
return ret;
}
......@@ -260,7 +260,7 @@ int tdb_traverse(struct tdb_context *tdb,
ret = _tdb_traverse(tdb, fn, private_data, &tl);
tdb->traverse_write--;
tdb_transaction_unlock(tdb);
tdb_transaction_unlock(tdb, F_WRLCK);
return ret;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7