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:	Tue, 27 May 2014 10:33:24 -0700
From:	Jason Low <jason.low2@...com>
To:	Waiman Long <waiman.long@...com>
Cc:	Davidlohr Bueso <davidlohr@...com>, mingo@...nel.org,
	peterz@...radead.org, tglx@...utronix.de,
	akpm@...ux-foundation.org, tim.c.chen@...ux.intel.com,
	paulmck@...ux.vnet.ibm.com, hpa@...or.com, aswin@...com,
	linux-kernel@...r.kernel.org, jason.low2@...com
Subject: Re: [PATCH v3] mutex: Documentation rewrite

On Tue, 2014-05-27 at 12:03 -0400, Waiman Long wrote:
> On 05/23/2014 11:16 AM, Davidlohr Bueso wrote:
> > From: Davidlohr Bueso<davidlohr@...com>
> >
> >
> > +
> > +(ii) midpath: aka optimistic spinning, tries to spin for acquisition
> > +     when there are no pending waiters and the lock owner is currently
> 
> That is not true. The spinning code doesn't check if there is any 
> pending waiters. So some tasks can be spinning while the others are 
> sleeping. The only checks are: 1) The lock owner is running; 2) there is 
> no other higher-priority ready-to-run task (need_resched).

Ah yes, I have noticed a similar comment in the mutex.c file and I have
been holding on to a patch which corrects the comments in that file. 

Below was the patch for your reference:

-----
Subject: [PATCH 2/3] mutex: Correct documention on mutex optimistic spinning

The mutex optimistic spinning documentation states that we spin for
acquisition when we find that there are no pending waiters. However,
in actuality, whether or not there are waiters for the mutex doesn't
determine if we will spin for it.

This patch removes that statement and also adds a comment which
mentions that we spin for the mutex while we don't need to reschedule.

Signed-off-by: Jason Low <jason.low2@...com>
---
 kernel/locking/mutex.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 0925968..fc55f72 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -389,12 +389,10 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
 	/*
 	 * Optimistic spinning.
 	 *
-	 * We try to spin for acquisition when we find that there are no
-	 * pending waiters and the lock owner is currently running on a
-	 * (different) CPU.
-	 *
-	 * The rationale is that if the lock owner is running, it is likely to
-	 * release the lock soon.
+	 * We try to spin for acquisition when we find that the lock owner
+	 * is currently running on a (different) CPU and while we don't
+	 * need to reschedule. The rationale is that if the lock owner is
+	 * running, it is likely to release the lock soon.
 	 *
 	 * Since this needs the lock owner, and this mutex implementation
 	 * doesn't track the owner atomically in the lock field, we need to
-- 
1.7.1



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