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>

diff --git a/source3/locking/share_mode_lock.c b/source3/locking/share_mode_lock.c
index fc4956071c855d868ff56efff322e0eba31b1bdf..e52cd3df22c2de4ab8e28cefb6ce2211a8e9205f 100644 (file)
--- 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 1702141c1783f2523abd9d38262d218d4b8d042d..b5ba2cffcbfcd7788ef2f4d43cb2ea3c17f561b3 100644 (file)
--- 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