diff mbox series

[1/2] Revert "locking/mutex: clear MUTEX_FLAGS if wait_list is empty due to signal"

Message ID 20210728150230.150985-2-stefan.bader@canonical.com
State New
Headers show
Series Reverts we probably want to do quickly | expand

Commit Message

Stefan Bader July 28, 2021, 3:02 p.m. UTC 
This reverts commit 2e1eb7b6e1e0e4bd988b0bbcbd96e2fee205885b, it seems
to depend on some additional changes to locking and we have reports of
strange corruption happening in various cases. Not really something that
can be pinpointed but a chance that something related to locking is
causing this.

Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
---
 kernel/locking/mutex-debug.c |  4 ++--
 kernel/locking/mutex-debug.h |  2 +-
 kernel/locking/mutex.c       | 16 ++++------------
 kernel/locking/mutex.h       |  4 +++-
 4 files changed, 10 insertions(+), 16 deletions(-)

Comments

Krzysztof Kozlowski July 29, 2021, 7:30 a.m. UTC | #1 
On 28/07/2021 17:02, Stefan Bader wrote:
> This reverts commit 2e1eb7b6e1e0e4bd988b0bbcbd96e2fee205885b, it seems
> to depend on some additional changes to locking and we have reports of
> strange corruption happening in various cases. Not really something that
> can be pinpointed but a chance that something related to locking is
> causing this.
> 

The "sched" attribute removal in original upstream patch seems like a
non-related part of patch. Probably should be split to separate one.

Anyway, the upstream commit even though marked as fixing old bug, was
not backported to v4.14 and v4.4, so maybe we also shouldn't take it. :)


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>



Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/kernel/locking/mutex-debug.c b/kernel/locking/mutex-debug.c
index 839df4383799..9aa713629387 100644
--- a/kernel/locking/mutex-debug.c
+++ b/kernel/locking/mutex-debug.c
@@ -57,7 +57,7 @@  void debug_mutex_add_waiter(struct mutex *lock, struct mutex_waiter *waiter,
 	task->blocked_on = waiter;
 }
 
-void debug_mutex_remove_waiter(struct mutex *lock, struct mutex_waiter *waiter,
+void mutex_remove_waiter(struct mutex *lock, struct mutex_waiter *waiter,
 			 struct task_struct *task)
 {
 	DEBUG_LOCKS_WARN_ON(list_empty(&waiter->list));
@@ -65,7 +65,7 @@  void debug_mutex_remove_waiter(struct mutex *lock, struct mutex_waiter *waiter,
 	DEBUG_LOCKS_WARN_ON(task->blocked_on != waiter);
 	task->blocked_on = NULL;
 
-	INIT_LIST_HEAD(&waiter->list);
+	list_del_init(&waiter->list);
 	waiter->task = NULL;
 }
 
diff --git a/kernel/locking/mutex-debug.h b/kernel/locking/mutex-debug.h
index 53e631e1d76d..1edd3f45a4ec 100644
--- a/kernel/locking/mutex-debug.h
+++ b/kernel/locking/mutex-debug.h
@@ -22,7 +22,7 @@  extern void debug_mutex_free_waiter(struct mutex_waiter *waiter);
 extern void debug_mutex_add_waiter(struct mutex *lock,
 				   struct mutex_waiter *waiter,
 				   struct task_struct *task);
-extern void debug_mutex_remove_waiter(struct mutex *lock, struct mutex_waiter *waiter,
+extern void mutex_remove_waiter(struct mutex *lock, struct mutex_waiter *waiter,
 				struct task_struct *task);
 extern void debug_mutex_unlock(struct mutex *lock);
 extern void debug_mutex_init(struct mutex *lock, const char *name,
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index a1be54f68207..858a07590e39 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -172,16 +172,6 @@  static inline bool __mutex_waiter_is_first(struct mutex *lock, struct mutex_wait
 	return list_first_entry(&lock->wait_list, struct mutex_waiter, list) == waiter;
 }
 
-static void
-__mutex_remove_waiter(struct mutex *lock, struct mutex_waiter *waiter)
-{
-	list_del(&waiter->list);
-	if (likely(list_empty(&lock->wait_list)))
-		__mutex_clear_flag(lock, MUTEX_FLAGS);
-
-	debug_mutex_remove_waiter(lock, waiter, current);
-}
-
 /*
  * Give up ownership to a specific task, when @task = NULL, this is equivalent
  * to a regular unlock. Sets PICKUP on a handoff, clears HANDOF, preserves
@@ -868,7 +858,9 @@  __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
 acquired:
 	__set_current_state(TASK_RUNNING);
 
-	__mutex_remove_waiter(lock, &waiter);
+	mutex_remove_waiter(lock, &waiter, current);
+	if (likely(list_empty(&lock->wait_list)))
+		__mutex_clear_flag(lock, MUTEX_FLAGS);
 
 	debug_mutex_free_waiter(&waiter);
 
@@ -885,7 +877,7 @@  __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
 
 err:
 	__set_current_state(TASK_RUNNING);
-	__mutex_remove_waiter(lock, &waiter);
+	mutex_remove_waiter(lock, &waiter, current);
 err_early_backoff:
 	spin_unlock(&lock->wait_lock);
 	debug_mutex_free_waiter(&waiter);
diff --git a/kernel/locking/mutex.h b/kernel/locking/mutex.h
index f0c710b1d192..1c2287d3fa71 100644
--- a/kernel/locking/mutex.h
+++ b/kernel/locking/mutex.h
@@ -10,10 +10,12 @@ 
  * !CONFIG_DEBUG_MUTEXES case. Most of them are NOPs:
  */
 
+#define mutex_remove_waiter(lock, waiter, task) \
+		__list_del((waiter)->list.prev, (waiter)->list.next)
+
 #define debug_mutex_wake_waiter(lock, waiter)		do { } while (0)
 #define debug_mutex_free_waiter(waiter)			do { } while (0)
 #define debug_mutex_add_waiter(lock, waiter, ti)	do { } while (0)
-#define debug_mutex_remove_waiter(lock, waiter, ti)     do { } while (0)
 #define debug_mutex_unlock(lock)			do { } while (0)
 #define debug_mutex_init(lock, name, key)		do { } while (0)