From 1aa4bfe39e6114e9110fc60d26034d9f85be71da Mon Sep 17 00:00:00 2001 From: Jeff Boody Date: Thu, 26 Apr 2012 11:12:44 -0600 Subject: [PATCH] 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 Signed-off-by: Stephen Boyd --- Documentation/genlock.txt | 13 +++- drivers/base/genlock.c | 146 ++++++++++++++++++++++++++++++-------- include/linux/genlock.h | 7 +- 3 files changed, 136 insertions(+), 30 deletions(-) diff --git a/Documentation/genlock.txt b/Documentation/genlock.txt index 6f24a7695615..cd8261467748 100644 --- a/Documentation/genlock.txt +++ b/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. diff --git a/drivers/base/genlock.c b/drivers/base/genlock.c index d9cd60028246..5e1d7af5e529 100644 --- a/drivers/base/genlock.c +++ b/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(¶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 9351a15626b6..587c49df7444 100644 --- a/include/linux/genlock.h +++ b/include/linux/genlock.h @@ -21,7 +21,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 (1 << 0) +#define GENLOCK_WRITE_TO_READ (1 << 1) struct genlock_lock { int fd; @@ -37,6 +38,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 +47,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