smbd: Don't store num_read_oplocks in brlock.tdb

This removes a kludgy implementation that worked around a locking
hierarchy problem: Setting a byte range lock had to contend the level2
oplocks, which are stored in locking.tdb/leases.tdb. We could not
access locking.tdb in the brlock.tdb code, as brlock.tdb might have
been locked first without locking.tdb, violating the locking hierarchy
locking.tdb->brlock.tdb. Now that that problem is gone (see the commit
wrapping do_lock() in share_mode_do_locked()), we can remove this
kludge.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c
index 75c020f57a604e01b224145b69e12f3cfc18e3a9..cdfd09ceff11c07e6da020d84dd8d95d2f1fc596 100644 (file)
--- a/source3/locking/brlock.c
+++ b/source3/locking/brlock.c
@@ -48,7 +48,6 @@ struct byte_range_lock {
        struct files_struct *fsp;
        unsigned int num_locks;
        bool modified;
-       uint32_t num_read_oplocks;
        struct lock_struct *lock_data;
        struct db_record *record;
 };
@@ -85,21 +84,6 @@ struct files_struct *brl_fsp(struct byte_range_lock *brl)
        return brl->fsp;
 }
 
-uint32_t brl_num_read_oplocks(const struct byte_range_lock *brl)
-{
-       return brl->num_read_oplocks;
-}
-
-void brl_set_num_read_oplocks(struct byte_range_lock *brl,
-                             uint32_t num_read_oplocks)
-{
-       DEBUG(10, ("Setting num_read_oplocks to %"PRIu32"\n",
-                  num_read_oplocks));
-       SMB_ASSERT(brl->record != NULL); /* otherwise we're readonly */
-       brl->num_read_oplocks = num_read_oplocks;
-       brl->modified = true;
-}
-
 /****************************************************************************
  See if two locking contexts are equal.
 ****************************************************************************/
@@ -1652,7 +1636,7 @@ static void byte_range_lock_flush(struct byte_range_lock *br_lck)
                }
        }
 
-       if ((br_lck->num_locks == 0) && (br_lck->num_read_oplocks == 0)) {
+       if (br_lck->num_locks == 0) {
                /* No locks - delete this entry. */
                NTSTATUS status = dbwrap_record_delete(br_lck->record);
                if (!NT_STATUS_IS_OK(status)) {
@@ -1661,25 +1645,13 @@ static void byte_range_lock_flush(struct byte_range_lock *br_lck)
                        smb_panic("Could not delete byte range lock entry");
                }
        } else {
-               size_t lock_len, data_len;
-               TDB_DATA data;
+               TDB_DATA data = {
+                       .dsize = br_lck->num_locks * sizeof(struct lock_struct),
+                       .dptr = (uint8_t *)br_lck->lock_data,
+               };
                NTSTATUS status;
 
-               lock_len = br_lck->num_locks * sizeof(struct lock_struct);
-               data_len = lock_len + sizeof(br_lck->num_read_oplocks);
-
-               data.dsize = data_len;
-               data.dptr = talloc_array(talloc_tos(), uint8_t, data_len);
-               SMB_ASSERT(data.dptr != NULL);
-
-               if (lock_len > 0) {
-                       memcpy(data.dptr, br_lck->lock_data, lock_len);
-               }
-               memcpy(data.dptr + lock_len, &br_lck->num_read_oplocks,
-                      sizeof(br_lck->num_read_oplocks));
-
                status = dbwrap_record_store(br_lck->record, data, TDB_REPLACE);
-               TALLOC_FREE(data.dptr);
                if (!NT_STATUS_IS_OK(status)) {
                        DEBUG(0, ("store returned %s\n", nt_errstr(status)));
                        smb_panic("Could not store byte range mode entry");
@@ -1706,8 +1678,7 @@ static bool brl_parse_data(struct byte_range_lock *br_lck, TDB_DATA data)
        if (data.dsize == 0) {
                return true;
        }
-       if (data.dsize % sizeof(struct lock_struct) !=
-           sizeof(br_lck->num_read_oplocks)) {
+       if (data.dsize % sizeof(struct lock_struct) != 0) {
                DEBUG(1, ("Invalid data size: %u\n", (unsigned)data.dsize));
                return false;
        }
@@ -1720,8 +1691,6 @@ static bool brl_parse_data(struct byte_range_lock *br_lck, TDB_DATA data)
                DEBUG(1, ("talloc_memdup failed\n"));
                return false;
        }
-       memcpy(&br_lck->num_read_oplocks, data.dptr + data_len,
-              sizeof(br_lck->num_read_oplocks));
        return true;
 }
 
@@ -1843,7 +1812,6 @@ struct byte_range_lock *brl_get_locks_readonly(files_struct *fsp)
                        return NULL;
                }
 
-               br_lock->num_read_oplocks = 0;
                br_lock->num_locks = 0;
                br_lock->lock_data = NULL;
 

diff --git a/source3/locking/proto.h b/source3/locking/proto.h
index a1c6529183ae69132d5fde4d484f2019635b9c0f..08d3f2c7ab8e88512fc3b3d74f4d08c38f1a65bf 100644 (file)
--- a/source3/locking/proto.h
+++ b/source3/locking/proto.h
@@ -30,9 +30,6 @@ void brl_shutdown(void);
 
 unsigned int brl_num_locks(const struct byte_range_lock *brl);
 struct files_struct *brl_fsp(struct byte_range_lock *brl);
-uint32_t brl_num_read_oplocks(const struct byte_range_lock *brl);
-void brl_set_num_read_oplocks(struct byte_range_lock *brl,
-                             uint32_t num_read_oplocks);
 
 NTSTATUS brl_lock_windows_default(struct byte_range_lock *br_lck,
                                  struct lock_struct *plock);

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index a46e42482f78e04e39ef9dd8416bccd17be2aab0..c15e8f67d90473aeb7ee1f96122e2c66c638f1ea 100644 (file)
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -2359,12 +2359,6 @@ static NTSTATUS grant_fsp_oplock_type(struct smb_request *req,
                lck->data->flags |= SHARE_MODE_HAS_READ_LEASE;
        }
 
-       ok = update_num_read_oplocks(fsp, lck);
-       if (!ok) {
-               del_share_mode(lck, fsp);
-               return NT_STATUS_INTERNAL_ERROR;
-       }
-
        DEBUG(10,("grant_fsp_oplock_type: oplock type 0x%x on file %s\n",
                  fsp->oplock_type, fsp_str_dbg(fsp)));
 

diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index c1625440fba9edb7a9742cc0a43edc170e4f4223..e1329094d0f5aa36b4d84260bfb6e46bbbfbb1c6 100644 (file)
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -195,75 +195,6 @@ uint32_t get_lease_type(const struct share_mode_data *d,
        return map_oplock_to_lease_type(e->op_type);
 }
 
-bool update_num_read_oplocks(files_struct *fsp, struct share_mode_lock *lck)
-{
-       struct share_mode_data *d = lck->data;
-       struct byte_range_lock *br_lck;
-       uint32_t num_read_oplocks = 0;
-       uint32_t i;
-
-       if (fsp_lease_type_is_exclusive(fsp)) {
-               const struct share_mode_entry *e = NULL;
-               uint32_t e_lease_type = 0;
-
-               /*
-                * If we're fully exclusive, we don't need a brlock entry
-                */
-               remove_stale_share_mode_entries(d);
-
-               e = find_share_mode_entry(lck, fsp);
-               if (e != NULL) {
-                       e_lease_type = get_lease_type(d, e);
-               }
-
-               if (!lease_type_is_exclusive(e_lease_type)) {
-                       char *timestr = NULL;
-
-                       timestr = timeval_string(talloc_tos(),
-                                                &fsp->open_time,
-                                                true);
-
-                       NDR_PRINT_DEBUG(share_mode_data, d);
-                       DBG_ERR("file [%s] file_id [%s] gen_id [%lu] "
-                               "open_time[%s] lease_type [0x%x] "
-                               "oplock_type [0x%x]\n",
-                               fsp_str_dbg(fsp),
-                               file_id_string_tos(&fsp->file_id),
-                               fsp->fh->gen_id, timestr,
-                               e_lease_type, fsp->oplock_type);
-
-                       smb_panic("Found non-exclusive lease");
-               }
-
-               return true;
-       }
-
-       for (i=0; i<d->num_share_modes; i++) {
-               struct share_mode_entry *e = &d->share_modes[i];
-               uint32_t e_lease_type = get_lease_type(d, e);
-
-               if (e_lease_type & SMB2_LEASE_READ) {
-                       num_read_oplocks += 1;
-               }
-       }
-
-       br_lck = brl_get_locks_readonly(fsp);
-       if (br_lck == NULL) {
-               return false;
-       }
-       if (brl_num_read_oplocks(br_lck) == num_read_oplocks) {
-               return true;
-       }
-
-       br_lck = brl_get_locks(talloc_tos(), fsp);
-       if (br_lck == NULL) {
-               return false;
-       }
-       brl_set_num_read_oplocks(br_lck, num_read_oplocks);
-       TALLOC_FREE(br_lck);
-       return true;
-}
-
 /****************************************************************************
  Remove a file oplock with lock already held. Copes with level II and exclusive.
 ****************************************************************************/
@@ -281,14 +212,6 @@ bool remove_oplock_under_lock(files_struct *fsp, struct share_mode_lock *lck)
        }
        release_file_oplock(fsp);
 
-       ret = update_num_read_oplocks(fsp, lck);
-       if (!ret) {
-               DBG_ERR("update_num_read_oplocks failed for "
-                       "file %s, %s, %s\n",
-                       fsp_str_dbg(fsp), fsp_fnum_dbg(fsp),
-                       file_id_string_tos(&fsp->file_id));
-       }
-
        return ret;
 }
 
@@ -345,14 +268,6 @@ bool downgrade_oplock(files_struct *fsp)
        }
        downgrade_file_oplock(fsp);
 
-       ret = update_num_read_oplocks(fsp, lck);
-       if (!ret) {
-               DEBUG(0, ("%s: update_num_read_oplocks failed for "
-                        "file %s, %s, %s\n",
-                         __func__, fsp_str_dbg(fsp), fsp_fnum_dbg(fsp),
-                        file_id_string_tos(&fsp->file_id)));
-       }
-
        TALLOC_FREE(lck);
        return ret;
 }

diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 2f28cf2076f26011f55b7acdba8c4d5a1c20d757..076702feca539e4349b11fcc366642a43d7ebdaf 100644 (file)
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -725,7 +725,6 @@ NTSTATUS get_relative_fid_filename(connection_struct *conn,
 
 uint32_t get_lease_type(const struct share_mode_data *d,
                        const struct share_mode_entry *e);
-bool update_num_read_oplocks(files_struct *fsp, struct share_mode_lock *lck);
 
 void break_kernel_oplock(struct messaging_context *msg_ctx, files_struct *fsp);
 NTSTATUS set_file_oplock(files_struct *fsp);