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

Jason Gunthorpe
2021-04-27T22:10:36Z

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.

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

Jason