linux-kernel - RE: [PATCH vfio 1/2] hisi_acc_vfio_pci: Change reset_lock to mutex

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <BN9PR11MB5276D3527421A07C5A9E4BD28CBCA@BN9PR11MB5276.namprd11.prod.outlook.com>
Date:   Tue, 28 Nov 2023 08:05:57 +0000
From:   "Tian, Kevin" <kevin.tian@...el.com>
To:     Jason Gunthorpe <jgg@...pe.ca>,
        Shameerali Kolothum Thodi 
        <shameerali.kolothum.thodi@...wei.com>
CC:     Brett Creeley <brett.creeley@....com>,
        "yishaih@...dia.com" <yishaih@...dia.com>,
        liulongfang <liulongfang@...wei.com>,
        "alex.williamson@...hat.com" <alex.williamson@...hat.com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "shannon.nelson@....com" <shannon.nelson@....com>
Subject: RE: [PATCH vfio 1/2] hisi_acc_vfio_pci: Change reset_lock to
 mutex_lock

> From: Jason Gunthorpe <jgg@...pe.ca>
> Sent: Tuesday, November 28, 2023 8:46 AM
> 
> On Fri, Nov 24, 2023 at 08:46:58AM +0000, Shameerali Kolothum Thodi wrote:
> > > diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> > > b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> > > index b2f9778c8366..2c049b8de4b4 100644
> > > --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> > > +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> > > @@ -638,17 +638,17 @@ static void
> > >  hisi_acc_vf_state_mutex_unlock(struct hisi_acc_vf_core_device
> > > *hisi_acc_vdev)
> > >  {
> > >  again:
> > > -	spin_lock(&hisi_acc_vdev->reset_lock);
> > > +	mutex_lock(&hisi_acc_vdev->reset_mutex);
> > >  	if (hisi_acc_vdev->deferred_reset) {
> > >  		hisi_acc_vdev->deferred_reset = false;
> > > -		spin_unlock(&hisi_acc_vdev->reset_lock);
> > > +		mutex_unlock(&hisi_acc_vdev->reset_mutex);
> >
> > Don't think we have that sleeping while atomic case for this here.
> > Same for mlx5 as well. But if the idea is to have a common locking
> > across vendor drivers, it is fine.
> 
> Yeah, I'm not sure about changing spinlocks to mutex's for no reason..
> If we don't sleep and don't hold it for very long then the spinlock is
> appropriate
> 

It's me suggesting Brett to fix other two drivers, expecting a common
locking pattern would cause less confusion to future new variant drivers.
If both of you don't think it necessary then let's drop it. 😊