Messages in this thread Patch in this message |  | | Date | Tue, 29 Sep 2009 23:19:02 -0400 | | From | Amerigo Wang <> | | Subject | [Patch] rwsem: fix rwsem_is_locked() bug |
| |
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.
Brian has a kernel module to reproduce this, I can include it if any of you need. Of course, with Brian's approval.
With this patch applied, I can't trigger that bug any more.
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> Cc: Stable Team <stable@kernel.org>
--- diff --git a/lib/rwsem-spinlock.c b/lib/rwsem-spinlock.c index 9df3ca5..44e4484 100644 --- a/lib/rwsem-spinlock.c +++ b/lib/rwsem-spinlock.c @@ -49,7 +49,6 @@ __rwsem_do_wake(struct rw_semaphore *sem, int wakewrite) { struct rwsem_waiter *waiter; struct task_struct *tsk; - int woken; waiter = list_entry(sem->wait_list.next, struct rwsem_waiter, list); @@ -78,24 +77,21 @@ __rwsem_do_wake(struct rw_semaphore *sem, int wakewrite) /* grant an infinite number of read locks to the front of the queue */ dont_wake_writers: - woken = 0; while (waiter->flags & RWSEM_WAITING_FOR_READ) { struct list_head *next = waiter->list.next; + sem->activity++; list_del(&waiter->list); tsk = waiter->task; smp_mb(); waiter->task = NULL; wake_up_process(tsk); put_task_struct(tsk); - woken++; if (list_empty(&sem->wait_list)) break; waiter = list_entry(next, struct rwsem_waiter, list); } - sem->activity += woken; - out: return sem; }
|  |