mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
synced 2024-12-29 09:12:07 +00:00
ipc/sem.c: change memory barrier in sem_lock() to smp_rmb()
When I fixed bugs in the sem_lock() logic, I was more conservative than necessary. Therefore it is safe to replace the smp_mb() with smp_rmb(). And: With smp_rmb(), semop() syscalls are up to 10% faster. The race we must protect against is: sem->lock is free sma->complex_count = 0 sma->sem_perm.lock held by thread B thread A: A: spin_lock(&sem->lock) B: sma->complex_count++; (now 1) B: spin_unlock(&sma->sem_perm.lock); A: spin_is_locked(&sma->sem_perm.lock); A: XXXXX memory barrier A: if (sma->complex_count == 0) Thread A must read the increased complex_count value, i.e. the read must not be reordered with the read of sem_perm.lock done by spin_is_locked(). Since it's about ordering of reads, smp_rmb() is sufficient. [akpm@linux-foundation.org: update sem_lock() comment, from Davidlohr] Signed-off-by: Manfred Spraul <manfred@colorfullife.com> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net> Acked-by: Rafael Aquini <aquini@redhat.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
parent
a060bfe032
commit
2e094abfd1
13
ipc/sem.c
13
ipc/sem.c
@ -326,10 +326,17 @@ static inline int sem_lock(struct sem_array *sma, struct sembuf *sops,
|
||||
|
||||
/* Then check that the global lock is free */
|
||||
if (!spin_is_locked(&sma->sem_perm.lock)) {
|
||||
/* spin_is_locked() is not a memory barrier */
|
||||
smp_mb();
|
||||
/*
|
||||
* The ipc object lock check must be visible on all
|
||||
* cores before rechecking the complex count. Otherwise
|
||||
* we can race with another thread that does:
|
||||
* complex_count++;
|
||||
* spin_unlock(sem_perm.lock);
|
||||
*/
|
||||
smp_rmb();
|
||||
|
||||
/* Now repeat the test of complex_count:
|
||||
/*
|
||||
* Now repeat the test of complex_count:
|
||||
* It can't change anymore until we drop sem->lock.
|
||||
* Thus: if is now 0, then it will stay 0.
|
||||
*/
|
||||
|
Loading…
Reference in New Issue
Block a user