On Fri, Dec 4, 2020 at 8:08 AM Bernd Edlinger <[email protected]> wrote:
>
> >
> > -static void kcmp_unlock(struct mutex *m1, struct mutex *m2)
> > +static void kcmp_unlock(struct rw_semaphore *l1, struct rw_semaphore *l2)
> > {
> > - if (likely(m2 != m1))
> > - mutex_unlock(m2);
> > - mutex_unlock(m1);
> > + if (likely(l2 != l1))
>
> is this still necessary ?
>
> > + up_read(l2);
> > + up_read(l1);
> > }
> >
> > -static int kcmp_lock(struct mutex *m1, struct mutex *m2)
> > +static int kcmp_lock(struct rw_semaphore *l1, struct rw_semaphore *l2)
> > {
> > int err;
> >
> > - if (m2 > m1)
> > - swap(m1, m2);
> > + if (l2 > l1)
> > + swap(l1, l2);
>
> and this is probably also no longer necessary?
These are still necessary, because even a recursive read lock can
still block on a writer trying to come in between the two read locks
due to fairness guarantees.
So taking the same read lock twice is still a source of possible deadlocks.
For the same reason, read locks still have ABBA deadlock and need to
be taken in order.
So switching from a mutex to a rwlock doesn't really change the
locking rules in this respect.
In fact, I'm not convinced this change even fixes the deadlock that
syzbot reported, for the same reason: it just requires a write lock in
between two read locks to deadlock.
Linus
