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/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)))