Since commit 48720ba56891 ("virtio/s390: use DMA memory for ccw I/O and
classic notifiers") we were supposed to make sure that
virtio_ccw_release_dev() completes before the ccw device, and the
attached dma pool are torn down, but unfortunately we did not.
Before that commit it used to be OK to delay cleaning up the memory
allocated by virtio-ccw indefinitely (which isn't really intuitive for
guys used to destruction happens in reverse construction order).
To accomplish this let us take a reference on the ccw device before we
allocate the dma_area and give it up after dma_area was freed.
Signed-off-by: Halil Pasic <pasic@xxxxxxxxxxxxx>
Fixes: 48720ba56891 ("virtio/s390: use DMA memory for ccw I/O and
classic notifiers")
Reported-by: bfu@xxxxxxxxxx
---
I'm not certain this is the only hot-unplug and teardonw related problem
with virtio-ccw.
Some things that are not perfectly clear to me:
* What would happen if we observed an hot-unplug while we are doing
wait_event() in ccw_io_helper()? Do we get stuck? I don't thin we
are guaranteed to receive an irq for a subchannel that is gone.
* cdev->online seems to be manipulated under cdev->ccwlock, but
in virtio_ccw_remove() we look at it to decide should we clean up
or not. What is the idea there? I guess we want to avoid doing
if nothing is there or twice. But I don't understand how stuff
interlocks.
* Can virtio_ccw_remove() get called while !cdev->online and
virtio_ccw_online() is running on a different cpu? If yes, what would
happen then?
The main addresse of these questions is Conny ;).
An alternative to this approach would be to inc and dec the refcount
in ccw_device_dma_zalloc() and ccw_device_dma_free() respectively.
---
drivers/s390/virtio/virtio_ccw.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index d35e7a3f7067..99141df3259b 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -1006,10 +1006,12 @@ static void virtio_ccw_release_dev(struct device *_d)
{
struct virtio_device *dev = dev_to_virtio(_d);
struct virtio_ccw_device *vcdev = to_vc_device(dev);
+ struct ccw_device *cdev = READ_ONCE(vcdev->cdev);
ccw_device_dma_free(vcdev->cdev, vcdev->dma_area,
sizeof(*vcdev->dma_area));
kfree(vcdev);
+ put_device(&cdev->dev);
}
static int irb_is_error(struct irb *irb)
@@ -1262,6 +1264,7 @@ static int virtio_ccw_online(struct ccw_device *cdev)
struct virtio_ccw_device *vcdev;
unsigned long flags;
+ get_device(&cdev->dev);
vcdev = kzalloc(sizeof(*vcdev), GFP_KERNEL);
if (!vcdev) {
dev_warn(&cdev->dev, "Could not get memory for virtio\n");
@@ -1315,6 +1318,7 @@ static int virtio_ccw_online(struct ccw_device *cdev)
sizeof(*vcdev->dma_area));
}
kfree(vcdev);
+ put_device(&cdev->dev);
return ret;
}
base-commit: 3ca706c189db861b2ca2019a0901b94050ca49d8
--
2.25.1
