diff options
author | Amerigo Wang <amwang@redhat.com> | 2009-12-14 21:00:21 -0500 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2009-12-15 11:53:26 -0500 |
commit | 29671f22a8b6522db3b126a3fdfb208759ce46e3 (patch) | |
tree | c3c434fc07aae30c401fe7662b8e1399b98ffdc2 | |
parent | 118d52da1816471ac875bb9f1ee51737e82b1d71 (diff) |
rwsem: fix rwsem_is_locked() bugs
rwsem_is_locked() tests ->activity without locks, so we should always keep
->activity consistent. However, the code in __rwsem_do_wake() breaks this
rule, it updates ->activity after _all_ readers waken up, this may give
some reader a wrong ->activity value, thus cause rwsem_is_locked() behaves
wrong.
Quote from Andrew:
"
- we have one or more processes sleeping in down_read(), waiting for access.
- we wake one or more processes up without altering ->activity
- they start to run and they do rwsem_is_locked(). This incorrectly
returns "false", because the waker process is still crunching away in
__rwsem_do_wake().
- the waker now alters ->activity, but it was too late.
"
So we need get a spinlock to protect this. And rwsem_is_locked() should
not block, thus we use spin_trylock_irqsave().
[akpm@linux-foundation.org: simplify code]
Reported-by: Brian Behlendorf <behlendorf1@llnl.gov>
Cc: Ben Woodard <bwoodard@llnl.gov>
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: WANG Cong <amwang@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r-- | include/linux/rwsem-spinlock.h | 6 | ||||
-rw-r--r-- | lib/rwsem-spinlock.c | 13 |
2 files changed, 14 insertions, 5 deletions
diff --git a/include/linux/rwsem-spinlock.h b/include/linux/rwsem-spinlock.h index 6c3c0f6c261f..bdfcc2527970 100644 --- a/include/linux/rwsem-spinlock.h +++ b/include/linux/rwsem-spinlock.h | |||
@@ -68,11 +68,7 @@ extern int __down_write_trylock(struct rw_semaphore *sem); | |||
68 | extern void __up_read(struct rw_semaphore *sem); | 68 | extern void __up_read(struct rw_semaphore *sem); |
69 | extern void __up_write(struct rw_semaphore *sem); | 69 | extern void __up_write(struct rw_semaphore *sem); |
70 | extern void __downgrade_write(struct rw_semaphore *sem); | 70 | extern void __downgrade_write(struct rw_semaphore *sem); |
71 | 71 | extern int rwsem_is_locked(struct rw_semaphore *sem); | |
72 | static inline int rwsem_is_locked(struct rw_semaphore *sem) | ||
73 | { | ||
74 | return (sem->activity != 0); | ||
75 | } | ||
76 | 72 | ||
77 | #endif /* __KERNEL__ */ | 73 | #endif /* __KERNEL__ */ |
78 | #endif /* _LINUX_RWSEM_SPINLOCK_H */ | 74 | #endif /* _LINUX_RWSEM_SPINLOCK_H */ |
diff --git a/lib/rwsem-spinlock.c b/lib/rwsem-spinlock.c index 39a74110f55b..ccf95bff7984 100644 --- a/lib/rwsem-spinlock.c +++ b/lib/rwsem-spinlock.c | |||
@@ -17,6 +17,19 @@ struct rwsem_waiter { | |||
17 | #define RWSEM_WAITING_FOR_WRITE 0x00000002 | 17 | #define RWSEM_WAITING_FOR_WRITE 0x00000002 |
18 | }; | 18 | }; |
19 | 19 | ||
20 | int rwsem_is_locked(struct rw_semaphore *sem) | ||
21 | { | ||
22 | int ret = 1; | ||
23 | unsigned long flags; | ||
24 | |||
25 | if (spin_trylock_irqsave(&sem->wait_lock, flags)) { | ||
26 | ret = (sem->activity != 0); | ||
27 | spin_unlock_irqrestore(&sem->wait_lock, flags); | ||
28 | } | ||
29 | return ret; | ||
30 | } | ||
31 | EXPORT_SYMBOL(rwsem_is_locked); | ||
32 | |||
20 | /* | 33 | /* |
21 | * initialise the semaphore | 34 | * initialise the semaphore |
22 | */ | 35 | */ |