Re: [PATCH v1 03/18] vfio/ccw: Ensure mdev->dev is cleared on mdev remove

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 03, 2022 at 09:25:19AM -0400, Matthew Rosato wrote:
> On 6/2/22 1:19 PM, Eric Farman wrote:
> > The mdev is linked with the vfio_ccw_private pointer when the mdev
> > is probed, but it's not cleared once the mdev is removed.
> > 
> > This isn't much of a concern based on the current device lifecycle,
> > but fix it so that things make sense in later shuffling.
> > 
> > Fixes: 3bf1311f351ef ("vfio/ccw: Convert to use vfio_register_emulated_iommu_dev()")
> > Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxx>
> >   drivers/s390/cio/vfio_ccw_ops.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
> > index a403d059a4e6..a0a3200b0b04 100644
> > +++ b/drivers/s390/cio/vfio_ccw_ops.c
> > @@ -159,6 +159,7 @@ static void vfio_ccw_mdev_remove(struct mdev_device *mdev)
> >   			   private->sch->schid.ssid,
> >   			   private->sch->schid.sch_no);
> > +	dev_set_drvdata(&mdev->dev, NULL);
> >   	vfio_unregister_group_dev(&private->vdev);
> >   	if ((private->state != VFIO_CCW_STATE_NOT_OPER) &&
> 
> Seems harmless enough.
> 
> Reviewed-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>
> 
> But is this just precautionary or is it fixing a real problem (if the former
> I don't think a fixes tag makes sense)
> 
> I also ask because I note vfio-ap clears its driver_data in mdev_remove but
> also leaves the pointer set, meaning they might need a similar cleanup and
> should probably have a look (CC Tony & Jason H)

There should be no reason to clear the drvdata on remove - the driver
must be designed to guarentee all references to the dev stop before
the remove function returns.

Jason



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux