rwsem: use single atomic update for sem count when waking up readers.
in [Kernel]
|
Prev: [PATCH] lib/atomic64_test: Fix missing include of linux/kernel.h
Next: [PATCH 9/9] lockup_detector: Fix forgotten config conversion
From: Michel Lespinasse on 12 May 2010 21:00 On Wed, May 12, 2010 at 4:01 AM, David Howells <dhowells(a)redhat.com> wrote: > Michel Lespinasse <walken(a)google.com> wrote: > >> - * � - there must be someone on the queue >> + * - there must be someone on the queue > > Why did you change this comment? �This is still a guarantee up_xxxx() must > make about the state of the rwsem. What I meant is that we do rely on there being someone on the queue even if not coming from up_xxxx(). >> + retry_readers: >> + � � oldcount = rwsem_atomic_update(woken, sem) - woken; >> + � � if (!downgrading && (oldcount & RWSEM_ACTIVE_MASK)) > > The problem with doing this here is that you may just have wasted all the work > you did working out what woken is going to be. �That may have been quite slow > as the CPU may have had to get hold of a bunch of cachelines that weren't in > its cache. �Furthermore, you are doing this under a spinlock, so you may have > lost your right to wake anyone up, and you'll be blocking the CPU that will be > allowed to perform the wakeup. > > Incrementing the count first nets you a guarantee that you have the right to > wake things up. > > You may point out that if there's no contention, then what your revised code > does doesn't slow anything down. �That's true, but on modern CPU's, neither > does the old code as the exclusively held cache line will lurk in the CPU's > cache until there's contention on it. My reasoning was more that in all cases except downgrade_write() (which gets an exemption anyway because we know nobody can grab the write lock from it) we've already checked the active count and found it to be zero, so there is not much point checking again at the start of __rwsem_do_wake(). However you are correct that incrementing the active count at that point could be important, just to guarantee that nobody can grab a write lock while we count how many readers we want to wake. I'll make sure to address this. -- Michel "Walken" Lespinasse A program is never fully debugged until the last user dies. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |