| Message ID | 20221102150152.2521475-5-farman@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio-ccw parent rework | expand |
On 11/2/22 11:01 AM, Eric Farman wrote: > Now that the mdev parent data is split out into its own struct, > it is safe to move the remaining private data to follow the > mdev probe/remove lifecycle. The mdev parent data will remain > where it is, and follow the subchannel and the css driver > interfaces. > > Signed-off-by: Eric Farman <farman@linux.ibm.com> > --- > drivers/s390/cio/vfio_ccw_drv.c | 15 +-------------- > drivers/s390/cio/vfio_ccw_ops.c | 26 +++++++++++++------------- > drivers/s390/cio/vfio_ccw_private.h | 2 ++ > 3 files changed, 16 insertions(+), 27 deletions(-) > ... > diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c > index eb0b8cc210bb..e45d4acb109b 100644 > --- a/drivers/s390/cio/vfio_ccw_ops.c > +++ b/drivers/s390/cio/vfio_ccw_ops.c > @@ -100,15 +100,20 @@ static int vfio_ccw_mdev_probe(struct mdev_device *mdev) > { > struct subchannel *sch = to_subchannel(mdev->dev.parent); > struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev); > - struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev); > + struct vfio_ccw_private *private; > int ret; > > - if (private->state == VFIO_CCW_STATE_NOT_OPER) > - return -ENODEV; > + private = kzalloc(sizeof(*private), GFP_KERNEL); > + if (!private) > + return -ENOMEM; Ha, looks like you time traveled and took my advice :) In fact it looks like some of my other comments from patch 1 get cleaned up here too -- but would still be good to make those changes in patch 1 for completeness/bisect. Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
On Thu, 2022-11-03 at 19:22 -0400, Matthew Rosato wrote: > On 11/2/22 11:01 AM, Eric Farman wrote: > > Now that the mdev parent data is split out into its own struct, > > it is safe to move the remaining private data to follow the > > mdev probe/remove lifecycle. The mdev parent data will remain > > where it is, and follow the subchannel and the css driver > > interfaces. > > > > Signed-off-by: Eric Farman <farman@linux.ibm.com> > > --- > > drivers/s390/cio/vfio_ccw_drv.c | 15 +-------------- > > drivers/s390/cio/vfio_ccw_ops.c | 26 +++++++++++++------------ > > - > > drivers/s390/cio/vfio_ccw_private.h | 2 ++ > > 3 files changed, 16 insertions(+), 27 deletions(-) > > > > ... > > > diff --git a/drivers/s390/cio/vfio_ccw_ops.c > > b/drivers/s390/cio/vfio_ccw_ops.c > > index eb0b8cc210bb..e45d4acb109b 100644 > > --- a/drivers/s390/cio/vfio_ccw_ops.c > > +++ b/drivers/s390/cio/vfio_ccw_ops.c > > @@ -100,15 +100,20 @@ static int vfio_ccw_mdev_probe(struct > > mdev_device *mdev) > > { > > struct subchannel *sch = to_subchannel(mdev->dev.parent); > > struct vfio_ccw_parent *parent = dev_get_drvdata(&sch- > > >dev); > > - struct vfio_ccw_private *private = dev_get_drvdata(&parent- > > >dev); > > + struct vfio_ccw_private *private; > > int ret; > > > > - if (private->state == VFIO_CCW_STATE_NOT_OPER) > > - return -ENODEV; > > + private = kzalloc(sizeof(*private), GFP_KERNEL); > > + if (!private) > > + return -ENOMEM; > > Ha, looks like you time traveled and took my advice :) Ha, I forgot I did this in the future. :) > > In fact it looks like some of my other comments from patch 1 get > cleaned up here too -- but would still be good to make those changes > in patch 1 for completeness/bisect. Agreed, I'll pull those down to patch 1; thanks. > > Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> >
diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c index fd5720cbf005..041cc0860f0e 100644 --- a/drivers/s390/cio/vfio_ccw_drv.c +++ b/drivers/s390/cio/vfio_ccw_drv.c @@ -151,7 +151,7 @@ static void vfio_ccw_sch_irq(struct subchannel *sch) vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_INTERRUPT); } -static void vfio_ccw_free_private(struct vfio_ccw_private *private) +void vfio_ccw_free_private(struct vfio_ccw_private *private) { struct vfio_ccw_crw *crw, *temp; @@ -179,7 +179,6 @@ static void vfio_ccw_free_parent(struct device *dev) static int vfio_ccw_sch_probe(struct subchannel *sch) { struct pmcw *pmcw = &sch->schib.pmcw; - struct vfio_ccw_private *private; struct vfio_ccw_parent *parent; int ret = -ENOMEM; @@ -200,14 +199,7 @@ static int vfio_ccw_sch_probe(struct subchannel *sch) if (ret) goto out_free; - private = kzalloc(sizeof(*private), GFP_KERNEL); - if (IS_ERR(private)) { - put_device(&parent->dev); - return PTR_ERR(private); - } - dev_set_drvdata(&sch->dev, parent); - dev_set_drvdata(&parent->dev, private); parent->mdev_type.sysfs_name = "io"; parent->mdev_type.pretty_name = "I/O subchannel (Non-QDIO)"; @@ -226,9 +218,7 @@ static int vfio_ccw_sch_probe(struct subchannel *sch) out_unreg: device_unregister(&parent->dev); out_free: - dev_set_drvdata(&parent->dev, NULL); dev_set_drvdata(&sch->dev, NULL); - vfio_ccw_free_private(private); put_device(&parent->dev); return ret; } @@ -236,14 +226,11 @@ static int vfio_ccw_sch_probe(struct subchannel *sch) static void vfio_ccw_sch_remove(struct subchannel *sch) { struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev); - struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev); mdev_unregister_parent(&parent->parent); device_unregister(&parent->dev); dev_set_drvdata(&sch->dev, NULL); - - vfio_ccw_free_private(private); put_device(&parent->dev); VFIO_CCW_MSG_EVENT(4, "unbound from subchannel %x.%x.%04x\n", diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c index eb0b8cc210bb..e45d4acb109b 100644 --- a/drivers/s390/cio/vfio_ccw_ops.c +++ b/drivers/s390/cio/vfio_ccw_ops.c @@ -100,15 +100,20 @@ static int vfio_ccw_mdev_probe(struct mdev_device *mdev) { struct subchannel *sch = to_subchannel(mdev->dev.parent); struct vfio_ccw_parent *parent = dev_get_drvdata(&sch->dev); - struct vfio_ccw_private *private = dev_get_drvdata(&parent->dev); + struct vfio_ccw_private *private; int ret; - if (private->state == VFIO_CCW_STATE_NOT_OPER) - return -ENODEV; + private = kzalloc(sizeof(*private), GFP_KERNEL); + if (!private) + return -ENOMEM; ret = vfio_init_device(&private->vdev, &mdev->dev, &vfio_ccw_dev_ops); - if (ret) + if (ret) { + kfree(private); return ret; + } + + dev_set_drvdata(&parent->dev, private); VFIO_CCW_MSG_EVENT(2, "sch %x.%x.%04x: create\n", sch->schid.cssid, @@ -122,6 +127,7 @@ static int vfio_ccw_mdev_probe(struct mdev_device *mdev) return 0; err_put_vdev: + dev_set_drvdata(&parent->dev, NULL); vfio_put_device(&private->vdev); return ret; } @@ -131,15 +137,6 @@ static void vfio_ccw_mdev_release_dev(struct vfio_device *vdev) struct vfio_ccw_private *private = container_of(vdev, struct vfio_ccw_private, vdev); - /* - * We cannot free vfio_ccw_private here because it includes - * parent info which must be free'ed by css driver. - * - * Use a workaround by memset'ing the core device part and - * then notifying the remove path that all active references - * to this device have been released. - */ - memset(vdev, 0, sizeof(*vdev)); complete(&private->release_comp); } @@ -156,6 +153,7 @@ static void vfio_ccw_mdev_remove(struct mdev_device *mdev) vfio_unregister_group_dev(&private->vdev); + dev_set_drvdata(&parent->dev, NULL); vfio_put_device(&private->vdev); /* * Wait for all active references on mdev are released so it @@ -166,6 +164,8 @@ static void vfio_ccw_mdev_remove(struct mdev_device *mdev) * cycle. */ wait_for_completion(&private->release_comp); + + vfio_ccw_free_private(private); } static int vfio_ccw_mdev_open_device(struct vfio_device *vdev) diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h index 55d636225cff..747aba5f5272 100644 --- a/drivers/s390/cio/vfio_ccw_private.h +++ b/drivers/s390/cio/vfio_ccw_private.h @@ -134,6 +134,8 @@ int vfio_ccw_sch_quiesce(struct subchannel *sch); void vfio_ccw_sch_io_todo(struct work_struct *work); void vfio_ccw_crw_todo(struct work_struct *work); +void vfio_ccw_free_private(struct vfio_ccw_private *private); + extern struct mdev_driver vfio_ccw_mdev_driver; /*
Now that the mdev parent data is split out into its own struct, it is safe to move the remaining private data to follow the mdev probe/remove lifecycle. The mdev parent data will remain where it is, and follow the subchannel and the css driver interfaces. Signed-off-by: Eric Farman <farman@linux.ibm.com> --- drivers/s390/cio/vfio_ccw_drv.c | 15 +-------------- drivers/s390/cio/vfio_ccw_ops.c | 26 +++++++++++++------------- drivers/s390/cio/vfio_ccw_private.h | 2 ++ 3 files changed, 16 insertions(+), 27 deletions(-)