RE: [PATCH] vduse: Tie vduse mgmtdev and its device

[Parav Pandit via Virtualization]

> From: Yongji Xie <xieyon...@bytedance.com>
> Sent: Monday, June 13, 2022 5:54 AM
> 
> On Mon, Jun 13, 2022 at 2:02 AM Parav Pandit <pa...@nvidia.com> wrote:
> >
> > vduse devices are not backed by any real devices such as PCI. Hence it
> > doesn't have any parent device linked to it.
> >
> > Kernel driver model in [1] suggests to avoid an empty device release
> > callback.
> >
> > Hence tie the mgmtdevice object's life cycle to an allocate dummy
> > struct device instead of static one.
> >
> > [1]
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.
> >
> kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git
> %2
> > Ftree%2FDocumentation%2Fcore-api%2Fkobject.rst%3Fh%3Dv5.18-
> rc7%23n284&
> >
> amp;data=05%7C01%7Cparav%40nvidia.com%7C6b16045f2b7a4e168bc008da
> 4d2277
> >
> f7%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C6379071076788198
> 90%7CU
> >
> nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI
> 6Ik1ha
> >
> WwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=G8R1%2BsfBPTe4P
> sOYqrv0pAhM
> > 3qFg%2F%2BWw7GmTli8%2BNNw%3D&amp;reserved=0
> >
> > Signed-off-by: Parav Pandit <pa...@nvidia.com>
> > ---
> >  drivers/vdpa/vdpa_user/vduse_dev.c | 60
> > ++++++++++++++++++------------
> >  1 file changed, 37 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
> > b/drivers/vdpa/vdpa_user/vduse_dev.c
> > index f85d1a08ed87..ebe272575fb8 100644
> > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > @@ -1475,16 +1475,12 @@ static char *vduse_devnode(struct device
> *dev, umode_t *mode)
> >         return kasprintf(GFP_KERNEL, "vduse/%s", dev_name(dev));  }
> >
> > -static void vduse_mgmtdev_release(struct device *dev) -{ -}
> > -
> > -static struct device vduse_mgmtdev = {
> > -       .init_name = "vduse",
> > -       .release = vduse_mgmtdev_release,
> > +struct vduse_mgmt_dev {
> > +       struct vdpa_mgmt_dev mgmt_dev;
> > +       struct device dev;
> >  };
> >
> > -static struct vdpa_mgmt_dev mgmt_dev;
> > +static struct vduse_mgmt_dev *vduse_mgmt;
> >
> >  static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char
> > *name)  { @@ -1509,7 +1505,7 @@ static int vduse_dev_init_vdpa(struct
> > vduse_dev *dev, const char *name)
> >         }
> >         set_dma_ops(&vdev->vdpa.dev, &vduse_dev_dma_ops);
> >         vdev->vdpa.dma_dev = &vdev->vdpa.dev;
> > -       vdev->vdpa.mdev = &mgmt_dev;
> > +       vdev->vdpa.mdev = &vduse_mgmt->mgmt_dev;
> >
> >         return 0;
> >  }
> > @@ -1555,34 +1551,52 @@ static struct virtio_device_id id_table[] = {
> >         { 0 },
> >  };
> >
> > -static struct vdpa_mgmt_dev mgmt_dev = {
> > -       .device = &vduse_mgmtdev,
> > -       .id_table = id_table,
> > -       .ops = &vdpa_dev_mgmtdev_ops,
> > -};
> > +static void vduse_mgmtdev_release(struct device *dev) {
> > +       struct vduse_mgmt_dev *mgmt_dev;
> > +
> > +       mgmt_dev = container_of(dev, struct vduse_mgmt_dev, dev);
> > +       kfree(mgmt_dev);
> > +}
> >
> >  static int vduse_mgmtdev_init(void)
> >  {
> >         int ret;
> >
> > -       ret = device_register(&vduse_mgmtdev);
> > -       if (ret)
> > +       vduse_mgmt = kzalloc(sizeof(*vduse_mgmt), GFP_KERNEL);
> > +       if (!vduse_mgmt)
> > +               return -ENOMEM;
> > +
> > +       ret = dev_set_name(&vduse_mgmt->dev, "vduse-la");
> 
> Do we need to keep using "vduse" as the device name. We have
> documented it in qemu docs.
Yes. I added suffix of -la while testing to ensure a new code in action.
I missed to remove it.
Will send v2 with the correction.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization