Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 15 Jul 2021 13:31:36 +0200
From:   Halil Pasic <pasic@linux.ibm.com>
To:     Jason Gunthorpe <jgg@nvidia.com>
Cc:     Tony Krowiak <akrowiak@linux.ibm.com>, linux-s390@vger.kernel.org,
        linux-kernel@vger.kernel.org, borntraeger@de.ibm.com,
        cohuck@redhat.com, pasic@linux.vnet.ibm.com, jjherne@linux.ibm.com,
        alex.williamson@redhat.com, kwankhede@nvidia.com,
        frankja@linux.ibm.com, david@redhat.com, imbrenda@linux.ibm.com,
        hca@linux.ibm.com
Subject: Re: [PATCH] s390/vfio-ap: do not open code locks for
 VFIO_GROUP_NOTIFY_SET_KVM notification
Message-ID: <20210715133136.420c40b0.pasic@linux.ibm.com>
In-Reply-To: <20210713170533.GF136586@nvidia.com>
References: <20210707154156.297139-1-akrowiak@linux.ibm.com>
        <20210713013815.57e8a8cb.pasic@linux.ibm.com>
        <5dd3cc05-f789-21a3-50c7-ee80d850a105@linux.ibm.com>
        <20210713184517.48eacee6.pasic@linux.ibm.com>
        <20210713170533.GF136586@nvidia.com>
Organization: IBM
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
Precedence: bulk

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