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 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.

> 
> eg the only reads are here:
> 
> drivers/s390/cio/vfio_ccw_drv.c:        if (private->mdev &&
> is_final)
> drivers/s390/cio/vfio_ccw_drv.c:                private->state =
> private->mdev ? VFIO_CCW_STATE_IDLE :
> 
> Which is from a WQ, if someone thinks setting mdev to NULL should
> effect those WQs then there are problems...
> 
> The non-atomic state is equally confusing

Agreed, it's already on the list.

Eric

> 
> Jason




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux