base: genlock: allow synchronization with a single gralloc handle

In order to support synchronization in a process with a single
gralloc handle we require the ability to write lock a buffer
while it is already read locked by the same handle. This change
extends the concept of an exclusive write lock or recursive read
locks to a genlock handle (rather than the genlock lock).
Genlock cannot provide deadlock protection because the same
handle can be used simultaneously by a producer and consumer.
In practice an error will still be generated when the timeout
expires.

CRs-fixed: 356263
Change-Id: I322e7fadc8b43287f53b211242b176d3de731db2
Signed-off-by: Jeff Boody <jboody@codeaurora.org>

Documentation/genlock.txt

@@ -34,6 +34,12 @@ instance for a lock known as a handle. Handles can be shared between user
 space and kernel space to allow a kernel driver to unlock or lock a buffer
 on behalf of a user process.
 
+Locks within a process using a single genlock handle follow the same rules for
+exclusive write locks with multiple readers. Genlock cannot provide deadlock
+protection because the same handle can be used simultaneously by a producer and
+consumer. In practice in the event that the client creates a deadlock an error
+will still be generated when the timeout expires.
+
 Kernel API
 
 Access to the genlock API can either be via the in-kernel API or via an
@@ -137,7 +143,12 @@ genlock_lock.op:
  * GENLOCK_UNLOCK - unlock an existing lock
 
 Pass flags in genlock_lock.flags:
- * GENLOCK_NOBLOCK - Do not block if the lock is already taken
+ * GENLOCK_NOBLOCK       - Do not block if the lock is already taken
+ * GENLOCK_WRITE_TO_READ - Convert a write lock that the handle owns to a read
+                           lock. For instance graphics may hold a write lock
+                           while rendering the back buffer then when swapping
+                           convert the lock to a read lock to copy the front
+                           buffer in the next frame for preserved buffers.
 
 Pass a timeout value in milliseconds in genlock_lock.timeout.
 genlock_lock.flags and genlock_lock.timeout are not used for UNLOCK.

drivers/base/genlock.c

@@ -307,12 +307,15 @@ static int _genlock_lock(struct genlock *lock, struct genlock_handle *handle,
 	if (handle_has_lock(lock, handle)) {
 
 		/*
-		 * If the handle already holds the lock and the type matches,
-		 * then just increment the active pointer. This allows the
-		 * handle to do recursive locks
+		 * If the handle already holds the lock and the lock type is
+		 * a read lock then just increment the active pointer. This
+		 * allows the handle to do recursive read locks. Recursive
+		 * write locks are not allowed in order to support
+		 * synchronization within a process using a single gralloc
+		 * handle.
 		 */
 
-		if (lock->state == op) {
+		if (lock->state == _RDLOCK && op == _RDLOCK) {
 			handle->active++;
 			goto done;
 		}
@@ -321,33 +324,46 @@ static int _genlock_lock(struct genlock *lock, struct genlock_handle *handle,
 		 * If the handle holds a write lock then the owner can switch
 		 * to a read lock if they want. Do the transition atomically
 		 * then wake up any pending waiters in case they want a read
-		 * lock too.
+		 * lock too. In order to support synchronization within a
+		 * process the caller must explicity request to convert the
+		 * lock type with the GENLOCK_WRITE_TO_READ flag.
 		 */
 
-		if (op == _RDLOCK && handle->active == 1) {
-			lock->state = _RDLOCK;
-			wake_up(&lock->queue);
+		if (flags & GENLOCK_WRITE_TO_READ) {
+			if (lock->state == _WRLOCK && op == _RDLOCK) {
+				lock->state = _RDLOCK;
+				wake_up(&lock->queue);
+				goto done;
+			} else {
+				GENLOCK_LOG_ERR("Invalid state to convert"
+					"write to read\n");
+				ret = -EINVAL;
+				goto done;
+			}
+		}
+	} else {
+
+		/*
+		 * Check to ensure the caller has not attempted to convert a
+		 * write to a read without holding the lock.
+		 */
+
+		if (flags & GENLOCK_WRITE_TO_READ) {
+			GENLOCK_LOG_ERR("Handle must have lock to convert"
+				"write to read\n");
+			ret = -EINVAL;
 			goto done;
 		}
 
 		/*
-		 * Otherwise the user tried to turn a read into a write, and we
-		 * don't allow that.
+		 * If we request a read and the lock is held by a read, then go
+		 * ahead and share the lock
 		 */
-		GENLOCK_LOG_ERR("Trying to upgrade a read lock to a write"
-				"lock\n");
-		ret = -EINVAL;
-		goto done;
+
+		if (op == GENLOCK_RDLOCK && lock->state == _RDLOCK)
+			goto dolock;
 	}
 
-	/*
-	 * If we request a read and the lock is held by a read, then go
-	 * ahead and share the lock
-	 */
-
-	if (op == GENLOCK_RDLOCK && lock->state == _RDLOCK)
-		goto dolock;
-
 	/* Treat timeout 0 just like a NOBLOCK flag and return if the
 	   lock cannot be aquired without blocking */
 
@@ -356,15 +372,26 @@ static int _genlock_lock(struct genlock *lock, struct genlock_handle *handle,
 		goto done;
 	}
 
-	/* Wait while the lock remains in an incompatible state */
+	/*
+	 * Wait while the lock remains in an incompatible state
+	 * state    op    wait
+	 * -------------------
+	 * unlocked n/a   no
+	 * read     read  no
+	 * read     write yes
+	 * write    n/a   yes
+	 */
 
-	while (lock->state != _UNLOCKED) {
+	while ((lock->state == _RDLOCK && op == _WRLOCK) ||
+			lock->state == _WRLOCK) {
 		signed long elapsed;
 
 		spin_unlock_irqrestore(&lock->lock, irqflags);
 
 		elapsed = wait_event_interruptible_timeout(lock->queue,
-			lock->state == _UNLOCKED, ticks);
+			lock->state == _UNLOCKED ||
+			(lock->state == _RDLOCK && op == _RDLOCK),
+			ticks);
 
 		spin_lock_irqsave(&lock->lock, irqflags);
 
@@ -381,7 +408,7 @@ dolock:
 
 	list_add_tail(&handle->entry, &lock->active);
 	lock->state = op;
-	handle->active = 1;
+	handle->active++;
 
 done:
 	spin_unlock_irqrestore(&lock->lock, irqflags);
@@ -390,7 +417,7 @@ done:
 }
 
 /**
- * genlock_lock - Acquire or release a lock
+ * genlock_lock - Acquire or release a lock (depreciated)
  * @handle - pointer to the genlock handle that is requesting the lock
  * @op - the operation to perform (RDLOCK, WRLOCK, UNLOCK)
  * @flags - flags to control the operation
@@ -403,6 +430,61 @@ int genlock_lock(struct genlock_handle *handle, int op, int flags,
 	uint32_t timeout)
 {
 	struct genlock *lock;
+	unsigned long irqflags;
+
+	int ret = 0;
+
+	if (IS_ERR_OR_NULL(handle)) {
+		GENLOCK_LOG_ERR("Invalid handle\n");
+		return -EINVAL;
+	}
+
+	lock = handle->lock;
+
+	if (lock == NULL) {
+		GENLOCK_LOG_ERR("Handle does not have a lock attached\n");
+		return -EINVAL;
+	}
+
+	switch (op) {
+	case GENLOCK_UNLOCK:
+		ret = _genlock_unlock(lock, handle);
+		break;
+	case GENLOCK_RDLOCK:
+		spin_lock_irqsave(&lock->lock, irqflags);
+		if (handle_has_lock(lock, handle)) {
+			/* request the WRITE_TO_READ flag for compatibility */
+			flags |= GENLOCK_WRITE_TO_READ;
+		}
+		spin_unlock_irqrestore(&lock->lock, irqflags);
+		/* fall through to take lock */
+	case GENLOCK_WRLOCK:
+		ret = _genlock_lock(lock, handle, op, flags, timeout);
+		break;
+	default:
+		GENLOCK_LOG_ERR("Invalid lock operation\n");
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL(genlock_lock);
+
+/**
+ * genlock_dreadlock - Acquire or release a lock
+ * @handle - pointer to the genlock handle that is requesting the lock
+ * @op - the operation to perform (RDLOCK, WRLOCK, UNLOCK)
+ * @flags - flags to control the operation
+ * @timeout - optional timeout to wait for the lock to come free
+ *
+ * Returns: 0 on success or error code on failure
+ */
+
+int genlock_dreadlock(struct genlock_handle *handle, int op, int flags,
+	uint32_t timeout)
+{
+	struct genlock *lock;
 
 	int ret = 0;
 
@@ -434,7 +516,7 @@ int genlock_lock(struct genlock_handle *handle, int op, int flags,
 
 	return ret;
 }
-EXPORT_SYMBOL(genlock_lock);
+EXPORT_SYMBOL(genlock_dreadlock);
 
 /**
  * genlock_wait - Wait for the lock to be released
@@ -657,6 +739,14 @@ static long genlock_dev_ioctl(struct file *filep, unsigned int cmd,
 		return genlock_lock(handle, param.op, param.flags,
 			param.timeout);
 	}
+	case GENLOCK_IOC_DREADLOCK: {
+		if (copy_from_user(&param, (void __user *) arg,
+		sizeof(param)))
+			return -EFAULT;
+
+		return genlock_dreadlock(handle, param.op, param.flags,
+			param.timeout);
+	}
 	case GENLOCK_IOC_WAIT: {
 		if (copy_from_user(&param, (void __user *) arg,
 		sizeof(param)))

include/linux/genlock.h

@@ -1,6 +1,8 @@
 #ifndef _GENLOCK_H_
 #define _GENLOCK_H_
 
+#include <linux/bitops.h>
+
 #ifdef __KERNEL__
 
 struct genlock;
@@ -21,7 +23,8 @@ int genlock_lock(struct genlock_handle *handle, int op, int flags,
 #define GENLOCK_WRLOCK 1
 #define GENLOCK_RDLOCK 2
 
-#define GENLOCK_NOBLOCK (1 << 0)
+#define GENLOCK_NOBLOCK       BIT(0)
+#define GENLOCK_WRITE_TO_READ BIT(1)
 
 struct genlock_lock {
 	int fd;
@@ -37,6 +40,8 @@ struct genlock_lock {
 	struct genlock_lock)
 #define GENLOCK_IOC_ATTACH _IOW(GENLOCK_IOC_MAGIC, 2, \
 	struct genlock_lock)
+
+/* Deprecated */
 #define GENLOCK_IOC_LOCK _IOW(GENLOCK_IOC_MAGIC, 3, \
 	struct genlock_lock)
 
@@ -44,4 +49,6 @@ struct genlock_lock {
 #define GENLOCK_IOC_RELEASE _IO(GENLOCK_IOC_MAGIC, 4)
 #define GENLOCK_IOC_WAIT _IOW(GENLOCK_IOC_MAGIC, 5, \
 	struct genlock_lock)
+#define GENLOCK_IOC_DREADLOCK _IOW(GENLOCK_IOC_MAGIC, 6, \
+	struct genlock_lock)
 #endif