linux-kernel - [PATCH] mutex: Remove ww_ctx unlikely() from __mutex_lock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 19 Jan 2017 08:54:41 -0500
From:   "Steven Rostedt (VMware)" <rostedt@...dmis.org>
To:     LKML <linux-kernel@...r.kernel.org>
Cc:     Chris Wilson <chris@...is-wilson.co.uk>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: [PATCH] mutex: Remove ww_ctx unlikely() from __mutex_lock_common()

The unikely() used in __mutex_lock_common() when use_ww_ctx is set is
currently dependent on the hardware if it is likely or unlikely. The
intel drm code calls into this function and triggers this branch 100%
of the time. As this hardware is very commonly used, this is not a rare
case at all (the three boxes I tested this on, all triggered it).

The likely/unlikely annotation should be used for logical cases that
cause it to mostly be hit or not, to let gcc optimize for a certain
case. If hardware causes it to be the opposite, then the hint is
punishing some hardware over other hardware, and no hint should be
placed at all.

I added a comment stating that the branch should be unlikely, but due
to the intel drm logic, it currently isn't. Then if drm changes in the
future, we could then try it again.

Signed-off-by: Steven Rostedt (VMware) <rostedt@...dmis.org>
---
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index a70b90d..577bb74 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -513,7 +513,11 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,

 	if (use_ww_ctx) {
 		struct ww_mutex *ww = container_of(lock, struct ww_mutex, base);
-		if (unlikely(ww_ctx == READ_ONCE(ww->ctx)))
+		/*
+		 * This really should be an unlikely() but currently
+		 * the intel drm makes this a very likely case.
+		 */
+		if (ww_ctx == READ_ONCE(ww->ctx))
 			return -EALREADY;
 	}