From: Volker Lendecke Date: Sun, 30 Jun 2019 17:51:13 +0000 (+0200) Subject: smbd: Don't store num_read_oplocks in brlock.tdb X-Git-Tag: samba-4.11.0rc1~97 X-Git-Url: http://git.samba.org/?a=commitdiff_plain;h=8bae5d82e2a101d37f5572e1d174b37816576840;p=samba.git 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 Reviewed-by: Ralph Boehme --- diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c index 75c020f57a6..cdfd09ceff1 100644 --- 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 a1c6529183a..08d3f2c7ab8 100644 --- 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 a46e42482f7..c15e8f67d90 100644 --- 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 c1625440fba..e1329094d0f 100644 --- 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; inum_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 2f28cf2076f..076702feca5 100644 --- 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);