linux-kernel - [PATCH 2/2] locking/rwsem: Use a return variable in rwsem_spin_on

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:	Wed,  8 Apr 2015 12:39:20 -0700
From:	Jason Low <jason.low2@...com>
To:	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Davidlohr Bueso <dave@...olabs.net>,
	Tim Chen <tim.c.chen@...ux.intel.com>,
	Aswin Chandramouleeswaran <aswin@...com>
Cc:	LKML <linux-kernel@...r.kernel.org>, Jason Low <jason.low2@...com>
Subject: [PATCH 2/2] locking/rwsem: Use a return variable in rwsem_spin_on_owner()

Ingo suggested for mutex_spin_on_owner() that having multiple return
statements is not the cleanest approach, especially when holding locks.

The same thing applies to the rwsem variant. This patch rewrites
much of this function to use a "ret" return value.

Signed-off-by: Jason Low <jason.low2@...com>
---
 kernel/locking/rwsem-xadd.c |   25 ++++++++++++-------------
 1 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c
index 3417d01..b1c9156 100644
--- a/kernel/locking/rwsem-xadd.c
+++ b/kernel/locking/rwsem-xadd.c
@@ -327,38 +327,37 @@ done:
 static noinline
 bool rwsem_spin_on_owner(struct rw_semaphore *sem, struct task_struct *owner)
 {
-	long count;
+	bool ret = true;
 
 	rcu_read_lock();
 	while (sem->owner == owner) {
 		/*
 		 * Ensure we emit the owner->on_cpu, dereference _after_
-		 * checking sem->owner still matches owner, if that fails,
-		 * owner might point to free()d memory, if it still matches,
+		 * checking sem->owner still matches owner. If that fails,
+		 * owner might point to freed memory. If it still matches,
 		 * the rcu_read_lock() ensures the memory stays valid.
 		 */
 		barrier();
 
-		/* abort spinning when need_resched or owner is not running */
+		/* Abort spinning when need_resched or owner is not running. */
 		if (!owner->on_cpu || need_resched()) {
-			rcu_read_unlock();
-			return false;
+			ret = false;
+			break;
 		}
 
 		cpu_relax_lowlatency();
 	}
 	rcu_read_unlock();
 
-	if (READ_ONCE(sem->owner))
-		return true; /* new owner, continue spinning */
-
 	/*
 	 * When the owner is not set, the lock could be free or
-	 * held by readers. Check the counter to verify the
-	 * state.
+	 * held by readers. Check the counter to verify the state.
 	 */
-	count = READ_ONCE(sem->count);
-	return (count == 0 || count == RWSEM_WAITING_BIAS);
+	if (!READ_ONCE(sem->owner)) {
+		long count = READ_ONCE(sem->count);
+		ret = (count == 0 || count == RWSEM_WAITING_BIAS);
+	}
+	return ret;
 }
 
 static bool rwsem_optimistic_spin(struct rw_semaphore *sem)
-- 
1.7.2.5

--
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/