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-next>] [day] [month] [year] [list]
Date:	Wed, 8 Jun 2011 20:49:53 +0800
From:	Hillf Danton <dhillf@...il.com>
To:	LKML <linux-kernel@...r.kernel.org>
Cc:	Ingo Molnar <mingo@...e.hu>, Peter Zijlstra <peterz@...radead.org>
Subject: [PATCH] sched: remove unreliable pointer in mutex_spin_on_owner()

The dereference of unreliable owner pointer is unnecessary in owner_running(),
though under RCU protection, because the true result is only determined by
checking the validity of lock owner, as the comment says, due to likely heavy
lock contention, which has little to do with whether owner->on_cpu is false.

If owner->on_cpu is really false, only the owner_running loop is shortened,
but also returns incorrect result, since the lock owner is not changed, though
maybe changed soon.

Signed-off-by: Hillf Danton <dhillf@...il.com>
---
 kernel/sched.c |   36 +-----------------------------------
 1 files changed, 1 insertions(+), 35 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index fd18f39..2c32616 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -4293,52 +4293,18 @@ EXPORT_SYMBOL(schedule);

 #ifdef CONFIG_MUTEX_SPIN_ON_OWNER

-static inline bool owner_running(struct mutex *lock, struct task_struct *owner)
-{
-	bool ret = false;
-
-	rcu_read_lock();
-	if (lock->owner != owner)
-		goto fail;
-
-	/*
-	 * Ensure we emit the owner->on_cpu, dereference _after_ checking
-	 * lock->owner still matches owner, if that fails, owner might
-	 * point to free()d memory, if it still matches, the rcu_read_lock()
-	 * ensures the memory stays valid.
-	 */
-	barrier();
-
-	ret = owner->on_cpu;
-fail:
-	rcu_read_unlock();
-
-	return ret;
-}
-
-/*
- * Look out! "owner" is an entirely speculative pointer
- * access and not reliable.
- */
 int mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner)
 {
 	if (!sched_feat(OWNER_SPIN))
 		return 0;

-	while (owner_running(lock, owner)) {
+	while (lock->owner != NULL) {
 		if (need_resched())
 			return 0;

 		arch_mutex_cpu_relax();
 	}

-	/*
-	 * If the owner changed to another task there is likely
-	 * heavy contention, stop spinning.
-	 */
-	if (lock->owner)
-		return 0;
-
 	return 1;
 }
 #endif
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ