Re: [PATCH V3 6/7] virtio: introduce a mdev based transport

[Jason Wang]

On 2019/10/15 上午1:39, Stefan Hajnoczi wrote:

On Fri, Oct 11, 2019 at 04:15:56PM +0800, Jason Wang wrote:

+struct virtio_mdev_device {
+   struct virtio_device vdev;
+   struct mdev_device *mdev;
+   unsigned long version;
+
+   struct virtqueue **vqs;
+   /* The lock to protect virtqueue list */
+   spinlock_t lock;
+   struct list_head virtqueues;

Is this a list of struct virtio_mdev_vq_info?  Please document the
actual type in a comment.



Ok.



+static int virtio_mdev_find_vqs(struct virtio_device *vdev, unsigned nvqs,
+   struct virtqueue *vqs[],
+   vq_callback_t *callbacks[],
+   const char * const names[],
+   const bool *ctx,
+   struct irq_affinity *desc)
+{
+   struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
+   struct mdev_device *mdev = vm_get_mdev(vdev);
+   const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev);
+   struct virtio_mdev_callback cb;
+   int i, err, queue_idx = 0;
+
+   vm_dev->vqs = kmalloc_array(queue_idx, sizeof(*vm_dev->vqs),
+   GFP_KERNEL);

kmalloc_array(0, ...)?  I would have expected nvqs instead of queue_idx
(0).

What is this the purpose of vm_dev->vqs and does anything ever access it?



It's useless, will remove it.

Thanks

Re: [PATCH V3 6/7] virtio: introduce a mdev based transport

[Stefan Hajnoczi]

On Fri, Oct 11, 2019 at 04:15:56PM +0800, Jason Wang wrote:
> +struct virtio_mdev_device {
> + struct virtio_device vdev;
> + struct mdev_device *mdev;
> + unsigned long version;
> +
> + struct virtqueue **vqs;
> + /* The lock to protect virtqueue list */
> + spinlock_t lock;
> + struct list_head virtqueues;

Is this a list of struct virtio_mdev_vq_info?  Please document the
actual type in a comment.

> +static int virtio_mdev_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> + struct virtqueue *vqs[],
> + vq_callback_t *callbacks[],
> + const char * const names[],
> + const bool *ctx,
> + struct irq_affinity *desc)
> +{
> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
> + struct mdev_device *mdev = vm_get_mdev(vdev);
> + const struct virtio_mdev_device_ops *ops = mdev_get_dev_ops(mdev);
> + struct virtio_mdev_callback cb;
> + int i, err, queue_idx = 0;
> +
> + vm_dev->vqs = kmalloc_array(queue_idx, sizeof(*vm_dev->vqs),
> + GFP_KERNEL);

kmalloc_array(0, ...)?  I would have expected nvqs instead of queue_idx
(0).

What is this the purpose of vm_dev->vqs and does anything ever access it?


signature.asc
Description: PGP signature