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)

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) 
@@ -211,6 +211,19 @@ static BOOL brl_conflict_other(const struct lock_struct *lck1, const struct lock
 } 
 
 /****************************************************************************
+ 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
  app depends on this ?
@@ -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;
 }
 
@@ -711,19 +761,6 @@ NTSTATUS brl_lock(struct byte_range_lock *br_lck,
 }
 
 /****************************************************************************
- 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. */