From 2c6030415e0b1f421ea6e85fe6ffe7389ee7a941 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Sat, 29 Jul 2006 19:14:24 +0000 Subject: r17314: Optimisation for POSIX locking. If we're downgrading a POSIX lock (applying a read-lock) and we overlap pending read locks then send them an unlock message, we may have allowed them to proceed. Jeremy. (This used to be commit a7a0b6ba50f4cf7c5a0a29809fdff9e1266a29e7) --- source3/include/locking.h | 8 +++- source3/locking/brlock.c | 95 ++++++++++++++++++++++++++++++++--------------- source3/locking/locking.c | 6 ++- source3/smbd/blocking.c | 2 +- 4 files changed, 77 insertions(+), 34 deletions(-) diff --git a/source3/include/locking.h b/source3/include/locking.h index 9e70411fa6..a09e7d1aff 100644 --- a/source3/include/locking.h +++ b/source3/include/locking.h @@ -23,11 +23,15 @@ #define _LOCKING_H /* passed to br lock code - the UNLOCK_LOCK should never be stored into the tdb - and is used in calculating POSIX unlock ranges only. */ + and is used in calculating POSIX unlock ranges only. We differentiate between + PENDING read and write locks to allow posix lock downgrades to trigger a lock + re-evaluation. */ -enum brl_type {READ_LOCK, WRITE_LOCK, PENDING_LOCK, UNLOCK_LOCK}; +enum brl_type {READ_LOCK, WRITE_LOCK, PENDING_READ_LOCK, PENDING_WRITE_LOCK, UNLOCK_LOCK}; enum brl_flavour {WINDOWS_LOCK = 0, POSIX_LOCK = 1}; +#define IS_PENDING_LOCK(type) ((type) == PENDING_READ_LOCK || (type) == PENDING_WRITE_LOCK) + /* This contains elements that differentiate locks. The smbpid is a client supplied pid, and is essentially the locking context for this client */ diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c index 4a36d938ad..568c80f3ee 100644 --- a/source3/locking/brlock.c +++ b/source3/locking/brlock.c @@ -98,7 +98,7 @@ static BOOL brl_conflict(const struct lock_struct *lck1, const struct lock_struct *lck2) { /* Ignore PENDING locks. */ - if (lck1->lock_type == PENDING_LOCK || lck2->lock_type == PENDING_LOCK ) + if (IS_PENDING_LOCK(lck1->lock_type) || IS_PENDING_LOCK(lck2->lock_type)) return False; /* Read locks never conflict. */ @@ -129,7 +129,7 @@ static BOOL brl_conflict_posix(const struct lock_struct *lck1, #endif /* Ignore PENDING locks. */ - if (lck1->lock_type == PENDING_LOCK || lck2->lock_type == PENDING_LOCK ) + if (IS_PENDING_LOCK(lck1->lock_type) || IS_PENDING_LOCK(lck2->lock_type)) return False; /* Read locks never conflict. */ @@ -151,7 +151,7 @@ static BOOL brl_conflict_posix(const struct lock_struct *lck1, static BOOL brl_conflict1(const struct lock_struct *lck1, const struct lock_struct *lck2) { - if (lck1->lock_type == PENDING_LOCK || lck2->lock_type == PENDING_LOCK ) + if (IS_PENDING_LOCK(lck1->lock_type) || IS_PENDING_LOCK(lck2->lock_type)) return False; if (lck1->lock_type == READ_LOCK && lck2->lock_type == READ_LOCK) { @@ -184,7 +184,7 @@ static BOOL brl_conflict1(const struct lock_struct *lck1, static BOOL brl_conflict_other(const struct lock_struct *lck1, const struct lock_struct *lck2) { - if (lck1->lock_type == PENDING_LOCK || lck2->lock_type == PENDING_LOCK ) + if (IS_PENDING_LOCK(lck1->lock_type) || IS_PENDING_LOCK(lck2->lock_type)) return False; if (lck1->lock_type == READ_LOCK && lck2->lock_type == READ_LOCK) @@ -210,6 +210,19 @@ static BOOL brl_conflict_other(const struct lock_struct *lck1, const struct lock return brl_overlap(lck1, lck2); } +/**************************************************************************** + Check if an unlock overlaps a pending lock. +****************************************************************************/ + +static BOOL brl_pending_overlap(const struct lock_struct *lock, const struct lock_struct *pend_lock) +{ + if ((lock->start <= pend_lock->start) && (lock->start + lock->size > pend_lock->start)) + return True; + if ((lock->start >= pend_lock->start) && (lock->start <= pend_lock->start + pend_lock->size)) + return True; + return False; +} + /**************************************************************************** Amazingly enough, w2k3 "remembers" whether the last lock failure on a fnum is the same as this one and changes its error code. I wonder if any @@ -320,7 +333,7 @@ static NTSTATUS brl_lock_windows(struct byte_range_lock *br_lck, be mapped into a lower level POSIX one, and if so can we get it ? */ - if ((plock->lock_type != PENDING_LOCK) && lp_posix_locking(SNUM(fsp->conn))) { + if (!IS_PENDING_LOCK(plock->lock_type) && lp_posix_locking(SNUM(fsp->conn))) { int errno_ret; if (!set_posix_lock_windows_flavour(fsp, plock->start, @@ -575,6 +588,7 @@ static NTSTATUS brl_lock_posix(struct byte_range_lock *br_lck, struct lock_struct *locks = (struct lock_struct *)br_lck->lock_data; struct lock_struct *tp; BOOL lock_was_added = False; + BOOL signal_pending_read = False; /* No zero-zero locks for POSIX. */ if (plock->start == 0 && plock->size == 0) { @@ -598,19 +612,28 @@ static NTSTATUS brl_lock_posix(struct byte_range_lock *br_lck, count = 0; for (i=0; i < br_lck->num_locks; i++) { - if (locks[i].lock_flav == WINDOWS_LOCK) { + struct lock_struct *curr_lock = &locks[i]; + + /* If we have a pending read lock, a lock downgrade should + trigger a lock re-evaluation. */ + if (curr_lock->lock_type == PENDING_READ_LOCK && + brl_pending_overlap(plock, curr_lock)) { + signal_pending_read = True; + } + + if (curr_lock->lock_flav == WINDOWS_LOCK) { /* Do any Windows flavour locks conflict ? */ - if (brl_conflict(&locks[i], plock)) { + if (brl_conflict(curr_lock, plock)) { /* No games with error messages. */ SAFE_FREE(tp); return NT_STATUS_FILE_LOCK_CONFLICT; } /* Just copy the Windows lock into the new array. */ - memcpy(&tp[count], &locks[i], sizeof(struct lock_struct)); + memcpy(&tp[count], curr_lock, sizeof(struct lock_struct)); count++; } else { /* POSIX conflict semantics are different. */ - if (brl_conflict_posix(&locks[i], plock)) { + if (brl_conflict_posix(curr_lock, plock)) { /* Can't block ourselves with POSIX locks. */ /* No games with error messages. */ SAFE_FREE(tp); @@ -618,7 +641,7 @@ static NTSTATUS brl_lock_posix(struct byte_range_lock *br_lck, } /* Work out overlaps. */ - count += brlock_posix_split_merge(&tp[count], &locks[i], plock, &lock_was_added); + count += brlock_posix_split_merge(&tp[count], curr_lock, plock, &lock_was_added); } } @@ -631,7 +654,7 @@ static NTSTATUS brl_lock_posix(struct byte_range_lock *br_lck, be mapped into a lower level POSIX one, and if so can we get it ? */ - if ((plock->lock_type != PENDING_LOCK) && lp_posix_locking(SNUM(br_lck->fsp->conn))) { + if (!IS_PENDING_LOCK(plock->lock_type) && lp_posix_locking(SNUM(br_lck->fsp->conn))) { int errno_ret; /* The lower layer just needs to attempt to @@ -661,7 +684,34 @@ static NTSTATUS brl_lock_posix(struct byte_range_lock *br_lck, br_lck->num_locks = count; SAFE_FREE(br_lck->lock_data); br_lck->lock_data = (void *)tp; + locks = tp; br_lck->modified = True; + + /* A successful downgrade from write to read lock can trigger a lock + re-evalutation where waiting readers can now proceed. */ + + if (signal_pending_read) { + /* Send unlock messages to any pending read waiters that overlap. */ + for (i=0; i < br_lck->num_locks; i++) { + struct lock_struct *pend_lock = &locks[i]; + + /* Ignore non-pending locks. */ + if (!IS_PENDING_LOCK(pend_lock->lock_type)) { + continue; + } + + if (pend_lock->lock_type == PENDING_READ_LOCK && + brl_pending_overlap(plock, pend_lock)) { + DEBUG(10,("brl_lock_posix: sending unlock message to pid %s\n", + procid_str_static(&pend_lock->context.pid ))); + + message_send_pid(pend_lock->context.pid, + MSG_SMB_UNLOCK, + NULL, 0, True); + } + } + } + return NT_STATUS_OK; } @@ -710,19 +760,6 @@ NTSTATUS brl_lock(struct byte_range_lock *br_lck, return ret; } -/**************************************************************************** - Check if an unlock overlaps a pending lock. -****************************************************************************/ - -static BOOL brl_pending_overlap(const struct lock_struct *lock, const struct lock_struct *pend_lock) -{ - if ((lock->start <= pend_lock->start) && (lock->start + lock->size > pend_lock->start)) - return True; - if ((lock->start >= pend_lock->start) && (lock->start <= pend_lock->start + pend_lock->size)) - return True; - return False; -} - /**************************************************************************** Unlock a range of bytes - Windows semantics. ****************************************************************************/ @@ -807,7 +844,7 @@ static BOOL brl_unlock_windows(struct byte_range_lock *br_lck, const struct lock struct lock_struct *pend_lock = &locks[j]; /* Ignore non-pending locks. */ - if (pend_lock->lock_type != PENDING_LOCK) { + if (!IS_PENDING_LOCK(pend_lock->lock_type)) { continue; } @@ -866,7 +903,7 @@ static BOOL brl_unlock_posix(struct byte_range_lock *br_lck, const struct lock_s unsigned int tmp_count; /* Only remove our own locks - ignore fnum. */ - if (lock->lock_type == PENDING_LOCK || + if (IS_PENDING_LOCK(lock->lock_type) || !brl_same_context(&lock->context, &plock->context)) { memcpy(&tp[count], lock, sizeof(struct lock_struct)); count++; @@ -974,7 +1011,7 @@ static BOOL brl_unlock_posix(struct byte_range_lock *br_lck, const struct lock_s struct lock_struct *pend_lock = &locks[j]; /* Ignore non-pending locks. */ - if (pend_lock->lock_type != PENDING_LOCK) { + if (!IS_PENDING_LOCK(pend_lock->lock_type)) { continue; } @@ -1173,7 +1210,7 @@ BOOL brl_lock_cancel(struct byte_range_lock *br_lck, /* For pending locks we *always* care about the fnum. */ if (brl_same_context(&lock->context, &context) && lock->fnum == br_lck->fsp->fnum && - lock->lock_type == PENDING_LOCK && + IS_PENDING_LOCK(lock->lock_type) && lock->lock_flav == lock_flav && lock->start == start && lock->size == size) { @@ -1288,7 +1325,7 @@ void brl_close_fnum(struct byte_range_lock *br_lck) struct lock_struct *pend_lock = &locks[j]; /* Ignore our own or non-pending locks. */ - if (pend_lock->lock_type != PENDING_LOCK) { + if (!IS_PENDING_LOCK(pend_lock->lock_type)) { continue; } diff --git a/source3/locking/locking.c b/source3/locking/locking.c index 3879d40cba..4cd6b436c3 100644 --- a/source3/locking/locking.c +++ b/source3/locking/locking.c @@ -55,8 +55,10 @@ const char *lock_type_name(enum brl_type lock_type) return "READ"; case WRITE_LOCK: return "WRITE"; - case PENDING_LOCK: - return "PENDING"; + case PENDING_READ_LOCK: + return "PENDING_READ"; + case PENDING_WRITE_LOCK: + return "PENDING_WRITE"; default: return "other"; } diff --git a/source3/smbd/blocking.c b/source3/smbd/blocking.c index ed57c9f621..0304a6559f 100644 --- a/source3/smbd/blocking.c +++ b/source3/smbd/blocking.c @@ -137,7 +137,7 @@ BOOL push_blocking_lock_request( struct byte_range_lock *br_lck, procid_self(), offset, count, - PENDING_LOCK, + lock_type == READ_LOCK ? PENDING_READ_LOCK : PENDING_WRITE_LOCK, blr->lock_flav, lock_timeout ? True : False); /* blocking_lock. */ -- cgit