Messages in this thread |  | | | Date | Fri, 09 Oct 2009 17:02:30 +0800 | | From | Amerigo Wang <> | | Subject | Re: [Patch v4] rwsem: fix rwsem_is_locked() bugs |
| |
David Howells wrote: > Amerigo Wang <amwang@redhat.com> wrote: > >> 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. >> >> 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> > > I'd say the comment in __rwsem_do_wake() is unnecessary, but other than > that...
The reason why I added it is to show that we have considered that case already. :) If you have strong opinions to remove it, I can update the patch.
> > Acked-by: David Howells <dhowells@redhat.com>
Thanks!
|  |