Re: [PATCH v2 07/13] vfio/ccw: Convert to use vfio_register_group_dev()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Apr 28, 2021 at 08:55:51AM -0400, Eric Farman wrote:
> On Tue, 2021-04-27 at 19:10 -0300, Jason Gunthorpe wrote:
> > On Tue, Apr 27, 2021 at 04:06:04PM -0400, Eric Farman wrote:
> > > > @@ -132,19 +137,28 @@ static int vfio_ccw_mdev_create(struct
> > > > mdev_device *mdev)
> > > >  			   private->sch->schid.ssid,
> > > >  			   private->sch->schid.sch_no);
> > > >  
> > > > +	ret = vfio_register_group_dev(&private->vdev);
> > > > +	if (ret)
> > > > +		goto err_atomic;
> > > > +	dev_set_drvdata(&mdev->dev, private);
> > > >  	return 0;
> > > > +
> > > > +err_atomic:
> > > > +	atomic_inc(&private->avail);
> > > 
> > > Since we're unwinding, should also do
> > > 
> > > private->mdev = NULL
> > > private->state = VFIO_CCW_STATE_STANDBY
> > 
> > I can change this, but it looks quite weird to do stuff like this
> > with
> > no locking.
> 
> I agree, but mdev_create didn't fail before, so backing out part of its
> work seems weird too.

Before if vfio_register_group_dev() failed the device would be left
half created but without a driver attached. It wasn't good.

The way it should work is up until vfio_register_group_dev() returns
success there should be no concurrancy and no touches to 'private' -
those WQs should all be shutdown.

Ideally the private would be allocated here as well so these rules are
clear and obvious

Jason



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux