Messages in this thread | | | Subject | Re: [PATCH] vfio/mtty: Enforce available_instances | From | Kirti Wankhede <> | Date | Tue, 29 Jun 2021 01:22:00 +0530 |
| |
On 6/29/2021 12:26 AM, Alex Williamson wrote: > On Mon, 28 Jun 2021 23:19:54 +0530 > Kirti Wankhede <kwankhede@nvidia.com> wrote: > >> On 6/26/2021 2:56 AM, Alex Williamson wrote: >>> The sample mtty mdev driver doesn't actually enforce the number of >>> device instances it claims are available. Implement this properly. >>> >>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com> >>> --- >>> >>> Applies to vfio next branch + Jason's atomic conversion >>> >> >> >> Does this need to be on top of Jason's patch? > > Yes, see immediately above. > >> Patch to use mdev_used_ports is reverted here, can it be changed from >> mdev_devices_list to mdev_avail_ports atomic variable? > > It doesn't revert Jason's change, it builds on it. The patches could > we squashed, but there's no bug in Jason's patch that we're trying to > avoid exposing, so I don't see why we'd do that. >
'Squashed' is the correct word that 'revert', my bad.
>> Change here to use atomic variable looks good to me. >> >> Reviewed by: Kirti Wankhede <kwankhede@nvidia.com> > > Thanks! It was Jason's patch[1] that converted to use an atomic > though, so I'm slightly confused if this R-b is for the patch below, > Jason's patch, or both. Thanks,
I liked 'mdev_avail_ports' approach than 'mdev_used_ports' approach here. This R-b is for below patch.
Thanks, Kirti
> > Alex > > [1]https://lore.kernel.org/kvm/0-v1-0bc56b362ca7+62-mtty_used_ports_jgg@nvidia.com/ > >>> samples/vfio-mdev/mtty.c | 22 ++++++++++++++++------ >>> 1 file changed, 16 insertions(+), 6 deletions(-) >>> >>> diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c >>> index ffbaf07a17ea..8b26fecc4afe 100644 >>> --- a/samples/vfio-mdev/mtty.c >>> +++ b/samples/vfio-mdev/mtty.c >>> @@ -144,7 +144,7 @@ struct mdev_state { >>> int nr_ports; >>> }; >>> >>> -static atomic_t mdev_used_ports; >>> +static atomic_t mdev_avail_ports = ATOMIC_INIT(MAX_MTTYS); >>> >>> static const struct file_operations vd_fops = { >>> .owner = THIS_MODULE, >>> @@ -707,11 +707,20 @@ static int mtty_probe(struct mdev_device *mdev) >>> { >>> struct mdev_state *mdev_state; >>> int nr_ports = mdev_get_type_group_id(mdev) + 1; >>> + int avail_ports = atomic_read(&mdev_avail_ports); >>> int ret; >>> >>> + do { >>> + if (avail_ports < nr_ports) >>> + return -ENOSPC; >>> + } while (!atomic_try_cmpxchg(&mdev_avail_ports, >>> + &avail_ports, avail_ports - nr_ports)); >>> + >>> mdev_state = kzalloc(sizeof(struct mdev_state), GFP_KERNEL); >>> - if (mdev_state == NULL) >>> + if (mdev_state == NULL) { >>> + atomic_add(nr_ports, &mdev_avail_ports); >>> return -ENOMEM; >>> + } >>> >>> vfio_init_group_dev(&mdev_state->vdev, &mdev->dev, &mtty_dev_ops); >>> >>> @@ -724,6 +733,7 @@ static int mtty_probe(struct mdev_device *mdev) >>> >>> if (mdev_state->vconfig == NULL) { >>> kfree(mdev_state); >>> + atomic_add(nr_ports, &mdev_avail_ports); >>> return -ENOMEM; >>> } >>> >>> @@ -735,9 +745,9 @@ static int mtty_probe(struct mdev_device *mdev) >>> ret = vfio_register_group_dev(&mdev_state->vdev); >>> if (ret) { >>> kfree(mdev_state); >>> + atomic_add(nr_ports, &mdev_avail_ports); >>> return ret; >>> } >>> - atomic_add(mdev_state->nr_ports, &mdev_used_ports); >>> >>> dev_set_drvdata(&mdev->dev, mdev_state); >>> return 0; >>> @@ -746,12 +756,13 @@ static int mtty_probe(struct mdev_device *mdev) >>> static void mtty_remove(struct mdev_device *mdev) >>> { >>> struct mdev_state *mdev_state = dev_get_drvdata(&mdev->dev); >>> + int nr_ports = mdev_state->nr_ports; >>> >>> - atomic_sub(mdev_state->nr_ports, &mdev_used_ports); >>> vfio_unregister_group_dev(&mdev_state->vdev); >>> >>> kfree(mdev_state->vconfig); >>> kfree(mdev_state); >>> + atomic_add(nr_ports, &mdev_avail_ports); >>> } >>> >>> static int mtty_reset(struct mdev_state *mdev_state) >>> @@ -1271,8 +1282,7 @@ static ssize_t available_instances_show(struct mdev_type *mtype, >>> { >>> unsigned int ports = mtype_get_type_group_id(mtype) + 1; >>> >>> - return sprintf(buf, "%d\n", >>> - (MAX_MTTYS - atomic_read(&mdev_used_ports)) / ports); >>> + return sprintf(buf, "%d\n", atomic_read(&mdev_avail_ports) / ports); >>> } >>> >>> static MDEV_TYPE_ATTR_RO(available_instances); >>> >>> >> >
| |