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
