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>
diff --git a/Documentation/genlock.txt b/Documentation/genlock.txt
index 6f24a76..cd82614 100644
--- a/Documentation/genlock.txt
+++ b/Documentation/genlock.txt
@@ -34,6 +34,12 @@
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_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.
diff --git a/drivers/base/genlock.c b/drivers/base/genlock.c
index d9cd600..5e1d7af 100644
--- a/drivers/base/genlock.c
+++ b/drivers/base/genlock.c
@@ -307,12 +307,15 @@
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 @@
* 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 @@
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 @@
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 @@
}
/**
- * 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 @@
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 @@
return ret;
}
-EXPORT_SYMBOL(genlock_lock);
+EXPORT_SYMBOL(genlock_dreadlock);
/**
* genlock_wait - Wait for the lock to be released
@@ -657,6 +739,14 @@
return genlock_lock(handle, param.op, param.flags,
param.timeout);
}
+ case GENLOCK_IOC_DREADLOCK: {
+ if (copy_from_user(¶m, (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(¶m, (void __user *) arg,
sizeof(param)))
diff --git a/include/linux/genlock.h b/include/linux/genlock.h
index 9351a15..60bc84c 100644
--- a/include/linux/genlock.h
+++ b/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 @@
#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)
#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 @@
#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