From 2f624278626677bfaf73fef97f86b37981621f5c Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Fri, 4 May 2012 14:46:02 -0700 Subject: Fix __read_seqcount_begin() to use ACCESS_ONCE for sequence value read We really need to use a ACCESS_ONCE() on the sequence value read in __read_seqcount_begin(), because otherwise the compiler might end up reloading the value in between the test and the return of it. As a result, it might end up returning an odd value (which means that a write is in progress). If the reader is then fast enough that that odd value is still the current one when the read_seqcount_retry() is done, we might end up with a "successful" read sequence, even despite the concurrent write being active. In practice this probably never really happens - there just isn't anything else going on around the read of the sequence count, and the common case is that we end up having a read barrier immediately afterwards. So the code sequence in which gcc might decide to reaload from memory is small, and there's no reason to believe it would ever actually do the reload. But if the compiler ever were to decide to do so, it would be incredibly annoying to debug. Let's just make sure. Cc: stable@kernel.org Signed-off-by: Linus Torvalds --- include/linux/seqlock.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/linux/seqlock.h') diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h index c6db9fb33c4..bb1fac5b8ee 100644 --- a/include/linux/seqlock.h +++ b/include/linux/seqlock.h @@ -141,7 +141,7 @@ static inline unsigned __read_seqcount_begin(const seqcount_t *s) unsigned ret; repeat: - ret = s->sequence; + ret = ACCESS_ONCE(s->sequence); if (unlikely(ret & 1)) { cpu_relax(); goto repeat; -- cgit v1.2.3-70-g09d2 From 4f988f152ee087831ea5c1c77cda4454cacc052c Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Fri, 4 May 2012 15:13:54 -0700 Subject: seqlock: add 'raw_seqcount_begin()' function The normal read_seqcount_begin() function will wait for any current writers to exit their critical region by looping until the sequence count is even. That "wait for sequence count to stabilize" is the right thing to do if the read-locker will just retry the whole operation on contention: no point in doing a potentially expensive reader sequence if we know at the beginning that we'll just end up re-doing it all. HOWEVER. Some users don't actually retry the operation, but instead will abort and do the operation with proper locking. So the sequence count case may be the optimistic quick case, but in the presense of writers you may want to do full locking in order to guarantee forward progress. The prime example of this would be the RCU name lookup. And in that case, you may well be better off without the "retry early", and are in a rush to instead get to the failure handling. Thus this "raw" interface that just returns the sequence number without testing it - it just forces the low bit to zero so that read_seqcount_retry() will always fail such a "active concurrent writer" scenario. Signed-off-by: Linus Torvalds --- include/linux/seqlock.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'include/linux/seqlock.h') diff --git a/include/linux/seqlock.h b/include/linux/seqlock.h index bb1fac5b8ee..600060e25ec 100644 --- a/include/linux/seqlock.h +++ b/include/linux/seqlock.h @@ -165,6 +165,27 @@ static inline unsigned read_seqcount_begin(const seqcount_t *s) return ret; } +/** + * raw_seqcount_begin - begin a seq-read critical section + * @s: pointer to seqcount_t + * Returns: count to be passed to read_seqcount_retry + * + * raw_seqcount_begin opens a read critical section of the given seqcount. + * Validity of the critical section is tested by checking read_seqcount_retry + * function. + * + * Unlike read_seqcount_begin(), this function will not wait for the count + * to stabilize. If a writer is active when we begin, we will fail the + * read_seqcount_retry() instead of stabilizing at the beginning of the + * critical section. + */ +static inline unsigned raw_seqcount_begin(const seqcount_t *s) +{ + unsigned ret = ACCESS_ONCE(s->sequence); + smp_rmb(); + return ret & ~1; +} + /** * __read_seqcount_retry - end a seq-read critical section (without barrier) * @s: pointer to seqcount_t -- cgit v1.2.3-70-g09d2