[PATCH 0/4] acpi semaphore removal v3

[PATCH 0/4] acpi semaphore removal v3

[Daniel Walker]

I fixed the lockdep issue. It was due to the mutex changes calling
mutex_lock_timeout with a timeout of 0. That acts like a trylock, but it doesn't
get treated by lockdep as a trylock. Now when the timeout is zero we just trylock.
I also removed completion_done() since it was already merged.

I think this is merge ready now ..

Daniel

[PATCH 1/4] add mutex_lock_timeout()

[Daniel Walker]

Adds mutex_lock_timeout() and mutex_lock_timeout_nested() used inside ACPI.

Cc: linux-acpi@vger.kernel.org
Signed-off-by: Daniel Walker <dwalker@mvista.com>
---
 include/asm-generic/mutex-dec.h  |   23 ++++++++++++
 include/asm-generic/mutex-null.h |    3 ++
 include/asm-generic/mutex-xchg.h |   23 ++++++++++++
 include/asm-x86/mutex_32.h       |   21 +++++++++++
 include/asm-x86/mutex_64.h       |   21 +++++++++++
 include/linux/mutex.h            |    8 ++++
 kernel/mutex.c                   |   71 +++++++++++++++++++++++++++++++++-----
 7 files changed, 161 insertions(+), 9 deletions(-)

diff --git a/include/asm-generic/mutex-dec.h b/include/asm-generic/mutex-dec.h
index ed108be..eddc8c4 100644
--- a/include/asm-generic/mutex-dec.h
+++ b/include/asm-generic/mutex-dec.h
@@ -109,4 +109,27 @@ __mutex_fastpath_trylock(atomic_t *count, int (*fail_fn)(atomic_t *))
 #endif
 }
 
+/**
+ *  __mutex_fastpath_lock_timeout - try to take the lock by moving the count
+ *                                  from 1 to a 0 value
+ *  @count: pointer of type atomic_t
+ *  @fail_fn: function to call if the original value was not 1
+ *  	      This function will need to accept two arguments.
+ *  @timeout: Timeout value as second argument to fail_fn.
+ *
+ * Change the count from 1 to a value lower than 1, and call <fail_fn> if
+ * it wasn't 1 originally. This function returns 0 if the fastpath succeeds,
+ * or anything the slow path function returns.
+ */
+static inline int
+__mutex_fastpath_lock_timeout(atomic_t *count, long timeout,
+					int (*fail_fn)(atomic_t *, long))
+{
+	if (unlikely(atomic_dec_return(count) < 0))
+		return fail_fn(count, timeout);
+	else {
+		smp_mb();
+		return 0;
+	}
+}
 #endif
diff --git a/include/asm-generic/mutex-null.h b/include/asm-generic/mutex-null.h
index e1bbbc7..192a756 100644
--- a/include/asm-generic/mutex-null.h
+++ b/include/asm-generic/mutex-null.h
@@ -14,6 +14,9 @@
 #define __mutex_fastpath_lock_retval(count, fail_fn)	fail_fn(count)
 #define __mutex_fastpath_unlock(count, fail_fn)		fail_fn(count)
 #define __mutex_fastpath_trylock(count, fail_fn)	fail_fn(count)
+
+#define __mutex_fastpath_timeout(count, timeout, fail_fn) \
+	fail_fn(count, timeout)
 #define __mutex_slowpath_needs_to_unlock()		1
 
 #endif
diff --git a/include/asm-generic/mutex-xchg.h b/include/asm-generic/mutex-xchg.h
index 7b9cd2c..34ccdd3 100644
--- a/include/asm-generic/mutex-xchg.h
+++ b/include/asm-generic/mutex-xchg.h
@@ -115,4 +115,27 @@ __mutex_fastpath_trylock(atomic_t *count, int (*fail_fn)(atomic_t *))
 	return prev;
 }
 
+/**
+ *  __mutex_fastpath_lock_timeout - try to take the lock by moving the count
+ *                                  from 1 to a 0 value
+ *  @count: pointer of type atomic_t
+ *  @fail_fn: function to call if the original value was not 1
+ *	      This function will need to accept two arguments.
+ *  @timeout: Timeout value as second argument to fail_fn.
+ *
+ * Change the count from 1 to a value lower than 1, and call <fail_fn> if it
+ * wasn't 1 originally. This function returns 0 if the fastpath succeeds,
+ * or anything the slow path function returns
+ */
+static inline int
+__mutex_fastpath_lock_timeout(atomic_t *count, timeout,
+			      int (*fail_fn)(atomic_t *, long))
+{
+	if (unlikely(atomic_xchg(count, 0) != 1))
+		return fail_fn(count, timeout);
+	else {
+		smp_mb();
+		return 0;
+	}
+}
 #endif
diff --git a/include/asm-x86/mutex_32.h b/include/asm-x86/mutex_32.h
index 73e928e..7d4696d 100644
--- a/include/asm-x86/mutex_32.h
+++ b/include/asm-x86/mutex_32.h
@@ -122,4 +122,25 @@ static inline int __mutex_fastpath_trylock(atomic_t *count,
 #endif
 }
 
+/**
+ *  __mutex_fastpath_lock_timeout - try to take the lock by moving the count
+ *                                  from 1 to a 0 value
+ *  @count: pointer of type atomic_t
+ *  @fail_fn: function to call if the original value was not 1
+ *	      This function will need to accept two arguments.
+ *  @timeout: Timeout value as second argument to fail_fn.
+ *
+ * Change the count from 1 to a value lower than 1, and call <fail_fn> if it
+ * wasn't 1 originally. This function returns 0 if the fastpath succeeds,
+ * or anything the slow path function returns
+ */
+static inline int
+__mutex_fastpath_lock_timeout(atomic_t *count, long timeout,
+			      int (*fail_fn)(atomic_t *, long))
+{
+	if (unlikely(atomic_dec_return(count) < 0))
+		return fail_fn(count, timeout);
+	else
+		return 0;
+}
 #endif
diff --git a/include/asm-x86/mutex_64.h b/include/asm-x86/mutex_64.h
index f3fae9b..3e63b61 100644
--- a/include/asm-x86/mutex_64.h
+++ b/include/asm-x86/mutex_64.h
@@ -97,4 +97,25 @@ static inline int __mutex_fastpath_trylock(atomic_t *count,
 		return 0;
 }
 
+/**
+ *  __mutex_fastpath_lock_timeout - try to take the lock by moving the count
+ *                                  from 1 to a 0 value
+ *  @count: pointer of type atomic_t
+ *  @fail_fn: function to call if the original value was not 1
+ *	      This function will need to accept two arguments.
+ *  @timeout: Timeout value as second argument to fail_fn.
+ *
+ * Change the count from 1 to a value lower than 1, and call <fail_fn> if
+ * it wasn't 1 originally. This function returns 0 if the fastpath succeeds,
+ * or anything the slow path function returns
+ */
+static inline int
+__mutex_fastpath_lock_timeout(atomic_t *count, long timeout,
+			      int (*fail_fn)(atomic_t *, long))
+{
+	if (unlikely(atomic_dec_return(count) < 0))
+		return fail_fn(count, timeout);
+	else
+		return 0;
+}
 #endif
diff --git a/include/linux/mutex.h b/include/linux/mutex.h
index bc6da10..bb84cd4 100644
--- a/include/linux/mutex.h
+++ b/include/linux/mutex.h
@@ -127,18 +127,26 @@ extern int __must_check mutex_lock_interruptible_nested(struct mutex *lock,
 					unsigned int subclass);
 extern int __must_check mutex_lock_killable_nested(struct mutex *lock,
 					unsigned int subclass);
+extern int __must_check
+mutex_lock_timeout_nested(struct mutex *lock, long jiffies,
+			  unsigned int subclass);
 
 #define mutex_lock(lock) mutex_lock_nested(lock, 0)
 #define mutex_lock_interruptible(lock) mutex_lock_interruptible_nested(lock, 0)
 #define mutex_lock_killable(lock) mutex_lock_killable_nested(lock, 0)
+#define mutex_lock_timeout(lock, jiffies) \
+	mutex_lock_timeout_nested(lock, jiffies, 0)
 #else
 extern void mutex_lock(struct mutex *lock);
 extern int __must_check mutex_lock_interruptible(struct mutex *lock);
 extern int __must_check mutex_lock_killable(struct mutex *lock);
+extern int __must_check mutex_lock_timeout(struct mutex *lock, long jiffies);
 
 # define mutex_lock_nested(lock, subclass) mutex_lock(lock)
 # define mutex_lock_interruptible_nested(lock, subclass) mutex_lock_interruptible(lock)
 # define mutex_lock_killable_nested(lock, subclass) mutex_lock_killable(lock)
+# define mutex_lock_timeout_nested(lock, jiffies, subclass) \
+	mutex_lock_timeout(lock, jiffies)
 #endif
 
 /*
diff --git a/kernel/mutex.c b/kernel/mutex.c
index 12c779d..cf06f62 100644
--- a/kernel/mutex.c
+++ b/kernel/mutex.c
@@ -124,8 +124,8 @@ EXPORT_SYMBOL(mutex_unlock);
  * Lock a mutex (possibly interruptible), slowpath:
  */
 static inline int __sched
-__mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
-	       	unsigned long ip)
+__mutex_lock_common(struct mutex *lock, long state, long timeout,
+		    unsigned int subclass, unsigned long ip)
 {
 	struct task_struct *task = current;
 	struct mutex_waiter waiter;
@@ -179,7 +179,22 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
 
 		/* didnt get the lock, go to sleep: */
 		spin_unlock_mutex(&lock->wait_lock, flags);
-		schedule();
+		if (timeout == MAX_SCHEDULE_TIMEOUT)
+			schedule();
+		else {
+			timeout = schedule_timeout(timeout);
+
+			if (timeout == 0) {
+				spin_lock_mutex(&lock->wait_lock, flags);
+				mutex_remove_waiter(lock, &waiter,
+						    task_thread_info(task));
+				mutex_release(&lock->dep_map, 1, ip);
+				spin_unlock_mutex(&lock->wait_lock, flags);
+
+				debug_mutex_free_waiter(&waiter);
+				return -ETIME;
+			}
+		}
 		spin_lock_mutex(&lock->wait_lock, flags);
 	}
 
@@ -205,7 +220,8 @@ void __sched
 mutex_lock_nested(struct mutex *lock, unsigned int subclass)
 {
 	might_sleep();
-	__mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, subclass, _RET_IP_);
+	__mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT,
+			    subclass, _RET_IP_);
 }
 
 EXPORT_SYMBOL_GPL(mutex_lock_nested);
@@ -214,7 +230,8 @@ int __sched
 mutex_lock_killable_nested(struct mutex *lock, unsigned int subclass)
 {
 	might_sleep();
-	return __mutex_lock_common(lock, TASK_KILLABLE, subclass, _RET_IP_);
+	return __mutex_lock_common(lock, TASK_KILLABLE, MAX_SCHEDULE_TIMEOUT,
+				   subclass, _RET_IP_);
 }
 EXPORT_SYMBOL_GPL(mutex_lock_killable_nested);
 
@@ -222,10 +239,23 @@ int __sched
 mutex_lock_interruptible_nested(struct mutex *lock, unsigned int subclass)
 {
 	might_sleep();
-	return __mutex_lock_common(lock, TASK_INTERRUPTIBLE, subclass, _RET_IP_);
+	return __mutex_lock_common(lock, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT,
+				   subclass, _RET_IP_);
 }
 
 EXPORT_SYMBOL_GPL(mutex_lock_interruptible_nested);
+
+
+int __sched
+mutex_lock_timeout_nested(struct mutex *lock, long timeout,
+			  unsigned int subclass)
+{
+	might_sleep();
+	return __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, timeout,
+				   subclass, _RET_IP_);
+}
+EXPORT_SYMBOL_GPL(mutex_lock_timeout_nested);
+
 #endif
 
 /*
@@ -285,6 +315,9 @@ __mutex_lock_killable_slowpath(atomic_t *lock_count);
 static noinline int __sched
 __mutex_lock_interruptible_slowpath(atomic_t *lock_count);
 
+static noinline int __sched
+__mutex_lock_timeout_slowpath(atomic_t *lock_count, long timeout);
+
 /***
  * mutex_lock_interruptible - acquire the mutex, interruptable
  * @lock: the mutex to be acquired
@@ -313,12 +346,21 @@ int __sched mutex_lock_killable(struct mutex *lock)
 }
 EXPORT_SYMBOL(mutex_lock_killable);
 
+int __sched mutex_lock_timeout(struct mutex *lock, long timeout)
+{
+	might_sleep();
+	return __mutex_fastpath_lock_timeout
+			(&lock->count, timeout, __mutex_lock_timeout_slowpath);
+}
+EXPORT_SYMBOL(mutex_lock_timeout);
+
 static noinline void __sched
 __mutex_lock_slowpath(atomic_t *lock_count)
 {
 	struct mutex *lock = container_of(lock_count, struct mutex, count);
 
-	__mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, 0, _RET_IP_);
+	__mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT,
+			    _RET_IP_);
 }
 
 static noinline int __sched
@@ -326,7 +368,8 @@ __mutex_lock_killable_slowpath(atomic_t *lock_count)
 {
 	struct mutex *lock = container_of(lock_count, struct mutex, count);
 
-	return __mutex_lock_common(lock, TASK_KILLABLE, 0, _RET_IP_);
+	return __mutex_lock_common(lock, TASK_KILLABLE, MAX_SCHEDULE_TIMEOUT,
+				   _RET_IP_);
 }
 
 static noinline int __sched
@@ -334,7 +377,17 @@ __mutex_lock_interruptible_slowpath(atomic_t *lock_count)
 {
 	struct mutex *lock = container_of(lock_count, struct mutex, count);
 
-	return __mutex_lock_common(lock, TASK_INTERRUPTIBLE, 0, _RET_IP_);
+	return __mutex_lock_common(lock, TASK_INTERRUPTIBLE,
+				   MAX_SCHEDULE_TIMEOUT, _RET_IP_);
+}
+
+static noinline int __sched
+__mutex_lock_timeout_slowpath(atomic_t *lock_count, long timeout)
+{
+	struct mutex *lock = container_of(lock_count, struct mutex, count);
+
+	return __mutex_lock_common(lock, TASK_INTERRUPTIBLE, timeout,
+				   _RET_IP_);
 }
 #endif
 
-- 
1.5.5.1.32.gba7d2

[PATCH 2/4] acpi: add real mutex function calls

[Daniel Walker]

Instead of re-using semaphores for the mutex operation, I've
added usage of the kernel mutex for the acpi os mutex
implementation.

Cc: linux-acpi@vger.kernel.org
Signed-off-by: Daniel Walker <dwalker@mvista.com>
---
 drivers/acpi/osl.c      |   93 +++++++++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpiosxf.h |   11 +-----
 2 files changed, 95 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 235a138..e6f7337 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -871,6 +871,99 @@ acpi_status acpi_os_signal_semaphore(acpi_handle handle, u32 units)
 	return AE_OK;
 }
 
+acpi_status
+acpi_os_create_mutex(acpi_mutex *handle)
+{
+	struct mutex *mutex = NULL;
+
+	mutex = acpi_os_allocate(sizeof(struct mutex));
+	if (!mutex)
+		return AE_NO_MEMORY;
+	memset(mutex, 0, sizeof(struct mutex));
+
+	mutex_init(mutex);
+
+	*handle = (acpi_handle *) mutex;
+
+	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Creating mutex[%p].\n",
+			  *handle));
+
+	return AE_OK;
+}
+
+acpi_status acpi_os_delete_mutex(acpi_mutex handle)
+{
+	struct mutex *mutex = (struct mutex *)handle;
+
+	if (!mutex)
+		return AE_BAD_PARAMETER;
+
+	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Deleting mutex[%p].\n", handle));
+
+	BUG_ON(mutex_is_locked(mutex));
+	kfree(mutex);
+	mutex = NULL;
+
+	return AE_OK;
+}
+
+acpi_status acpi_os_acquire_mutex(acpi_mutex handle, u16 timeout)
+{
+	acpi_status status = AE_OK;
+	struct mutex *mutex = (struct mutex *)handle;
+	long jiffies;
+	int ret = 0;
+
+	if (!mutex)
+		return AE_BAD_PARAMETER;
+
+	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Waiting for mutex[%p|%d]\n",
+			  handle, timeout));
+
+	if (timeout == ACPI_DO_NOT_WAIT) {
+		if (mutex_trylock(mutex))
+			return status;
+		else
+			return -ETIME;
+	}
+
+	if (timeout == ACPI_WAIT_FOREVER)
+		jiffies = MAX_SCHEDULE_TIMEOUT;
+	else
+		jiffies = msecs_to_jiffies(timeout);
+
+	ret = mutex_lock_timeout(mutex, jiffies);
+	if (ret == -ETIME)
+		status = AE_TIME;
+
+	if (ACPI_FAILURE(status)) {
+		ACPI_DEBUG_PRINT((ACPI_DB_MUTEX,
+				  "Failed to acquire mutex[%p|%d], %s",
+				  handle, timeout,
+				  acpi_format_exception(status)));
+	} else {
+		ACPI_DEBUG_PRINT((ACPI_DB_MUTEX,
+				  "Acquired mutex[%p|%d]", handle,
+				  timeout));
+	}
+
+	return status;
+}
+
+acpi_status acpi_os_release_mutex(acpi_mutex handle)
+{
+	struct mutex *mutex = (struct mutex *)handle;
+
+	if (!mutex)
+		return AE_BAD_PARAMETER;
+
+	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Signaling mutex[%p]\n", handle));
+
+	mutex_unlock(mutex);
+
+	return AE_OK;
+}
+
 #ifdef ACPI_FUTURE_USAGE
 u32 acpi_os_get_line(char *buffer)
 {
diff --git a/include/acpi/acpiosxf.h b/include/acpi/acpiosxf.h
index 3f93a6b..9032ec3 100644
--- a/include/acpi/acpiosxf.h
+++ b/include/acpi/acpiosxf.h
@@ -125,18 +125,11 @@ acpi_status acpi_os_signal_semaphore(acpi_semaphore handle, u32 units);
  */
 acpi_status acpi_os_create_mutex(acpi_mutex * out_handle);
 
-void acpi_os_delete_mutex(acpi_mutex handle);
+acpi_status acpi_os_delete_mutex(acpi_mutex handle);
 
 acpi_status acpi_os_acquire_mutex(acpi_mutex handle, u16 timeout);
 
-void acpi_os_release_mutex(acpi_mutex handle);
-
-/* Temporary macros for Mutex* interfaces, map to existing semaphore xfaces */
-
-#define acpi_os_create_mutex(out_handle)    acpi_os_create_semaphore (1, 1, out_handle)
-#define acpi_os_delete_mutex(handle)        (void) acpi_os_delete_semaphore (handle)
-#define acpi_os_acquire_mutex(handle,time)  acpi_os_wait_semaphore (handle, 1, time)
-#define acpi_os_release_mutex(handle)       (void) acpi_os_signal_semaphore (handle, 1)
+acpi_status acpi_os_release_mutex(acpi_mutex handle);
 
 /*
  * Memory allocation and mapping
-- 
1.5.5.1.32.gba7d2

[PATCH 3/4] acpi: add lockdep magic

[Daniel Walker]

Add lockdep integration for the ACPI mutex usage.

Cc: linux-acpi@vger.kernel.org
Signed-off-by: Daniel Walker <dwalker@mvista.com>
---
 drivers/acpi/dispatcher/dsmethod.c |    4 ++++
 drivers/acpi/executer/excreate.c   |    7 ++++++-
 drivers/acpi/executer/exmutex.c    |    5 +++--
 drivers/acpi/executer/exsystem.c   |    3 ++-
 drivers/acpi/namespace/nsaccess.c  |    5 +++++
 drivers/acpi/osl.c                 |    3 ++-
 drivers/acpi/utilities/utmutex.c   |    6 ++++++
 include/acpi/acpiosxf.h            |    6 ++++--
 8 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/dispatcher/dsmethod.c b/drivers/acpi/dispatcher/dsmethod.c
index 4613b9c..a3cf3d4 100644
--- a/drivers/acpi/dispatcher/dsmethod.c
+++ b/drivers/acpi/dispatcher/dsmethod.c
@@ -130,6 +130,7 @@ acpi_ds_method_error(acpi_status status, struct acpi_walk_state *walk_state)
 static acpi_status
 acpi_ds_create_method_mutex(union acpi_operand_object *method_desc)
 {
+	static struct lock_class_key method_mutex;
 	union acpi_operand_object *mutex_desc;
 	acpi_status status;
 
@@ -149,6 +150,9 @@ acpi_ds_create_method_mutex(union acpi_operand_object *method_desc)
 		return_ACPI_STATUS(status);
 	}
 
+	lockdep_set_class((struct mutex *)&mutex_desc->mutex.os_mutex,
+			  &method_mutex);
+
 	mutex_desc->mutex.sync_level = method_desc->method.sync_level;
 	method_desc->method.mutex = mutex_desc;
 	return_ACPI_STATUS(AE_OK);
diff --git a/drivers/acpi/executer/excreate.c b/drivers/acpi/executer/excreate.c
index ad09696..0700c02 100644
--- a/drivers/acpi/executer/excreate.c
+++ b/drivers/acpi/executer/excreate.c
@@ -222,6 +222,7 @@ acpi_status acpi_ex_create_mutex(struct acpi_walk_state *walk_state)
 {
 	acpi_status status = AE_OK;
 	union acpi_operand_object *obj_desc;
+	static struct lock_class_key interpreter_class;
 
 	ACPI_FUNCTION_TRACE_PTR(ex_create_mutex, ACPI_WALK_OPERANDS);
 
@@ -235,11 +236,15 @@ acpi_status acpi_ex_create_mutex(struct acpi_walk_state *walk_state)
 
 	/* Create the actual OS Mutex */
 
-	status = acpi_os_create_mutex(&obj_desc->mutex.os_mutex);
+	status =
+	acpi_os_create_mutex(&obj_desc->mutex.os_mutex);
 	if (ACPI_FAILURE(status)) {
 		goto cleanup;
 	}
 
+	lockdep_set_class((struct mutex *)obj_desc->mutex.os_mutex,
+			  &interpreter_class);
+
 	/* Init object and attach to NS node */
 
 	obj_desc->mutex.sync_level =
diff --git a/drivers/acpi/executer/exmutex.c b/drivers/acpi/executer/exmutex.c
index a8bf3d7..6472a24 100644
--- a/drivers/acpi/executer/exmutex.c
+++ b/drivers/acpi/executer/exmutex.c
@@ -176,8 +176,9 @@ acpi_ex_acquire_mutex_object(u16 timeout,
 	if (obj_desc == acpi_gbl_global_lock_mutex) {
 		status = acpi_ev_acquire_global_lock(timeout);
 	} else {
-		status = acpi_ex_system_wait_mutex(obj_desc->mutex.os_mutex,
-						   timeout);
+		status =
+		acpi_ex_system_wait_mutex(obj_desc->mutex.os_mutex,
+						 timeout);
 	}
 
 	if (ACPI_FAILURE(status)) {
diff --git a/drivers/acpi/executer/exsystem.c b/drivers/acpi/executer/exsystem.c
index 68990f1..1a4aa5e 100644
--- a/drivers/acpi/executer/exsystem.c
+++ b/drivers/acpi/executer/exsystem.c
@@ -108,7 +108,8 @@ acpi_status acpi_ex_system_wait_semaphore(acpi_semaphore semaphore, u16 timeout)
  *
  ******************************************************************************/
 
-acpi_status acpi_ex_system_wait_mutex(acpi_mutex mutex, u16 timeout)
+acpi_status
+acpi_ex_system_wait_mutex(acpi_mutex mutex, u16 timeout)
 {
 	acpi_status status;
 
diff --git a/drivers/acpi/namespace/nsaccess.c b/drivers/acpi/namespace/nsaccess.c
index c39a7f6..293c0ba 100644
--- a/drivers/acpi/namespace/nsaccess.c
+++ b/drivers/acpi/namespace/nsaccess.c
@@ -64,6 +64,7 @@ ACPI_MODULE_NAME("nsaccess")
  ******************************************************************************/
 acpi_status acpi_ns_root_initialize(void)
 {
+	static struct lock_class_key ns_root_mutex;
 	acpi_status status;
 	const struct acpi_predefined_names *init_val = NULL;
 	struct acpi_namespace_node *new_node;
@@ -205,6 +206,10 @@ acpi_status acpi_ns_root_initialize(void)
 					goto unlock_and_exit;
 				}
 
+				lockdep_set_class((struct mutex *)
+					obj_desc->mutex.os_mutex,
+					&ns_root_mutex);
+
 				/* Special case for ACPI Global Lock */
 
 				if (ACPI_STRCMP(init_val->name, "_GL_") == 0) {
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index e6f7337..3c6c228 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -907,7 +907,8 @@ acpi_status acpi_os_delete_mutex(acpi_mutex handle)
 	return AE_OK;
 }
 
-acpi_status acpi_os_acquire_mutex(acpi_mutex handle, u16 timeout)
+acpi_status
+acpi_os_acquire_mutex(acpi_mutex handle, u16 timeout)
 {
 	acpi_status status = AE_OK;
 	struct mutex *mutex = (struct mutex *)handle;
diff --git a/drivers/acpi/utilities/utmutex.c b/drivers/acpi/utilities/utmutex.c
index 7331dde..3f7f16c 100644
--- a/drivers/acpi/utilities/utmutex.c
+++ b/drivers/acpi/utilities/utmutex.c
@@ -134,6 +134,7 @@ void acpi_ut_mutex_terminate(void)
 
 static acpi_status acpi_ut_create_mutex(acpi_mutex_handle mutex_id)
 {
+	static struct lock_class_key utility_class_keys[ACPI_MAX_MUTEX];
 	acpi_status status = AE_OK;
 
 	ACPI_FUNCTION_TRACE_U32(ut_create_mutex, mutex_id);
@@ -145,6 +146,11 @@ static acpi_status acpi_ut_create_mutex(acpi_mutex_handle mutex_id)
 	if (!acpi_gbl_mutex_info[mutex_id].mutex) {
 		status =
 		    acpi_os_create_mutex(&acpi_gbl_mutex_info[mutex_id].mutex);
+
+		lockdep_set_class(
+			(struct mutex *)acpi_gbl_mutex_info[mutex_id].mutex,
+			&utility_class_keys[mutex_id]);
+
 		acpi_gbl_mutex_info[mutex_id].thread_id =
 		    ACPI_MUTEX_NOT_ACQUIRED;
 		acpi_gbl_mutex_info[mutex_id].use_count = 0;
diff --git a/include/acpi/acpiosxf.h b/include/acpi/acpiosxf.h
index 9032ec3..ecf1e4d 100644
--- a/include/acpi/acpiosxf.h
+++ b/include/acpi/acpiosxf.h
@@ -123,11 +123,13 @@ acpi_status acpi_os_signal_semaphore(acpi_semaphore handle, u32 units);
 /*
  * Mutex primitives
  */
-acpi_status acpi_os_create_mutex(acpi_mutex * out_handle);
+acpi_status
+acpi_os_create_mutex(acpi_mutex *handle);
 
 acpi_status acpi_os_delete_mutex(acpi_mutex handle);
 
-acpi_status acpi_os_acquire_mutex(acpi_mutex handle, u16 timeout);
+acpi_status
+acpi_os_acquire_mutex(acpi_mutex handle, u16 timeout);
 
 acpi_status acpi_os_release_mutex(acpi_mutex handle);
 
-- 
1.5.5.1.32.gba7d2

[PATCH 4/4] acpi: semaphore removal

[Daniel Walker]

The semaphore usage in ACPI is more like completions. The ASL
functions getting implemented here are signals which follow a
"wait for", signaled, or reset format.

This implements the ACPI signaling methods with the Linux
completion API, instead of using semaphores.

Cc: linux-acpi@vger.kernel.org
Signed-off-by: Daniel Walker <dwalker@mvista.com>
---
 drivers/acpi/osl.c |   54 ++++++++++++++++++++++++++-------------------------
 1 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 3c6c228..dfd25d3 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -43,7 +43,7 @@
 #include <linux/ioport.h>
 #include <linux/list.h>
 #include <linux/jiffies.h>
-#include <linux/semaphore.h>
+#include <linux/completion.h>
 
 #include <asm/io.h>
 #include <asm/uaccess.h>
@@ -768,42 +768,44 @@ void acpi_os_delete_lock(acpi_spinlock handle)
 acpi_status
 acpi_os_create_semaphore(u32 max_units, u32 initial_units, acpi_handle * handle)
 {
-	struct semaphore *sem = NULL;
+	struct completion *comp = NULL;
 
-	sem = acpi_os_allocate(sizeof(struct semaphore));
-	if (!sem)
+	BUG_ON(initial_units);
+
+	comp = acpi_os_allocate(sizeof(struct completion));
+	if (!comp)
 		return AE_NO_MEMORY;
-	memset(sem, 0, sizeof(struct semaphore));
+	memset(comp, 0, sizeof(struct completion));
 
-	sema_init(sem, initial_units);
+	init_completion(comp);
 
-	*handle = (acpi_handle *) sem;
+	*handle = (acpi_handle *) comp;
 
-	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Creating semaphore[%p|%d].\n",
+	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Creating completion[%p|%d].\n",
 			  *handle, initial_units));
 
 	return AE_OK;
 }
 
 /*
- * TODO: A better way to delete semaphores?  Linux doesn't have a
- * 'delete_semaphore()' function -- may result in an invalid
+ * TODO: A better way to delete completions?  Linux doesn't have a
+ * 'delete_completions()' function -- may result in an invalid
  * pointer dereference for non-synchronized consumers.	Should
  * we at least check for blocked threads and signal/cancel them?
  */
 
 acpi_status acpi_os_delete_semaphore(acpi_handle handle)
 {
-	struct semaphore *sem = (struct semaphore *)handle;
+	struct completion *comp = (struct completion *)handle;
 
-	if (!sem)
+	if (!comp)
 		return AE_BAD_PARAMETER;
 
-	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Deleting semaphore[%p].\n", handle));
+	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Deleting completion[%p].\n", handle));
 
-	BUG_ON(!list_empty(&sem->wait_list));
-	kfree(sem);
-	sem = NULL;
+	BUG_ON(!completion_done(comp));
+	kfree(comp);
+	comp = NULL;
 
 	return AE_OK;
 }
@@ -814,17 +816,17 @@ acpi_status acpi_os_delete_semaphore(acpi_handle handle)
 acpi_status acpi_os_wait_semaphore(acpi_handle handle, u32 units, u16 timeout)
 {
 	acpi_status status = AE_OK;
-	struct semaphore *sem = (struct semaphore *)handle;
+	struct completion *comp = (struct completion *)handle;
 	long jiffies;
 	int ret = 0;
 
-	if (!sem || (units < 1))
+	if (!comp || (units < 1))
 		return AE_BAD_PARAMETER;
 
 	if (units > 1)
 		return AE_SUPPORT;
 
-	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Waiting for semaphore[%p|%d|%d]\n",
+	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Waiting for completion[%p|%d|%d]\n",
 			  handle, units, timeout));
 
 	if (timeout == ACPI_WAIT_FOREVER)
@@ -832,18 +834,18 @@ acpi_status acpi_os_wait_semaphore(acpi_handle handle, u32 units, u16 timeout)
 	else
 		jiffies = msecs_to_jiffies(timeout);
 	
-	ret = down_timeout(sem, jiffies);
+	ret = wait_for_completion_timeout(comp, jiffies);
 	if (ret)
 		status = AE_TIME;
 
 	if (ACPI_FAILURE(status)) {
 		ACPI_DEBUG_PRINT((ACPI_DB_MUTEX,
-				  "Failed to acquire semaphore[%p|%d|%d], %s",
+				  "Failed to acquire completion[%p|%d|%d], %s",
 				  handle, units, timeout,
 				  acpi_format_exception(status)));
 	} else {
 		ACPI_DEBUG_PRINT((ACPI_DB_MUTEX,
-				  "Acquired semaphore[%p|%d|%d]", handle,
+				  "Acquired completion[%p|%d|%d]", handle,
 				  units, timeout));
 	}
 
@@ -855,18 +857,18 @@ acpi_status acpi_os_wait_semaphore(acpi_handle handle, u32 units, u16 timeout)
  */
 acpi_status acpi_os_signal_semaphore(acpi_handle handle, u32 units)
 {
-	struct semaphore *sem = (struct semaphore *)handle;
+	struct completion *comp = (struct completion *)handle;
 
-	if (!sem || (units < 1))
+	if (!comp || (units < 1))
 		return AE_BAD_PARAMETER;
 
 	if (units > 1)
 		return AE_SUPPORT;
 
-	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Signaling semaphore[%p|%d]\n", handle,
+	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Signaling completion[%p|%d]\n", handle,
 			  units));
 
-	up(sem);
+	complete(comp);
 
 	return AE_OK;
 }
-- 
1.5.5.1.32.gba7d2

Re: [PATCH 4/4] acpi: semaphore removal

[Andi Kleen]

Daniel Walker wrote:
> The semaphore usage in ACPI is more like completions. The ASL
> functions getting implemented here are signals which follow a
> "wait for", signaled, or reset format.
> 
> This implements the ACPI signaling methods with the Linux
> completion API, instead of using semaphores.

NACK. I don't see the point of emulating semaphores when
there are already perfectly fine semaphores available.

-Andi

Re: [PATCH 4/4] acpi: semaphore removal

[Daniel Walker]

On Sat, 2008-08-16 at 00:17 +0200, Andi Kleen wrote:
> Daniel Walker wrote:
> > The semaphore usage in ACPI is more like completions. The ASL
> > functions getting implemented here are signals which follow a
> > "wait for", signaled, or reset format.
> > 
> > This implements the ACPI signaling methods with the Linux
> > completion API, instead of using semaphores.
> 
> NACK. I don't see the point of emulating semaphores when
> there are already perfectly fine semaphores available.


With semaphores as a super set, under that you have mutexes and
completions, and other un-classified usages.. Mutex usage is obviously
for mutual exclusion (semaphore initialized to 1), completions are used
for signaling (semaphore initialized to 0).

The semaphores in ACPI are specifically used in the initialized to 0
configuration (i.e. signaling). There are a total of three
initialization locations of ACPI semaphores

1    186  drivers/acpi/executer/excreate.c <<acpi_ex_create_event>>
2    295  drivers/acpi/executer/exsystem.c <<acpi_ex_system_reset_event>>
3    216  drivers/acpi/namespace/nsaccess.c <<acpi_ns_root_initialize>>

ACPI doesn't use semaphores, it uses completions .. Although the name
semaphore remains, they are still completions.

It should really be the inverse , why is ACPI using semaphores when we
have perfectly good completions (specific tool for a specific job).. 

Daniel

Re: [PATCH 4/4] acpi: semaphore removal

[Andi Kleen]

Daniel Walker wrote:
> On Sat, 2008-08-16 at 00:17 +0200, Andi Kleen wrote:
>> Daniel Walker wrote:
>>> The semaphore usage in ACPI is more like completions. The ASL
>>> functions getting implemented here are signals which follow a
>>> "wait for", signaled, or reset format.
>>>
>>> This implements the ACPI signaling methods with the Linux
>>> completion API, instead of using semaphores.
>> NACK. I don't see the point of emulating semaphores when
>> there are already perfectly fine semaphores available.
> 
> 
> With semaphores as a super set, under that you have mutexes and
> completions, and other un-classified usages.. Mutex usage is obviously
> for mutual exclusion (semaphore initialized to 1), completions are used
> for signaling (semaphore initialized to 0).

Let's put it differently: I just don't see how your change improves
the code. Replacing one locking primitive with another has no value
in itself that I can see unless it fixes something.

And when there is the choice I think it's cleaner and less confusing to use
semaphores when the ACPICA asks for semaphores instead of something else.

-Andi

Re: [PATCH 4/4] acpi: semaphore removal

[Daniel Walker]

On Sat, 2008-08-16 at 04:06 +0200, Andi Kleen wrote:
> Daniel Walker wrote:
> > On Sat, 2008-08-16 at 00:17 +0200, Andi Kleen wrote:
> >> Daniel Walker wrote:
> >>> The semaphore usage in ACPI is more like completions. The ASL
> >>> functions getting implemented here are signals which follow a
> >>> "wait for", signaled, or reset format.
> >>>
> >>> This implements the ACPI signaling methods with the Linux
> >>> completion API, instead of using semaphores.
> >> NACK. I don't see the point of emulating semaphores when
> >> there are already perfectly fine semaphores available.
> > 
> > 
> > With semaphores as a super set, under that you have mutexes and
> > completions, and other un-classified usages.. Mutex usage is obviously
> > for mutual exclusion (semaphore initialized to 1), completions are used
> > for signaling (semaphore initialized to 0).
> 
> Let's put it differently: I just don't see how your change improves
> the code. Replacing one locking primitive with another has no value
> in itself that I can see unless it fixes something.
> 
> And when there is the choice I think it's cleaner and less confusing to use
> semaphores when the ACPICA asks for semaphores instead of something else.

Semaphore are very far from the right solution to any problem. They are
too broad, and they are _harder_ to understand.. They have so many
usages that who knows what ACPI is really doing unless you commit to a
specific sub-set of semaphores.

I think ACPI "asks" for semaphores because it's multi-OS, that's not an
excuse to keep using semaphores when they aren't needed. We have other
API's that will satisfy ACPI which are less broad, and ultimately
better.

Daniel

Re: [PATCH 2/4] acpi: add real mutex function calls

[Andi Kleen]

Daniel Walker wrote:
> Instead of re-using semaphores for the mutex operation, I've
> added usage of the kernel mutex for the acpi os mutex
> implementation.

This still implies that we'll get lockdep warnings if the AML
does something dodgy with mutexes, right?

I don't like that. We cannot generally fix the AML, the only
way would be to time out on a deadlock and let it handle that.
Also it's not that we don't get enough ACPI reports already.

Is there a way to just turn lockdep always off for these mutexes?

-Andi

Re: [PATCH 2/4] acpi: add real mutex function calls

[Daniel Walker]

On Sat, 2008-08-16 at 00:16 +0200, Andi Kleen wrote:
> Daniel Walker wrote:
> > Instead of re-using semaphores for the mutex operation, I've
> > added usage of the kernel mutex for the acpi os mutex
> > implementation.
> 
> This still implies that we'll get lockdep warnings if the AML
> does something dodgy with mutexes, right?

Right .. When I was first writing these changes, I got pretty freaked
out when I realized this .. However, I started reading about the Mutex
operations in the ACPI spec, "17.5.79 Mutex (Declare
Synchronization/Mutex Object)"

It says,

"To prevent deadlocks, wherever more than one synchronization object
must be owned, the synchronization
objects must always be released in the order opposite the order in which
they were acquired."

I think the ASL compiler should (does?) validate that. It also uses
something called a "synclevel" during mutex declaration which, I think,
is used to force a clear distinction between a mutex acquired at the
outer most level , and one which is nested X levels down.

Then the mutex release from "17.5.99 Release (Release a Mutex
Synchronization Object)" says when you release a mutex,

"If the mutex object is owned by the current invocation, ownership for
the Mutex is released once. It is fatal to release ownership on a Mutex
unless it is currently owned. A Mutex must be totally released before an
invocation completes."

With all that, and the fact that I don't see much of a reason to have
deep synchronization inside these AML's , I think it's going to be a
very rare occasion that we find a lockdep warning inside the AML ..Even
if we did find a problem, that's something we would want to know about
and/or try to get fixed I would think.

Daniel

Re: [PATCH 2/4] acpi: add real mutex function calls

[Andi Kleen]

> "To prevent deadlocks, wherever more than one synchronization object
> must be owned, the synchronization
> objects must always be released in the order opposite the order in which
> they were acquired."
> 
> I think the ASL compiler should (does?) validate that. 

Bob should comment, but before we could rely on that it would also
need to be ensured that _all_ AML compilers do that, including
the one from Microsoft.

> With all that, and the fact that I don't see much of a reason to have
> deep synchronization inside these AML's ,

I'm sure BIOS writers come up with things we never thought about.

> I think it's going to be a
> very rare occasion that we find a lockdep warning inside the AML ..Even
> if we did find a problem, that's something we would want to know about
> and/or try to get fixed I would think.

I don't see a clear path to fix those if they happen. And just
processing them would already tie up precious bughandling resources.
It borders towards Steinbach's rule.

Now one possibility would be to figure out how to disable lockdep
for them (perhaps optional with default to off).

But on the other hand your patch is non trivial amount of code.

Without the checking (which would seem more like a disadvantage in this case)
the only advantage I can see of the mutexes would be PI handling in RT
kernels, but that's clearly an out of tree usage right now (and also
since AML is not executed in normal operation it's unlikely
that PI for those is really critical for anyone)

Any other reasons right now we would want that in tree?

-Andi

Re: [PATCH 2/4] acpi: add real mutex function calls

[Daniel Walker]

On Sat, 2008-08-16 at 04:16 +0200, Andi Kleen wrote:
> > "To prevent deadlocks, wherever more than one synchronization object
> > must be owned, the synchronization
> > objects must always be released in the order opposite the order in which
> > they were acquired."
> > 
> > I think the ASL compiler should (does?) validate that. 
> 
> Bob should comment, but before we could rely on that it would also
> need to be ensured that _all_ AML compilers do that, including
> the one from Microsoft.

I think it would be great if they all did the _mandated_ checking, but
if they don't all do it, then it's not the end of the world.

> > With all that, and the fact that I don't see much of a reason to have
> > deep synchronization inside these AML's ,
> 
> I'm sure BIOS writers come up with things we never thought about.

True.

> > I think it's going to be a
> > very rare occasion that we find a lockdep warning inside the AML ..Even
> > if we did find a problem, that's something we would want to know about
> > and/or try to get fixed I would think.
> 
> I don't see a clear path to fix those if they happen. And just
> processing them would already tie up precious bughandling resources.
> It borders towards Steinbach's rule.

Your assuming this will be a huge problem, which it won't .. I would be
surprised if we even find one real problem.

> Now one possibility would be to figure out how to disable lockdep
> for them (perhaps optional with default to off).

It's possible .. You could also contact the authors , who should fix a
bona fide defect in their code. AML can also be de-compiled, and
modified, recompiled, then used in a modified state.

> But on the other hand your patch is non trivial amount of code.
> 
> Without the checking (which would seem more like a disadvantage in this case)
> the only advantage I can see of the mutexes would be PI handling in RT
> kernels, but that's clearly an out of tree usage right now (and also
> since AML is not executed in normal operation it's unlikely
> that PI for those is really critical for anyone)

Andi your saying you don't want to see the defects .. Maybe defects
exist, but you don't want to see them.. You don't know what's there, and
you don't want to know ?

We could simply revert the code if the defects are overwhelming..

> Any other reasons right now we would want that in tree?
> 

The checking is an advantage (contrary to your claim) .. I think it's
also a benefit to use the right API for what is needed, in fact ACPI is
not doing that. By using a mutex you are confined to a strict set of
rules which makes it much easier for outsiders to understand what the
code is doing. That also makes it easier to maintain.

Daniel

[PATCH 1/4] mutex: add mutex_lock_timeout()

[Daniel Walker]

Adds mutex_lock_timeout() (and mutex_lock_timeout_nested()) used inside ACPI.

Cc: linux-acpi@vger.kernel.org
Signed-off-by: Daniel Walker <dwalker@mvista.com>
---
 include/asm-generic/mutex-dec.h  |   23 ++++++++++++
 include/asm-generic/mutex-null.h |    3 ++
 include/asm-generic/mutex-xchg.h |   23 ++++++++++++
 include/asm-x86/mutex_32.h       |   21 +++++++++++
 include/asm-x86/mutex_64.h       |   21 +++++++++++
 include/linux/mutex.h            |    8 ++++
 kernel/mutex.c                   |   70 +++++++++++++++++++++++++++++++++-----
 7 files changed, 160 insertions(+), 9 deletions(-)

diff --git a/include/asm-generic/mutex-dec.h b/include/asm-generic/mutex-dec.h
index ed108be..eddc8c4 100644
--- a/include/asm-generic/mutex-dec.h
+++ b/include/asm-generic/mutex-dec.h
@@ -109,4 +109,27 @@ __mutex_fastpath_trylock(atomic_t *count, int (*fail_fn)(atomic_t *))
 #endif
 }
 
+/**
+ *  __mutex_fastpath_lock_timeout - try to take the lock by moving the count
+ *                                  from 1 to a 0 value
+ *  @count: pointer of type atomic_t
+ *  @fail_fn: function to call if the original value was not 1
+ *  	      This function will need to accept two arguments.
+ *  @timeout: Timeout value as second argument to fail_fn.
+ *
+ * Change the count from 1 to a value lower than 1, and call <fail_fn> if
+ * it wasn't 1 originally. This function returns 0 if the fastpath succeeds,
+ * or anything the slow path function returns.
+ */
+static inline int
+__mutex_fastpath_lock_timeout(atomic_t *count, long timeout,
+					int (*fail_fn)(atomic_t *, long))
+{
+	if (unlikely(atomic_dec_return(count) < 0))
+		return fail_fn(count, timeout);
+	else {
+		smp_mb();
+		return 0;
+	}
+}
 #endif
diff --git a/include/asm-generic/mutex-null.h b/include/asm-generic/mutex-null.h
index e1bbbc7..192a756 100644
--- a/include/asm-generic/mutex-null.h
+++ b/include/asm-generic/mutex-null.h
@@ -14,6 +14,9 @@
 #define __mutex_fastpath_lock_retval(count, fail_fn)	fail_fn(count)
 #define __mutex_fastpath_unlock(count, fail_fn)		fail_fn(count)
 #define __mutex_fastpath_trylock(count, fail_fn)	fail_fn(count)
+
+#define __mutex_fastpath_timeout(count, timeout, fail_fn) \
+	fail_fn(count, timeout)
 #define __mutex_slowpath_needs_to_unlock()		1
 
 #endif
diff --git a/include/asm-generic/mutex-xchg.h b/include/asm-generic/mutex-xchg.h
index 7b9cd2c..34ccdd3 100644
--- a/include/asm-generic/mutex-xchg.h
+++ b/include/asm-generic/mutex-xchg.h
@@ -115,4 +115,27 @@ __mutex_fastpath_trylock(atomic_t *count, int (*fail_fn)(atomic_t *))
 	return prev;
 }
 
+/**
+ *  __mutex_fastpath_lock_timeout - try to take the lock by moving the count
+ *                                  from 1 to a 0 value
+ *  @count: pointer of type atomic_t
+ *  @fail_fn: function to call if the original value was not 1
+ *	      This function will need to accept two arguments.
+ *  @timeout: Timeout value as second argument to fail_fn.
+ *
+ * Change the count from 1 to a value lower than 1, and call <fail_fn> if it
+ * wasn't 1 originally. This function returns 0 if the fastpath succeeds,
+ * or anything the slow path function returns
+ */
+static inline int
+__mutex_fastpath_lock_timeout(atomic_t *count, timeout,
+			      int (*fail_fn)(atomic_t *, long))
+{
+	if (unlikely(atomic_xchg(count, 0) != 1))
+		return fail_fn(count, timeout);
+	else {
+		smp_mb();
+		return 0;
+	}
+}
 #endif
diff --git a/include/asm-x86/mutex_32.h b/include/asm-x86/mutex_32.h
index 73e928e..7d4696d 100644
--- a/include/asm-x86/mutex_32.h
+++ b/include/asm-x86/mutex_32.h
@@ -122,4 +122,25 @@ static inline int __mutex_fastpath_trylock(atomic_t *count,
 #endif
 }
 
+/**
+ *  __mutex_fastpath_lock_timeout - try to take the lock by moving the count
+ *                                  from 1 to a 0 value
+ *  @count: pointer of type atomic_t
+ *  @fail_fn: function to call if the original value was not 1
+ *	      This function will need to accept two arguments.
+ *  @timeout: Timeout value as second argument to fail_fn.
+ *
+ * Change the count from 1 to a value lower than 1, and call <fail_fn> if it
+ * wasn't 1 originally. This function returns 0 if the fastpath succeeds,
+ * or anything the slow path function returns
+ */
+static inline int
+__mutex_fastpath_lock_timeout(atomic_t *count, long timeout,
+			      int (*fail_fn)(atomic_t *, long))
+{
+	if (unlikely(atomic_dec_return(count) < 0))
+		return fail_fn(count, timeout);
+	else
+		return 0;
+}
 #endif
diff --git a/include/asm-x86/mutex_64.h b/include/asm-x86/mutex_64.h
index f3fae9b..3e63b61 100644
--- a/include/asm-x86/mutex_64.h
+++ b/include/asm-x86/mutex_64.h
@@ -97,4 +97,25 @@ static inline int __mutex_fastpath_trylock(atomic_t *count,
 		return 0;
 }
 
+/**
+ *  __mutex_fastpath_lock_timeout - try to take the lock by moving the count
+ *                                  from 1 to a 0 value
+ *  @count: pointer of type atomic_t
+ *  @fail_fn: function to call if the original value was not 1
+ *	      This function will need to accept two arguments.
+ *  @timeout: Timeout value as second argument to fail_fn.
+ *
+ * Change the count from 1 to a value lower than 1, and call <fail_fn> if
+ * it wasn't 1 originally. This function returns 0 if the fastpath succeeds,
+ * or anything the slow path function returns
+ */
+static inline int
+__mutex_fastpath_lock_timeout(atomic_t *count, long timeout,
+			      int (*fail_fn)(atomic_t *, long))
+{
+	if (unlikely(atomic_dec_return(count) < 0))
+		return fail_fn(count, timeout);
+	else
+		return 0;
+}
 #endif
diff --git a/include/linux/mutex.h b/include/linux/mutex.h
index bc6da10..bb84cd4 100644
--- a/include/linux/mutex.h
+++ b/include/linux/mutex.h
@@ -127,18 +127,26 @@ extern int __must_check mutex_lock_interruptible_nested(struct mutex *lock,
 					unsigned int subclass);
 extern int __must_check mutex_lock_killable_nested(struct mutex *lock,
 					unsigned int subclass);
+extern int __must_check
+mutex_lock_timeout_nested(struct mutex *lock, long jiffies,
+			  unsigned int subclass);
 
 #define mutex_lock(lock) mutex_lock_nested(lock, 0)
 #define mutex_lock_interruptible(lock) mutex_lock_interruptible_nested(lock, 0)
 #define mutex_lock_killable(lock) mutex_lock_killable_nested(lock, 0)
+#define mutex_lock_timeout(lock, jiffies) \
+	mutex_lock_timeout_nested(lock, jiffies, 0)
 #else
 extern void mutex_lock(struct mutex *lock);
 extern int __must_check mutex_lock_interruptible(struct mutex *lock);
 extern int __must_check mutex_lock_killable(struct mutex *lock);
+extern int __must_check mutex_lock_timeout(struct mutex *lock, long jiffies);
 
 # define mutex_lock_nested(lock, subclass) mutex_lock(lock)
 # define mutex_lock_interruptible_nested(lock, subclass) mutex_lock_interruptible(lock)
 # define mutex_lock_killable_nested(lock, subclass) mutex_lock_killable(lock)
+# define mutex_lock_timeout_nested(lock, jiffies, subclass) \
+	mutex_lock_timeout(lock, jiffies)
 #endif
 
 /*
diff --git a/kernel/mutex.c b/kernel/mutex.c
index 12c779d..902be79 100644
--- a/kernel/mutex.c
+++ b/kernel/mutex.c
@@ -124,8 +124,8 @@ EXPORT_SYMBOL(mutex_unlock);
  * Lock a mutex (possibly interruptible), slowpath:
  */
 static inline int __sched
-__mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
-	       	unsigned long ip)
+__mutex_lock_common(struct mutex *lock, long state, long timeout,
+		    unsigned int subclass, unsigned long ip)
 {
 	struct task_struct *task = current;
 	struct mutex_waiter waiter;
@@ -179,7 +179,22 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
 
 		/* didnt get the lock, go to sleep: */
 		spin_unlock_mutex(&lock->wait_lock, flags);
-		schedule();
+		if (timeout == MAX_SCHEDULE_TIMEOUT)
+			schedule();
+		else {
+			timeout = schedule_timeout(timeout);
+
+			if (timeout == 0) {
+				spin_lock_mutex(&lock->wait_lock, flags);
+				mutex_remove_waiter(lock, &waiter,
+						    task_thread_info(task));
+				mutex_release(&lock->dep_map, 1, ip);
+				spin_unlock_mutex(&lock->wait_lock, flags);
+
+				debug_mutex_free_waiter(&waiter);
+				return -ETIME;
+			}
+		}
 		spin_lock_mutex(&lock->wait_lock, flags);
 	}
 
@@ -205,7 +220,8 @@ void __sched
 mutex_lock_nested(struct mutex *lock, unsigned int subclass)
 {
 	might_sleep();
-	__mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, subclass, _RET_IP_);
+	__mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT,
+			    subclass, _RET_IP_);
 }
 
 EXPORT_SYMBOL_GPL(mutex_lock_nested);
@@ -214,7 +230,8 @@ int __sched
 mutex_lock_killable_nested(struct mutex *lock, unsigned int subclass)
 {
 	might_sleep();
-	return __mutex_lock_common(lock, TASK_KILLABLE, subclass, _RET_IP_);
+	return __mutex_lock_common(lock, TASK_KILLABLE, MAX_SCHEDULE_TIMEOUT,
+				   subclass, _RET_IP_);
 }
 EXPORT_SYMBOL_GPL(mutex_lock_killable_nested);
 
@@ -222,10 +239,22 @@ int __sched
 mutex_lock_interruptible_nested(struct mutex *lock, unsigned int subclass)
 {
 	might_sleep();
-	return __mutex_lock_common(lock, TASK_INTERRUPTIBLE, subclass, _RET_IP_);
+	return __mutex_lock_common(lock, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT,
+				   subclass, _RET_IP_);
 }
 
 EXPORT_SYMBOL_GPL(mutex_lock_interruptible_nested);
+
+int __sched
+mutex_lock_timeout_nested(struct mutex *lock, long timeout,
+			  unsigned int subclass)
+{
+	might_sleep();
+	return __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, timeout,
+				   subclass, _RET_IP_);
+}
+EXPORT_SYMBOL_GPL(mutex_lock_timeout_nested);
+
 #endif
 
 /*
@@ -285,6 +314,9 @@ __mutex_lock_killable_slowpath(atomic_t *lock_count);
 static noinline int __sched
 __mutex_lock_interruptible_slowpath(atomic_t *lock_count);
 
+static noinline int __sched
+__mutex_lock_timeout_slowpath(atomic_t *lock_count, long timeout);
+
 /***
  * mutex_lock_interruptible - acquire the mutex, interruptable
  * @lock: the mutex to be acquired
@@ -313,12 +345,21 @@ int __sched mutex_lock_killable(struct mutex *lock)
 }
 EXPORT_SYMBOL(mutex_lock_killable);
 
+int __sched mutex_lock_timeout(struct mutex *lock, long timeout)
+{
+	might_sleep();
+	return __mutex_fastpath_lock_timeout
+			(&lock->count, timeout, __mutex_lock_timeout_slowpath);
+}
+EXPORT_SYMBOL(mutex_lock_timeout);
+
 static noinline void __sched
 __mutex_lock_slowpath(atomic_t *lock_count)
 {
 	struct mutex *lock = container_of(lock_count, struct mutex, count);
 
-	__mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, 0, _RET_IP_);
+	__mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT,
+			    0, _RET_IP_);
 }
 
 static noinline int __sched
@@ -326,7 +367,8 @@ __mutex_lock_killable_slowpath(atomic_t *lock_count)
 {
 	struct mutex *lock = container_of(lock_count, struct mutex, count);
 
-	return __mutex_lock_common(lock, TASK_KILLABLE, 0, _RET_IP_);
+	return __mutex_lock_common(lock, TASK_KILLABLE, MAX_SCHEDULE_TIMEOUT,
+				   0, _RET_IP_);
 }
 
 static noinline int __sched
@@ -334,7 +376,17 @@ __mutex_lock_interruptible_slowpath(atomic_t *lock_count)
 {
 	struct mutex *lock = container_of(lock_count, struct mutex, count);
 
-	return __mutex_lock_common(lock, TASK_INTERRUPTIBLE, 0, _RET_IP_);
+	return __mutex_lock_common(lock, TASK_INTERRUPTIBLE,
+				   MAX_SCHEDULE_TIMEOUT, 0, _RET_IP_);
+}
+
+static noinline int __sched
+__mutex_lock_timeout_slowpath(atomic_t *lock_count, long timeout)
+{
+	struct mutex *lock = container_of(lock_count, struct mutex, count);
+
+	return __mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, timeout,
+				   0, _RET_IP_);
 }
 #endif
 
-- 
1.5.5.1.32.gba7d2

[PATCH 2/4] acpi: add real mutex function calls

[Daniel Walker]

Instead of re-using semaphores for the mutex operation, I've
added usage of the kernel mutex for the acpi os mutex
implementation.

Cc: linux-acpi@vger.kernel.org
Signed-off-by: Daniel Walker <dwalker@mvista.com>
---
 drivers/acpi/osl.c      |   93 +++++++++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpiosxf.h |   11 +-----
 2 files changed, 95 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 235a138..e6f7337 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -871,6 +871,99 @@ acpi_status acpi_os_signal_semaphore(acpi_handle handle, u32 units)
 	return AE_OK;
 }
 
+acpi_status
+acpi_os_create_mutex(acpi_mutex *handle)
+{
+	struct mutex *mutex = NULL;
+
+	mutex = acpi_os_allocate(sizeof(struct mutex));
+	if (!mutex)
+		return AE_NO_MEMORY;
+	memset(mutex, 0, sizeof(struct mutex));
+
+	mutex_init(mutex);
+
+	*handle = (acpi_handle *) mutex;
+
+	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Creating mutex[%p].\n",
+			  *handle));
+
+	return AE_OK;
+}
+
+acpi_status acpi_os_delete_mutex(acpi_mutex handle)
+{
+	struct mutex *mutex = (struct mutex *)handle;
+
+	if (!mutex)
+		return AE_BAD_PARAMETER;
+
+	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Deleting mutex[%p].\n", handle));
+
+	BUG_ON(mutex_is_locked(mutex));
+	kfree(mutex);
+	mutex = NULL;
+
+	return AE_OK;
+}
+
+acpi_status acpi_os_acquire_mutex(acpi_mutex handle, u16 timeout)
+{
+	acpi_status status = AE_OK;
+	struct mutex *mutex = (struct mutex *)handle;
+	long jiffies;
+	int ret = 0;
+
+	if (!mutex)
+		return AE_BAD_PARAMETER;
+
+	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Waiting for mutex[%p|%d]\n",
+			  handle, timeout));
+
+	if (timeout == ACPI_DO_NOT_WAIT) {
+		if (mutex_trylock(mutex))
+			return status;
+		else
+			return -ETIME;
+	}
+
+	if (timeout == ACPI_WAIT_FOREVER)
+		jiffies = MAX_SCHEDULE_TIMEOUT;
+	else
+		jiffies = msecs_to_jiffies(timeout);
+
+	ret = mutex_lock_timeout(mutex, jiffies);
+	if (ret == -ETIME)
+		status = AE_TIME;
+
+	if (ACPI_FAILURE(status)) {
+		ACPI_DEBUG_PRINT((ACPI_DB_MUTEX,
+				  "Failed to acquire mutex[%p|%d], %s",
+				  handle, timeout,
+				  acpi_format_exception(status)));
+	} else {
+		ACPI_DEBUG_PRINT((ACPI_DB_MUTEX,
+				  "Acquired mutex[%p|%d]", handle,
+				  timeout));
+	}
+
+	return status;
+}
+
+acpi_status acpi_os_release_mutex(acpi_mutex handle)
+{
+	struct mutex *mutex = (struct mutex *)handle;
+
+	if (!mutex)
+		return AE_BAD_PARAMETER;
+
+	ACPI_DEBUG_PRINT((ACPI_DB_MUTEX, "Signaling mutex[%p]\n", handle));
+
+	mutex_unlock(mutex);
+
+	return AE_OK;
+}
+
 #ifdef ACPI_FUTURE_USAGE
 u32 acpi_os_get_line(char *buffer)
 {
diff --git a/include/acpi/acpiosxf.h b/include/acpi/acpiosxf.h
index 3f93a6b..9032ec3 100644
--- a/include/acpi/acpiosxf.h
+++ b/include/acpi/acpiosxf.h
@@ -125,18 +125,11 @@ acpi_status acpi_os_signal_semaphore(acpi_semaphore handle, u32 units);
  */
 acpi_status acpi_os_create_mutex(acpi_mutex * out_handle);
 
-void acpi_os_delete_mutex(acpi_mutex handle);
+acpi_status acpi_os_delete_mutex(acpi_mutex handle);
 
 acpi_status acpi_os_acquire_mutex(acpi_mutex handle, u16 timeout);
 
-void acpi_os_release_mutex(acpi_mutex handle);
-
-/* Temporary macros for Mutex* interfaces, map to existing semaphore xfaces */
-
-#define acpi_os_create_mutex(out_handle)    acpi_os_create_semaphore (1, 1, out_handle)
-#define acpi_os_delete_mutex(handle)        (void) acpi_os_delete_semaphore (handle)
-#define acpi_os_acquire_mutex(handle,time)  acpi_os_wait_semaphore (handle, 1, time)
-#define acpi_os_release_mutex(handle)       (void) acpi_os_signal_semaphore (handle, 1)
+acpi_status acpi_os_release_mutex(acpi_mutex handle);
 
 /*
  * Memory allocation and mapping
-- 
1.5.5.1.32.gba7d2