diff options
author | Stefan Metzmacher <metze@samba.org> | 2022-08-29 13:44:00 +0200 |
---|---|---|
committer | Jeremy Allison <jra@samba.org> | 2022-09-20 00:34:35 +0000 |
commit | 775dc007d211b8153fbc5741f52dc77f92d9c314 (patch) | |
tree | 387f4d6a51e15365bc7a724d4debdd46426e05d3 /source3 | |
parent | cba169252ea270bb725ec06aff71d841492099f5 (diff) | |
download | samba-775dc007d211b8153fbc5741f52dc77f92d9c314.tar.gz |
s3:locking: add share_mode_entry_prepare_{lock,unlock}() infrastructure
When adding or deleting share mode entries elements, we typically
have a pattern like this:
1. get the g_lock via get_[existing_]share_mode_lock()
2. do some checking of the existing record
3. add/delete a share_mode_entry to the record
4. do some vfs operations still protected by the g_lock
5. (optional) cleanup of the record on failure
6. release the g_lock
We can optimize this to:
- Run 1-3. under a tdb chainlock
- Only protect vfs operations with the g_lock
if a new file was created/will be deleted
- Regrab the g_lock for a cleanup.
The new share_mode_entry_prepare_lock()
allows the caller to run a function within a tdb chainlock
similar to share_mode_do_locked_vfs_denied() where vfs calls are denied
and the execution is done within a tdb chainlock.
But the callback function is allowed to decide if it wants to
keep the lock at the g_lock layer on return.
The decision is kept in struct share_mode_entry_prepare_state,
which is then passed to share_mode_entry_prepare_unlock()
with an optional callback to do some cleanup under the
still existing g_lock or a regrabed g_lock.
In the ideal case the callback function passed to
share_mode_entry_prepare_lock() is able to decide that
it can drop the g_lock and the share_mode_entry_prepare_unlock().
gets a NULL callback as there's nothing to cleanup.
In this case share_mode_entry_prepare_unlock() is a noop.
This will allow us to avoid fallbacks to the dbwrap_watch based
waiting for the g_lock in the SMB2 Create and Close code paths.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=15125
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Diffstat (limited to 'source3')
-rw-r--r-- | source3/locking/share_mode_lock.c | 267 | ||||
-rw-r--r-- | source3/locking/share_mode_lock.h | 50 |
2 files changed, 311 insertions, 6 deletions
diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c index fc4956071c8..e52cd3df22c 100644 --- a/source3/locking/share_mode_lock.c +++ b/source3/locking/share_mode_lock.c @@ -39,8 +39,13 @@ #include "lib/util/time_basic.h" #include "system/filesys.h" #include "lib/util/server_id.h" -#include "share_mode_lock.h" #include "share_mode_lock_private.h" +struct share_mode_lock { + struct file_id id; + struct share_mode_data *cached_data; +}; +#define SHARE_MODE_ENTRY_PREPARE_STATE_LCK_SPACE 1 +#include "share_mode_lock.h" #include "locking/proto.h" #include "smbd/globals.h" #include "dbwrap/dbwrap.h" @@ -68,11 +73,6 @@ DBGLVL_DEBUG : DBGLVL_ERR, \ (__VA_ARGS__)) -struct share_mode_lock { - struct file_id id; - struct share_mode_data *cached_data; -}; - /* the locking database handle */ static struct g_lock_ctx *lock_ctx; static struct g_lock_lock_cb_state *current_share_mode_glck = NULL; @@ -2935,3 +2935,258 @@ NTSTATUS _share_mode_do_locked_vfs_allowed( return NT_STATUS_OK; } + +struct share_mode_entry_prepare_lock_state { + struct file_id id; + const char *servicepath; + const struct smb_filename *smb_fname; + const struct timespec *old_write_time; + share_mode_entry_prepare_lock_fn_t fn; + void *private_data; + const char *location; + bool keep_locked; + struct share_mode_lock *lck; + NTSTATUS status; +}; + +static void share_mode_entry_prepare_lock_fn(struct g_lock_lock_cb_state *glck, + void *cb_private) +{ + struct share_mode_entry_prepare_lock_state *state = + (struct share_mode_entry_prepare_lock_state *)cb_private; + struct smb_vfs_deny_state vfs_deny = {}; + + SMB_ASSERT(glck != NULL); + current_share_mode_glck = glck; + + state->status = get_share_mode_lock_internal(state->id, + state->servicepath, + state->smb_fname, + state->old_write_time, + state->lck); + if (!NT_STATUS_IS_OK(state->status)) { + /* no DBG_GET_SHARE_MODE_LOCK here! */ + DBG_ERR("get_share_mode_lock_internal failed: %s\n", + nt_errstr(state->status)); + g_lock_lock_cb_unlock(glck); + current_share_mode_glck = NULL; + return; + } + + _smb_vfs_deny_push(&vfs_deny, state->location); + state->fn(state->lck, &state->keep_locked, state->private_data); + _smb_vfs_deny_pop(&vfs_deny, state->location); + + if (state->keep_locked) { + current_share_mode_glck = NULL; + return; + } + + state->status = put_share_mode_lock_internal(state->lck); + if (!NT_STATUS_IS_OK(state->status)) { + DBG_ERR("put_share_mode_lock_internal failed: %s\n", + nt_errstr(state->status)); + smb_panic("put_share_mode_lock_internal failed\n"); + return; + } + + g_lock_lock_cb_unlock(glck); + current_share_mode_glck = NULL; + return; +} + +NTSTATUS _share_mode_entry_prepare_lock( + struct share_mode_entry_prepare_state *prepare_state, + struct file_id id, + const char *servicepath, + const struct smb_filename *smb_fname, + const struct timespec *old_write_time, + share_mode_entry_prepare_lock_fn_t fn, + void *private_data, + const char *location) +{ + struct share_mode_entry_prepare_lock_state state = { + .id = id, + .servicepath = servicepath, + .smb_fname = smb_fname, + .old_write_time = old_write_time, + .fn = fn, + .private_data = private_data, + .location = location, + }; + TDB_DATA key = locking_key(&id); + NTSTATUS status; + + SMB_ASSERT(share_mode_lock_key_refcount == 0); + + SMB_ASSERT(__SHARE_MODE_LOCK_SPACE == sizeof(struct share_mode_lock)); + + *prepare_state = (struct share_mode_entry_prepare_state) { + .__fid = id, + .__lck_ptr = &prepare_state->__lck_space, + }; + + state.lck = prepare_state->__lck_ptr; + + share_mode_lock_skip_g_lock = true; + status = g_lock_lock( + lock_ctx, + key, + G_LOCK_WRITE, + (struct timeval) { .tv_sec = 3600 }, + share_mode_entry_prepare_lock_fn, + &state); + share_mode_lock_skip_g_lock = false; + if (!state.keep_locked) { + prepare_state->__lck_ptr = NULL; + } + if (!NT_STATUS_IS_OK(status)) { + DBG_DEBUG("g_lock_lock failed: %s\n", + nt_errstr(status)); + return status; + } + + return state.status; +} + +struct share_mode_entry_prepare_unlock_state { + struct file_id id; + share_mode_entry_prepare_unlock_fn_t fn; + void *private_data; + const char *location; + struct share_mode_lock *lck; + NTSTATUS status; +}; + +static void share_mode_entry_prepare_unlock_existing_fn( + struct share_mode_entry_prepare_unlock_state *state) +{ + if (state->fn != NULL) { + struct smb_vfs_deny_state vfs_deny = {}; + + _smb_vfs_deny_push(&vfs_deny, state->location); + state->fn(state->lck, state->private_data); + _smb_vfs_deny_pop(&vfs_deny, state->location); + } + + state->status = put_share_mode_lock_internal(state->lck); + if (!NT_STATUS_IS_OK(state->status)) { + DBG_ERR("put_share_mode_lock_internal failed: %s\n", + nt_errstr(state->status)); + smb_panic("put_share_mode_lock_internal failed\n"); + return; + } + + return; +} + +static void share_mode_entry_prepare_unlock_relock_fn(struct g_lock_lock_cb_state *glck, + void *cb_private) +{ + struct share_mode_entry_prepare_unlock_state *state = + (struct share_mode_entry_prepare_unlock_state *)cb_private; + struct smb_vfs_deny_state vfs_deny = {}; + + SMB_ASSERT(glck != NULL); + current_share_mode_glck = glck; + + state->status = get_share_mode_lock_internal(state->id, + NULL, /* servicepath */ + NULL, /* smb_fname */ + NULL, /* old_write_time */ + state->lck); + if (!NT_STATUS_IS_OK(state->status)) { + /* no DBG_GET_SHARE_MODE_LOCK here! */ + DBG_ERR("get_share_mode_lock_internal failed: %s\n", + nt_errstr(state->status)); + g_lock_lock_cb_unlock(glck); + current_share_mode_glck = NULL; + return; + } + + _smb_vfs_deny_push(&vfs_deny, state->location); + state->fn(state->lck, state->private_data); + _smb_vfs_deny_pop(&vfs_deny, state->location); + + state->status = put_share_mode_lock_internal(state->lck); + if (!NT_STATUS_IS_OK(state->status)) { + DBG_ERR("put_share_mode_lock_internal failed: %s\n", + nt_errstr(state->status)); + smb_panic("put_share_mode_lock_internal failed\n"); + return; + } + + g_lock_lock_cb_unlock(glck); + current_share_mode_glck = NULL; + return; +} + +NTSTATUS _share_mode_entry_prepare_unlock( + struct share_mode_entry_prepare_state *prepare_state, + share_mode_entry_prepare_unlock_fn_t fn, + void *private_data, + const char *location) +{ + struct share_mode_entry_prepare_unlock_state state = { + .id = prepare_state->__fid, + .fn = fn, + .private_data = private_data, + .location = location, + }; + TDB_DATA key = locking_key(&prepare_state->__fid); + NTSTATUS status; + + if (prepare_state->__lck_ptr != NULL) { + /* + * With an existing lock, we just run the unlock prepare + * function following by the unlock. + */ + + SMB_ASSERT(share_mode_lock_key_refcount == 1); + + state.lck = prepare_state->__lck_ptr; + prepare_state->__lck_ptr = NULL; + + share_mode_entry_prepare_unlock_existing_fn(&state); + return state.status; + } + + /* + * No existing lock, which means + * _share_mode_entry_prepare_lock() didn't steal + * the lock... + */ + SMB_ASSERT(share_mode_lock_key_refcount == 0); + + if (fn == NULL) { + /* + * Without an existing lock and without + * a prepare function there's nothing to + * do... + */ + return NT_STATUS_OK; + } + + /* + * In order to run the unlock prepare function + * we need to relock the entry. + */ + state.lck = &prepare_state->__lck_space; + + share_mode_lock_skip_g_lock = true; + status = g_lock_lock( + lock_ctx, + key, + G_LOCK_WRITE, + (struct timeval) { .tv_sec = 3600 }, + share_mode_entry_prepare_unlock_relock_fn, + &state); + share_mode_lock_skip_g_lock = false; + if (!NT_STATUS_IS_OK(status)) { + DBG_ERR("g_lock_lock failed: %s\n", + nt_errstr(status)); + return status; + } + + return state.status; +} diff --git a/source3/locking/share_mode_lock.h b/source3/locking/share_mode_lock.h index 1702141c178..b5ba2cffcbf 100644 --- a/source3/locking/share_mode_lock.h +++ b/source3/locking/share_mode_lock.h @@ -152,4 +152,54 @@ NTSTATUS _share_mode_do_locked_vfs_allowed( #define share_mode_do_locked_vfs_allowed(__id, __fn, __private_data) \ _share_mode_do_locked_vfs_allowed(__id, __fn, __private_data, __location__) +struct share_mode_entry_prepare_state { + struct file_id __fid; + struct share_mode_lock *__lck_ptr; + union { +#define __SHARE_MODE_LOCK_SPACE 32 + uint8_t __u8_space[__SHARE_MODE_LOCK_SPACE]; +#ifdef SHARE_MODE_ENTRY_PREPARE_STATE_LCK_SPACE + struct share_mode_lock __lck_space; +#endif + }; +}; + +typedef void (*share_mode_entry_prepare_lock_fn_t)( + struct share_mode_lock *lck, + bool *keep_locked, + void *private_data); +NTSTATUS _share_mode_entry_prepare_lock( + struct share_mode_entry_prepare_state *prepare_state, + struct file_id id, + const char *servicepath, + const struct smb_filename *smb_fname, + const struct timespec *old_write_time, + share_mode_entry_prepare_lock_fn_t fn, + void *private_data, + const char *location); +#define share_mode_entry_prepare_lock_add(__prepare_state, __id, \ + __servicepath, __smb_fname, __old_write_time, \ + __fn, __private_data) \ + _share_mode_entry_prepare_lock(__prepare_state, __id, \ + __servicepath, __smb_fname, __old_write_time, \ + __fn, __private_data, __location__); +#define share_mode_entry_prepare_lock_del(__prepare_state, __id, \ + __fn, __private_data) \ + _share_mode_entry_prepare_lock(__prepare_state, __id, \ + NULL, NULL, NULL, \ + __fn, __private_data, __location__); + +typedef void (*share_mode_entry_prepare_unlock_fn_t)( + struct share_mode_lock *lck, + void *private_data); +NTSTATUS _share_mode_entry_prepare_unlock( + struct share_mode_entry_prepare_state *prepare_state, + share_mode_entry_prepare_unlock_fn_t fn, + void *private_data, + const char *location); +#define share_mode_entry_prepare_unlock(__prepare_state, \ + __fn, __private_data) \ + _share_mode_entry_prepare_unlock(__prepare_state, \ + __fn, __private_data, __location__); + #endif |