* [PATCH v2] rwsem: Comments to explain the meaning of the rwsem's count field @ 2014-05-02 19:53 Tim Chen 2014-05-05 8:46 ` [tip:locking/core] rwsem: Add comments " tip-bot for Tim Chen 0 siblings, 1 reply; 12+ messages in thread From: Tim Chen @ 2014-05-02 19:53 UTC (permalink / raw) To: Ingo Molnar, Peter Zijlstra Cc: Andrew Morton, Davidlohr Bueso, Alex Shi, Andi Kleen, Michel Lespinasse, Rik van Riel, Peter Hurley, Thomas Gleixner, Paul E.McKenney, Jason Low, linux-kernel It takes me quite a while to understand how rwsem's count field mainifest itself in different scenarios. I'm adding comments to provide a quick reference on the the rwsem's count field for each scenario where readers and writers are contending for the lock. Hopefully it will be useful for future maintenance of the code and for people to get up to speed on how the logic in the code works. Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com> --- Changes from v1 (https://lkml.org/lkml/2014/5/1/327) - Account for the scenarios where readers/writers are attempting lock. - Grammatical corrections. - Corrected value for the second case of 0xffff0001 count. kernel/locking/rwsem-xadd.c | 49 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c index 1d66e08..b4219ff 100644 --- a/kernel/locking/rwsem-xadd.c +++ b/kernel/locking/rwsem-xadd.c @@ -12,6 +12,55 @@ #include <linux/export.h> /* + * Guide to the rw_semaphore's count field for common values. + * (32-bit case illustrated, similar for 64-bit) + * + * 0x0000000X (1) X readers active or attempting lock, no writer waiting + * X = #active_readers + #readers attempting to lock + * (X*ACTIVE_BIAS) + * + * 0x00000000 rwsem is unlocked, and no one is waiting for the lock or + * attempting to read lock or write lock. + * + * 0xffff000X (1) X readers active or attempting lock, with waiters for lock + * X = #active readers + # readers attempting lock + * (X*ACTIVE_BIAS + WAITING_BIAS) + * (2) 1 writer attempting lock, no waiters for lock + * X-1 = #active readers + #readers attempting lock + * ((X-1)*ACTIVE_BIAS + ACTIVE_WRITE_BIAS) + * (3) 1 writer active, no waiters for lock + * X-1 = #active readers + #readers attempting lock + * ((X-1)*ACTIVE_BIAS + ACTIVE_WRITE_BIAS) + * + * 0xffff0001 (1) 1 reader active or attempting lock, waiters for lock + * (WAITING_BIAS + ACTIVE_BIAS) + * (2) 1 writer active or attempting lock, no waiters for lock + * (ACTIVE_WRITE_BIAS) + * + * 0xffff0000 (1) There are writers or readers queued but none active + * or in the process of attempting lock. + * (WAITING_BIAS) + * Note: writer can attempt to steal lock for this count by adding + * ACTIVE_WRITE_BIAS in cmpxchg and checking the old count + * + * 0xfffe0001 (1) 1 writer active, or attempting lock. Waiters on queue. + * (ACTIVE_WRITE_BIAS + WAITING_BIAS) + * + * Note: Readers attempt to lock by adding ACTIVE_BIAS in down_read and checking + * the count becomes more than 0 for successful lock acquisition, + * i.e. the case where there are only readers or nobody has lock. + * (1st and 2nd case above). + * + * Writers attempt to lock by adding ACTIVE_WRITE_BIAS in down_write and + * checking the count becomes ACTIVE_WRITE_BIAS for successful lock + * acquisition (i.e. nobody else has lock or attempts lock). If + * unsuccessful, in rwsem_down_write_failed, we'll check to see if there + * are only waiters but none active (5th case above), and attempt to + * steal the lock. + * + */ + +/* * Initialize an rwsem: */ void __init_rwsem(struct rw_semaphore *sem, const char *name, -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [tip:locking/core] rwsem: Add comments to explain the meaning of the rwsem's count field 2014-05-02 19:53 [PATCH v2] rwsem: Comments to explain the meaning of the rwsem's count field Tim Chen @ 2014-05-05 8:46 ` tip-bot for Tim Chen 2014-05-05 16:03 ` Tim Chen 2014-05-14 14:27 ` Davidlohr Bueso 0 siblings, 2 replies; 12+ messages in thread From: tip-bot for Tim Chen @ 2014-05-05 8:46 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, hpa, mingo, torvalds, peterz, tim.c.chen, jason.low2, peter, paulmck, alex.shi, riel, akpm, tglx, davidlohr, walken Commit-ID: 3cf2f34e1a3d4d5ff209d087925cf950e52f4805 Gitweb: http://git.kernel.org/tip/3cf2f34e1a3d4d5ff209d087925cf950e52f4805 Author: Tim Chen <tim.c.chen@linux.intel.com> AuthorDate: Fri, 2 May 2014 12:53:57 -0700 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Sun, 4 May 2014 20:34:26 +0200 rwsem: Add comments to explain the meaning of the rwsem's count field It took me quite a while to understand how rwsem's count field mainifested itself in different scenarios. Add comments to provide a quick reference to the the rwsem's count field for each scenario where readers and writers are contending for the lock. Hopefully it will be useful for future maintenance of the code and for people to get up to speed on how the logic in the code works. Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com> Cc: Davidlohr Bueso <davidlohr@hp.com> Cc: Alex Shi <alex.shi@linaro.org> Cc: Michel Lespinasse <walken@google.com> Cc: Rik van Riel <riel@redhat.com> Cc: Peter Hurley <peter@hurleysoftware.com> Cc: Paul E.McKenney <paulmck@linux.vnet.ibm.com> Cc: Jason Low <jason.low2@hp.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com> Link: http://lkml.kernel.org/r/1399060437.2970.146.camel@schen9-DESK Signed-off-by: Ingo Molnar <mingo@kernel.org> --- kernel/locking/rwsem-xadd.c | 49 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c index 1d66e08..b4219ff 100644 --- a/kernel/locking/rwsem-xadd.c +++ b/kernel/locking/rwsem-xadd.c @@ -12,6 +12,55 @@ #include <linux/export.h> /* + * Guide to the rw_semaphore's count field for common values. + * (32-bit case illustrated, similar for 64-bit) + * + * 0x0000000X (1) X readers active or attempting lock, no writer waiting + * X = #active_readers + #readers attempting to lock + * (X*ACTIVE_BIAS) + * + * 0x00000000 rwsem is unlocked, and no one is waiting for the lock or + * attempting to read lock or write lock. + * + * 0xffff000X (1) X readers active or attempting lock, with waiters for lock + * X = #active readers + # readers attempting lock + * (X*ACTIVE_BIAS + WAITING_BIAS) + * (2) 1 writer attempting lock, no waiters for lock + * X-1 = #active readers + #readers attempting lock + * ((X-1)*ACTIVE_BIAS + ACTIVE_WRITE_BIAS) + * (3) 1 writer active, no waiters for lock + * X-1 = #active readers + #readers attempting lock + * ((X-1)*ACTIVE_BIAS + ACTIVE_WRITE_BIAS) + * + * 0xffff0001 (1) 1 reader active or attempting lock, waiters for lock + * (WAITING_BIAS + ACTIVE_BIAS) + * (2) 1 writer active or attempting lock, no waiters for lock + * (ACTIVE_WRITE_BIAS) + * + * 0xffff0000 (1) There are writers or readers queued but none active + * or in the process of attempting lock. + * (WAITING_BIAS) + * Note: writer can attempt to steal lock for this count by adding + * ACTIVE_WRITE_BIAS in cmpxchg and checking the old count + * + * 0xfffe0001 (1) 1 writer active, or attempting lock. Waiters on queue. + * (ACTIVE_WRITE_BIAS + WAITING_BIAS) + * + * Note: Readers attempt to lock by adding ACTIVE_BIAS in down_read and checking + * the count becomes more than 0 for successful lock acquisition, + * i.e. the case where there are only readers or nobody has lock. + * (1st and 2nd case above). + * + * Writers attempt to lock by adding ACTIVE_WRITE_BIAS in down_write and + * checking the count becomes ACTIVE_WRITE_BIAS for successful lock + * acquisition (i.e. nobody else has lock or attempts lock). If + * unsuccessful, in rwsem_down_write_failed, we'll check to see if there + * are only waiters but none active (5th case above), and attempt to + * steal the lock. + * + */ + +/* * Initialize an rwsem: */ void __init_rwsem(struct rw_semaphore *sem, const char *name, ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [tip:locking/core] rwsem: Add comments to explain the meaning of the rwsem's count field 2014-05-05 8:46 ` [tip:locking/core] rwsem: Add comments " tip-bot for Tim Chen @ 2014-05-05 16:03 ` Tim Chen 2014-05-05 16:31 ` Peter Zijlstra 2014-05-05 17:26 ` Ingo Molnar 2014-05-14 14:27 ` Davidlohr Bueso 1 sibling, 2 replies; 12+ messages in thread From: Tim Chen @ 2014-05-05 16:03 UTC (permalink / raw) To: mingo Cc: linux-tip-commits, linux-kernel, torvalds, peterz, peter, jason.low2, riel, alex.shi, paulmck, akpm, tglx, walken, davidlohr, H. Peter Anvin On Mon, 2014-05-05 at 01:46 -0700, tip-bot for Tim Chen wrote: > Commit-ID: 3cf2f34e1a3d4d5ff209d087925cf950e52f4805 > Gitweb: http://git.kernel.org/tip/3cf2f34e1a3d4d5ff209d087925cf950e52f4805 > Author: Tim Chen <tim.c.chen@linux.intel.com> > AuthorDate: Fri, 2 May 2014 12:53:57 -0700 > Committer: Ingo Molnar <mingo@kernel.org> > CommitDate: Sun, 4 May 2014 20:34:26 +0200 > Ingo, Can you pick up this version of the patch instead. I've updated the comments to reflect all cases for which the rwsem's count is less than WAITING_BIAS, as Peter has pointed out. Thanks. Tim --- >From edb145882b09e007b878eedd9d6683259c6ec506 Mon Sep 17 00:00:00 2001 Message-Id: <edb145882b09e007b878eedd9d6683259c6ec506.1399279707.git.tim.c.chen@linux.intel.com> From: Tim Chen <tim.c.chen@linux.intel.com> Date: Thu, 1 May 2014 03:13:09 -0700 Subject: [PATCH v3] rwsem: Comments to explain the meaning of the rwsem's count field To: Ingo Molnar <mingo@elte.hu>, Peter Zijlstra <peterz@infradead.org> Cc: Andrew Morton <akpm@linux-foundation.org>, Davidlohr Bueso <davidlohr@hp.com>, Alex Shi <alex.shi@linaro.org>, Andi Kleen <andi@firstfloor.org>, Michel Lespinasse <walken@google.com>, Rik van Riel <riel@redhat.com>, Peter Hurley <peter@hurleysoftware.com>, Thomas Gleixner <tglx@linutronix.de>, Paul E.McKenney <paulmck@linux.vnet.ibm.com>, Jason Low <jason.low2@hp.com>, linux-kernel@vger.kernel.org It takes me quite a while to understand how rwsem's count field mainifest itself in different scenarios. I'm adding comments to provide a quick reference on the the rwsem's count field for each scenario where readers and writers are contending for the lock. Hopefully it will be useful for future maintenance of the code and for people to get up to speed on how the logic in the code works. Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com> --- Changes from v2 (https://lkml.org/lkml/2014/5/2/514) - Account for all scenearios where count < WAITING_BIAS Changes from v1 (https://lkml.org/lkml/2014/5/1/327) - Account for the scenarios where readers/writers are attempting lock. - Grammatical corrections. - Corrected value for the second case of 0xffff0001 count. kernel/locking/rwsem-xadd.c | 60 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c index 1d66e08..a794aaa 100644 --- a/kernel/locking/rwsem-xadd.c +++ b/kernel/locking/rwsem-xadd.c @@ -12,6 +12,66 @@ #include <linux/export.h> /* + * Guide to the rw_semaphore's count field. + * (32-bit count illustrated in descending order, similar for 64-bit count) + * + * 0x0000000X (1) X readers active or attempting lock, no writer waiting + * where X = #active_readers + #readers attempting to lock + * count computed as (X*ACTIVE_BIAS) + * + * 0x00000000 (1) rwsem is unlocked, and no one is waiting for the lock or + * attempting to read lock or write lock. + * + * 0xffff000X (1) X readers active or attempting lock, with waiters for lock + * where X = #active readers + #readers attempting lock + * (X*ACTIVE_BIAS + WAITING_BIAS) + * (2) 1 writer attempting lock, no waiters for lock + * where X-1 = #active readers + #readers attempting lock + * ((X-1)*ACTIVE_BIAS + ACTIVE_WRITE_BIAS) + * (3) 1 writer active, no waiters for lock + * where X-1 = #active readers + #readers attempting lock + * ((X-1)*ACTIVE_BIAS + ACTIVE_WRITE_BIAS) + * + * 0xffff0001 (1) 1 reader active or attempting lock, waiters for lock + * (WAITING_BIAS + ACTIVE_BIAS) + * (2) 1 writer active or attempting lock, no waiters for lock + * (ACTIVE_WRITE_BIAS) + * + * 0xffff0000 (1) There are writers or readers queued but none active + * or in the process of attempting lock. + * (WAITING_BIAS) + * Note: writer can attempt to steal lock for this count by adding + * ACTIVE_WRITE_BIAS in cmpxchg and checking the old count + * + * count < WAITING_BIAS + * (1) X writer active, Y writer(s) attempting lock, + * Z readers attempting lock, no waiters + * where X = 0 or 1, (X+Y) >= 2, Z >= 0 + * ((X+Y) * ACTIVE_WRITE_BIAS + Z * ACTIVE_BIAS) + * (2) X writer active, Y writer(s) attempting lock, + * Z readers attempting lock, waiters for lock + * where X = 0 or 1, (X+Y) >= 1, Z >= 0 + * ((X+Y) * ACTIVE_WRITE_BIAS + Z * ACTIVE_BIAS + WAITING_BIAS) + * + * Note: Readers attempt to lock by adding ACTIVE_BIAS in down_read and checking + * the count becomes more than 0 for successful lock acquisition, + * i.e. the case where there are only readers locking or nobody has lock. + * (1st and 2nd case above). In rwsem_down_read failed, after + * putting itself on the wait queue, it will check again if there are + * only readers locking, nobody has lock or it is first in queue (1, 2, and + * 5th case), and call __rwsem_do_wake to wake up waiter at front + * of queue to attempt locking again. + * + * Writers attempt to lock by adding ACTIVE_WRITE_BIAS in down_write and + * checking the count becomes ACTIVE_WRITE_BIAS for successful lock + * acquisition (i.e. nobody else has lock or attempts lock). If + * unsuccessful, in rwsem_down_write_failed, we'll check to see if there + * are only waiters but none active (5th case), and attempt to + * steal the lock. + * + */ + +/* * Initialize an rwsem: */ void __init_rwsem(struct rw_semaphore *sem, const char *name, -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [tip:locking/core] rwsem: Add comments to explain the meaning of the rwsem's count field 2014-05-05 16:03 ` Tim Chen @ 2014-05-05 16:31 ` Peter Zijlstra 2014-05-05 16:59 ` Tim Chen 2014-05-05 17:26 ` Ingo Molnar 1 sibling, 1 reply; 12+ messages in thread From: Peter Zijlstra @ 2014-05-05 16:31 UTC (permalink / raw) To: Tim Chen Cc: mingo, linux-tip-commits, linux-kernel, torvalds, peter, jason.low2, riel, alex.shi, paulmck, akpm, tglx, walken, davidlohr, H. Peter Anvin On Mon, May 05, 2014 at 09:03:28AM -0700, Tim Chen wrote: > On Mon, 2014-05-05 at 01:46 -0700, tip-bot for Tim Chen wrote: > > Commit-ID: 3cf2f34e1a3d4d5ff209d087925cf950e52f4805 > > Gitweb: http://git.kernel.org/tip/3cf2f34e1a3d4d5ff209d087925cf950e52f4805 > > Author: Tim Chen <tim.c.chen@linux.intel.com> > > AuthorDate: Fri, 2 May 2014 12:53:57 -0700 > > Committer: Ingo Molnar <mingo@kernel.org> > > CommitDate: Sun, 4 May 2014 20:34:26 +0200 > > > > Ingo, > > Can you pick up this version of the patch instead. I've updated the > comments to reflect all cases for which the rwsem's count is less than > WAITING_BIAS, as Peter has pointed out. Wouldn't a state diagram be clearer than this gobs of text? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [tip:locking/core] rwsem: Add comments to explain the meaning of the rwsem's count field 2014-05-05 16:31 ` Peter Zijlstra @ 2014-05-05 16:59 ` Tim Chen 0 siblings, 0 replies; 12+ messages in thread From: Tim Chen @ 2014-05-05 16:59 UTC (permalink / raw) To: Peter Zijlstra Cc: mingo, linux-tip-commits, linux-kernel, torvalds, peter, jason.low2, riel, alex.shi, paulmck, akpm, tglx, walken, davidlohr, H. Peter Anvin On Mon, 2014-05-05 at 18:31 +0200, Peter Zijlstra wrote: > On Mon, May 05, 2014 at 09:03:28AM -0700, Tim Chen wrote: > > On Mon, 2014-05-05 at 01:46 -0700, tip-bot for Tim Chen wrote: > > > Commit-ID: 3cf2f34e1a3d4d5ff209d087925cf950e52f4805 > > > Gitweb: http://git.kernel.org/tip/3cf2f34e1a3d4d5ff209d087925cf950e52f4805 > > > Author: Tim Chen <tim.c.chen@linux.intel.com> > > > AuthorDate: Fri, 2 May 2014 12:53:57 -0700 > > > Committer: Ingo Molnar <mingo@kernel.org> > > > CommitDate: Sun, 4 May 2014 20:34:26 +0200 > > > > > > > Ingo, > > > > Can you pick up this version of the patch instead. I've updated the > > comments to reflect all cases for which the rwsem's count is less than > > WAITING_BIAS, as Peter has pointed out. > > Wouldn't a state diagram be clearer than this gobs of text? The state is represented by the count field with an explanation of what each state actually means. I started off with a pretty abbreviated version. But to account for the readers and writers that are in transient trying to acquire the lock, the comments get more verbose, unfortunately. Tim ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [tip:locking/core] rwsem: Add comments to explain the meaning of the rwsem's count field 2014-05-05 16:03 ` Tim Chen 2014-05-05 16:31 ` Peter Zijlstra @ 2014-05-05 17:26 ` Ingo Molnar 2014-05-05 18:21 ` Tim Chen 1 sibling, 1 reply; 12+ messages in thread From: Ingo Molnar @ 2014-05-05 17:26 UTC (permalink / raw) To: Tim Chen Cc: linux-tip-commits, linux-kernel, torvalds, peterz, peter, jason.low2, riel, alex.shi, paulmck, akpm, tglx, walken, davidlohr, H. Peter Anvin * Tim Chen <tim.c.chen@linux.intel.com> wrote: > On Mon, 2014-05-05 at 01:46 -0700, tip-bot for Tim Chen wrote: > > Commit-ID: 3cf2f34e1a3d4d5ff209d087925cf950e52f4805 > > Gitweb: http://git.kernel.org/tip/3cf2f34e1a3d4d5ff209d087925cf950e52f4805 > > Author: Tim Chen <tim.c.chen@linux.intel.com> > > AuthorDate: Fri, 2 May 2014 12:53:57 -0700 > > Committer: Ingo Molnar <mingo@kernel.org> > > CommitDate: Sun, 4 May 2014 20:34:26 +0200 > > > > Ingo, > > Can you pick up this version of the patch instead. I've updated the > comments to reflect all cases for which the rwsem's count is less > than WAITING_BIAS, as Peter has pointed out. Please send a delta patch against the one I applied - and also the state diagram suggestion with Peter, once it's clear what form it should take. I've yet to see a state diagram that was inferior to equivalent textual description - is this case an exception to that? Thanks, Ingo ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [tip:locking/core] rwsem: Add comments to explain the meaning of the rwsem's count field 2014-05-05 17:26 ` Ingo Molnar @ 2014-05-05 18:21 ` Tim Chen 2014-05-05 18:27 ` Ingo Molnar 0 siblings, 1 reply; 12+ messages in thread From: Tim Chen @ 2014-05-05 18:21 UTC (permalink / raw) To: Ingo Molnar Cc: linux-tip-commits, linux-kernel, torvalds, peterz, peter, jason.low2, riel, alex.shi, paulmck, akpm, tglx, walken, davidlohr, H. Peter Anvin On Mon, 2014-05-05 at 19:26 +0200, Ingo Molnar wrote: > * Tim Chen <tim.c.chen@linux.intel.com> wrote: > > > On Mon, 2014-05-05 at 01:46 -0700, tip-bot for Tim Chen wrote: > > > Commit-ID: 3cf2f34e1a3d4d5ff209d087925cf950e52f4805 > > > Gitweb: http://git.kernel.org/tip/3cf2f34e1a3d4d5ff209d087925cf950e52f4805 > > > Author: Tim Chen <tim.c.chen@linux.intel.com> > > > AuthorDate: Fri, 2 May 2014 12:53:57 -0700 > > > Committer: Ingo Molnar <mingo@kernel.org> > > > CommitDate: Sun, 4 May 2014 20:34:26 +0200 > > > > > > > Ingo, > > > > Can you pick up this version of the patch instead. I've updated the > > comments to reflect all cases for which the rwsem's count is less > > than WAITING_BIAS, as Peter has pointed out. > > Please send a delta patch against the one I applied - and also the > state diagram suggestion with Peter, once it's clear what form it > should take. I've yet to see a state diagram that was inferior to > equivalent textual description - is this case an exception to that? > Ingo, The delta patch is included below. Thinking a bit more, the state diagram approach is not necessarily less verbose because the state is a tuple (count, wait queue state). After enumerating the states, we may wind up with very similar to what I have. Thanks. Tim --- >From 490e647f5144a27e09cb987a5216de100de6c253 Mon Sep 17 00:00:00 2001 Message-Id: <490e647f5144a27e09cb987a5216de100de6c253.1399287355.git.tim.c.chen@linux.intel.com> From: Tim Chen <tim.c.chen@linux.intel.com> Date: Mon, 5 May 2014 03:53:08 -0700 Subject: [PATCH] rwsem: Update comments on rwsem count for count < WAITING_BIAS To: Ingo Molnar <mingo@elte.hu>, Peter Zijlstra <peterz@infradead.org> Cc: Andrew Morton <akpm@linux-foundation.org>, Davidlohr Bueso <davidlohr@hp.com>, Alex Shi <alex.shi@linaro.org>, Andi Kleen <andi@firstfloor.org>, Michel Lespinasse <walken@google.com>, Rik van Riel <riel@redhat.com>, Peter Hurley <peter@hurleysoftware.com>, Thomas Gleixner <tglx@linutronix.de>, Paul E.McKenney <paulmck@linux.vnet.ibm.com>, Jason Low <jason.low2@hp.com>, linux-kernel@vger.kernel.org Update the comments for rwsem count for the case where count < WAITING_BIAS. Also some clean up of comments and added explanation on how the rwsem_down_read_failed path uses the count field. Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com> --- kernel/locking/rwsem-xadd.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c index b4219ff..a794aaa 100644 --- a/kernel/locking/rwsem-xadd.c +++ b/kernel/locking/rwsem-xadd.c @@ -12,24 +12,24 @@ #include <linux/export.h> /* - * Guide to the rw_semaphore's count field for common values. - * (32-bit case illustrated, similar for 64-bit) + * Guide to the rw_semaphore's count field. + * (32-bit count illustrated in descending order, similar for 64-bit count) * * 0x0000000X (1) X readers active or attempting lock, no writer waiting - * X = #active_readers + #readers attempting to lock - * (X*ACTIVE_BIAS) + * where X = #active_readers + #readers attempting to lock + * count computed as (X*ACTIVE_BIAS) * - * 0x00000000 rwsem is unlocked, and no one is waiting for the lock or - * attempting to read lock or write lock. + * 0x00000000 (1) rwsem is unlocked, and no one is waiting for the lock or + * attempting to read lock or write lock. * * 0xffff000X (1) X readers active or attempting lock, with waiters for lock - * X = #active readers + # readers attempting lock + * where X = #active readers + #readers attempting lock * (X*ACTIVE_BIAS + WAITING_BIAS) * (2) 1 writer attempting lock, no waiters for lock - * X-1 = #active readers + #readers attempting lock + * where X-1 = #active readers + #readers attempting lock * ((X-1)*ACTIVE_BIAS + ACTIVE_WRITE_BIAS) * (3) 1 writer active, no waiters for lock - * X-1 = #active readers + #readers attempting lock + * where X-1 = #active readers + #readers attempting lock * ((X-1)*ACTIVE_BIAS + ACTIVE_WRITE_BIAS) * * 0xffff0001 (1) 1 reader active or attempting lock, waiters for lock @@ -43,19 +43,30 @@ * Note: writer can attempt to steal lock for this count by adding * ACTIVE_WRITE_BIAS in cmpxchg and checking the old count * - * 0xfffe0001 (1) 1 writer active, or attempting lock. Waiters on queue. - * (ACTIVE_WRITE_BIAS + WAITING_BIAS) + * count < WAITING_BIAS + * (1) X writer active, Y writer(s) attempting lock, + * Z readers attempting lock, no waiters + * where X = 0 or 1, (X+Y) >= 2, Z >= 0 + * ((X+Y) * ACTIVE_WRITE_BIAS + Z * ACTIVE_BIAS) + * (2) X writer active, Y writer(s) attempting lock, + * Z readers attempting lock, waiters for lock + * where X = 0 or 1, (X+Y) >= 1, Z >= 0 + * ((X+Y) * ACTIVE_WRITE_BIAS + Z * ACTIVE_BIAS + WAITING_BIAS) * * Note: Readers attempt to lock by adding ACTIVE_BIAS in down_read and checking * the count becomes more than 0 for successful lock acquisition, - * i.e. the case where there are only readers or nobody has lock. - * (1st and 2nd case above). + * i.e. the case where there are only readers locking or nobody has lock. + * (1st and 2nd case above). In rwsem_down_read failed, after + * putting itself on the wait queue, it will check again if there are + * only readers locking, nobody has lock or it is first in queue (1, 2, and + * 5th case), and call __rwsem_do_wake to wake up waiter at front + * of queue to attempt locking again. * * Writers attempt to lock by adding ACTIVE_WRITE_BIAS in down_write and * checking the count becomes ACTIVE_WRITE_BIAS for successful lock * acquisition (i.e. nobody else has lock or attempts lock). If * unsuccessful, in rwsem_down_write_failed, we'll check to see if there - * are only waiters but none active (5th case above), and attempt to + * are only waiters but none active (5th case), and attempt to * steal the lock. * */ -- 1.7.11.7 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [tip:locking/core] rwsem: Add comments to explain the meaning of the rwsem's count field 2014-05-05 18:21 ` Tim Chen @ 2014-05-05 18:27 ` Ingo Molnar 2014-05-05 22:51 ` Tim Chen 0 siblings, 1 reply; 12+ messages in thread From: Ingo Molnar @ 2014-05-05 18:27 UTC (permalink / raw) To: Tim Chen Cc: linux-tip-commits, linux-kernel, torvalds, peterz, peter, jason.low2, riel, alex.shi, paulmck, akpm, tglx, walken, davidlohr, H. Peter Anvin * Tim Chen <tim.c.chen@linux.intel.com> wrote: > On Mon, 2014-05-05 at 19:26 +0200, Ingo Molnar wrote: > > * Tim Chen <tim.c.chen@linux.intel.com> wrote: > > > > > On Mon, 2014-05-05 at 01:46 -0700, tip-bot for Tim Chen wrote: > > > > Commit-ID: 3cf2f34e1a3d4d5ff209d087925cf950e52f4805 > > > > Gitweb: http://git.kernel.org/tip/3cf2f34e1a3d4d5ff209d087925cf950e52f4805 > > > > Author: Tim Chen <tim.c.chen@linux.intel.com> > > > > AuthorDate: Fri, 2 May 2014 12:53:57 -0700 > > > > Committer: Ingo Molnar <mingo@kernel.org> > > > > CommitDate: Sun, 4 May 2014 20:34:26 +0200 > > > > > > > > > > Ingo, > > > > > > Can you pick up this version of the patch instead. I've updated the > > > comments to reflect all cases for which the rwsem's count is less > > > than WAITING_BIAS, as Peter has pointed out. > > > > Please send a delta patch against the one I applied - and also the > > state diagram suggestion with Peter, once it's clear what form it > > should take. I've yet to see a state diagram that was inferior to > > equivalent textual description - is this case an exception to that? > > > > Ingo, > > The delta patch is included below. Thinking a bit more, > the state diagram approach is not necessarily less verbose > because the state is a tuple (count, wait queue state). > After enumerating the states, we may wind up with very similar > to what I have. Could we at least try with one diagram and see how it goes? Thanks, Ingo ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [tip:locking/core] rwsem: Add comments to explain the meaning of the rwsem's count field 2014-05-05 18:27 ` Ingo Molnar @ 2014-05-05 22:51 ` Tim Chen 2014-05-06 2:30 ` Davidlohr Bueso 0 siblings, 1 reply; 12+ messages in thread From: Tim Chen @ 2014-05-05 22:51 UTC (permalink / raw) To: Ingo Molnar Cc: linux-tip-commits, linux-kernel, torvalds, peterz, peter, jason.low2, riel, alex.shi, paulmck, akpm, tglx, walken, davidlohr, H. Peter Anvin On Mon, 2014-05-05 at 20:27 +0200, Ingo Molnar wrote: > > Ingo, > > > > The delta patch is included below. Thinking a bit more, > > the state diagram approach is not necessarily less verbose > > because the state is a tuple (count, wait queue state). > > After enumerating the states, we may wind up with very similar > > to what I have. > > Could we at least try with one diagram and see how it goes? > I've tried (see below). But I don't like how it came out :( Tim --- Events: (1) Attempt read lock (+ACTIVE_BIAS) (2) Attempt write lock (+ACTIVE_WRITE_BIAS) (3) Abort read lock or read unlock (-ACTIVE_BIAS) (4) Abort write lock or write unlock (-ACTIVE_WRITE_BIAS) (5) Put reader/writer on queue after read/write lock attempt has been aborted (+WAITING_BIAS if queue empty) (6) Pull reader/writer from head of queue (-WAITING_BIAS if queue becomes empty) State Event Next-State ----- ----- ---------- (A) count > 0 (0x0000000X), queue empty 1 => (A) 2 => (C.0) 3 => (A if count > 1, B if count =1) 4 not applicable 5 => (C.0) 6 not applicable (B) count = 0 (0x00000000), queue empty 1 => (A) 2 => (C.0) 3, 4 not applicable 5 => (E.0) 6 not applicable (C.0) 0 > count > WAITING_BIAS (0xffff000X), queue empty 1 => (C.0) 2 => (E.0) 3 => (count=0xffff0001 implies 1 writer, no readers to abort, C.0 if count > 0xffff0001) 4 => (B if count=0xffff0001, A if count > 0xffff0001) 5 => (E.1) 6 not applicable (C.1) 0 > count > WAITING_BIAS (0xffff000X), queue non-empty 1 => (C.1) 2 => (E.1) 3 => (D if count=0xffff0001, C.1 if count > 0xffff0001) 4 => (B if count=0xffff0001, A if count > 0xffff0001) 5 => (E.1) 6 => (A or B if queue becomes empty, C.1 if queue remains non-empty) (D) count = WAITING_BIAS (0xffff0000), queue non-empty 1 => (C.0) 2 => (E.0) 3,4 not applicable 5 => (D) 6 => (A if queue becomes empty, D otherwise) (E.0) count < WAITING_BIAS, queue empty 1 => (E.0) 2 => (E.0) 3 => (E.0) 4 => (C.0 if count > 2*WAITING_BIAS E.0 otherwise) 5 => (E.1) 6 not applicable (E.1) count < WAITING_BIAS, queue non-empty 1 => (E.1) 2 => (E.1) 3 => (E.1) 4 => (C.1 if count < 2*WAITING_BIAS, E.1 otherwise) 5 => (E.1) 6 => (D if count = 2*WAITING_BIAS, E.1 otherwise, queue remains non-empty) or E.0 otherwise ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [tip:locking/core] rwsem: Add comments to explain the meaning of the rwsem's count field 2014-05-05 22:51 ` Tim Chen @ 2014-05-06 2:30 ` Davidlohr Bueso 0 siblings, 0 replies; 12+ messages in thread From: Davidlohr Bueso @ 2014-05-06 2:30 UTC (permalink / raw) To: Tim Chen Cc: Ingo Molnar, linux-tip-commits, linux-kernel, torvalds, peterz, peter, jason.low2, riel, alex.shi, paulmck, akpm, tglx, walken, H. Peter Anvin On Mon, 2014-05-05 at 15:51 -0700, Tim Chen wrote: > On Mon, 2014-05-05 at 20:27 +0200, Ingo Molnar wrote: > > > > Ingo, > > > > > > The delta patch is included below. Thinking a bit more, > > > the state diagram approach is not necessarily less verbose > > > because the state is a tuple (count, wait queue state). > > > After enumerating the states, we may wind up with very similar > > > to what I have. > > > > Could we at least try with one diagram and see how it goes? > > > > I've tried (see below). But I don't like how it came out :( And quite nice, thanks for doing this. Personally, however, I much prefer the already applied patch to this approach. Thanks, Davidlohr ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [tip:locking/core] rwsem: Add comments to explain the meaning of the rwsem's count field 2014-05-05 8:46 ` [tip:locking/core] rwsem: Add comments " tip-bot for Tim Chen 2014-05-05 16:03 ` Tim Chen @ 2014-05-14 14:27 ` Davidlohr Bueso 2014-05-14 14:53 ` Peter Zijlstra 1 sibling, 1 reply; 12+ messages in thread From: Davidlohr Bueso @ 2014-05-14 14:27 UTC (permalink / raw) To: mingo, hpa, linux-kernel, torvalds, peterz, tim.c.chen, peter, jason.low2, riel, alex.shi, paulmck, akpm, tglx, walken Cc: linux-tip-commits On Mon, 2014-05-05 at 01:46 -0700, tip-bot for Tim Chen wrote: > Commit-ID: 3cf2f34e1a3d4d5ff209d087925cf950e52f4805 > Gitweb: http://git.kernel.org/tip/3cf2f34e1a3d4d5ff209d087925cf950e52f4805 > Author: Tim Chen <tim.c.chen@linux.intel.com> > AuthorDate: Fri, 2 May 2014 12:53:57 -0700 > Committer: Ingo Molnar <mingo@kernel.org> > CommitDate: Sun, 4 May 2014 20:34:26 +0200 > > rwsem: Add comments to explain the meaning of the rwsem's count field > > It took me quite a while to understand how rwsem's count field > mainifested itself in different scenarios. > > Add comments to provide a quick reference to the the rwsem's count > field for each scenario where readers and writers are contending > for the lock. > > Hopefully it will be useful for future maintenance of the code and > for people to get up to speed on how the logic in the code works. sigh, after this commit, the opt spin patch no longer applies, I'm rebasing and send out v5. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [tip:locking/core] rwsem: Add comments to explain the meaning of the rwsem's count field 2014-05-14 14:27 ` Davidlohr Bueso @ 2014-05-14 14:53 ` Peter Zijlstra 0 siblings, 0 replies; 12+ messages in thread From: Peter Zijlstra @ 2014-05-14 14:53 UTC (permalink / raw) To: Davidlohr Bueso Cc: mingo, hpa, linux-kernel, torvalds, tim.c.chen, peter, jason.low2, riel, alex.shi, paulmck, akpm, tglx, walken, linux-tip-commits [-- Attachment #1: Type: text/plain, Size: 1128 bytes --] On Wed, May 14, 2014 at 07:27:04AM -0700, Davidlohr Bueso wrote: > On Mon, 2014-05-05 at 01:46 -0700, tip-bot for Tim Chen wrote: > > Commit-ID: 3cf2f34e1a3d4d5ff209d087925cf950e52f4805 > > Gitweb: http://git.kernel.org/tip/3cf2f34e1a3d4d5ff209d087925cf950e52f4805 > > Author: Tim Chen <tim.c.chen@linux.intel.com> > > AuthorDate: Fri, 2 May 2014 12:53:57 -0700 > > Committer: Ingo Molnar <mingo@kernel.org> > > CommitDate: Sun, 4 May 2014 20:34:26 +0200 > > > > rwsem: Add comments to explain the meaning of the rwsem's count field > > > > It took me quite a while to understand how rwsem's count field > > mainifested itself in different scenarios. > > > > Add comments to provide a quick reference to the the rwsem's count > > field for each scenario where readers and writers are contending > > for the lock. > > > > Hopefully it will be useful for future maintenance of the code and > > for people to get up to speed on how the logic in the code works. > > sigh, after this commit, the opt spin patch no longer applies, I'm > rebasing and send out v5. No worries, fixed it already. [-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-05-14 14:53 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-05-02 19:53 [PATCH v2] rwsem: Comments to explain the meaning of the rwsem's count field Tim Chen 2014-05-05 8:46 ` [tip:locking/core] rwsem: Add comments " tip-bot for Tim Chen 2014-05-05 16:03 ` Tim Chen 2014-05-05 16:31 ` Peter Zijlstra 2014-05-05 16:59 ` Tim Chen 2014-05-05 17:26 ` Ingo Molnar 2014-05-05 18:21 ` Tim Chen 2014-05-05 18:27 ` Ingo Molnar 2014-05-05 22:51 ` Tim Chen 2014-05-06 2:30 ` Davidlohr Bueso 2014-05-14 14:27 ` Davidlohr Bueso 2014-05-14 14:53 ` Peter Zijlstra
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.