[v2,4/7] vfio/ccw: move private to mdev lifecycle
Commit Message
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(-)
Comments
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>
>
@@ -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",
@@ -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)
@@ -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;
/*