Re: [PATCH] s390/vfio-ap: do not open code locks for VFIO_GROUP_NOTIFY_SET_KVM notification

[Halil Pasic <pasic@linux.ibm.com>]

On Tue, 13 Jul 2021 14:05:33 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Tue, Jul 13, 2021 at 06:45:17PM +0200, Halil Pasic wrote:
> 
> > Jason may give it another try to convince us that 0cc00c8d4050 only
> > silenced lockdep, but vfio_ap remained prone to deadlocks. To my best
> > knowledge using condition variable and a mutex is one of the well known
> > ways to implement an rwlock.   
> 
> The well known pattern is to use a rwsem.

I think you are missing the point. We are discussing whether
this qualifies for stable, i.e. if 0cc00c8d4050 is really broken
like the patch description says.

Using  a readers-writers lock (as a primitive) to implement a
a readers-writers lock is a fallacy, so I guess you wanted to
say that when a readers-writers lock is needed in the kernel the
obvious choices are rw_semaphore and/or rwlock_t (depending on the
spin).

What I wanted to say is using a condition variable and a mutex is
not per-see wrong, because one can even implement an readers-writers
lock with it. For reference see https://en.wikipedia.org/wiki/Readers%E2%80%93writer_lock


> 
> This:
>         wait_event_cmd(matrix_mdev->wait_for_kvm,
>                        !matrix_mdev->kvm_busy,
>                        mutex_unlock(&matrix_dev->lock),
>                        mutex_lock(&matrix_dev->lock));
> 
> 
> Is not really a rwsem, and is invsible to lockdep.
> 

I agree. But this is not a proof of a problem that qualifies to be fixed
using the stable process as documented in 
https://github.com/torvalds/linux/blob/master/Documentation/process/stable-kernel-rules.rst

I'm in favor of rewriting this to use rw_semaphore. I'm not in favor
of proclaiming this a fix for stable, because for that you first have
to prove that you fix a real problem.

I hope we are on the same page.

Regards,
Halil