From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jiri Pirko <jiri@resnulli.us>
Cc: virtualization@lists.linux.dev, jasowang@redhat.com,
xuanzhuo@linux.alibaba.com, eperezma@redhat.com,
parav@nvidia.com, feliu@nvidia.com, hengqi@linux.alibaba.com
Subject: Re: [PATCH virtio 17/19] virtio: convert the rest virtio_find_vqs() users to virtio_find_vqs_info()
Date: Wed, 3 Jul 2024 09:11:52 -0400 [thread overview]
Message-ID: <20240703090519-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20240703123913.969202-18-jiri@resnulli.us>
On Wed, Jul 03, 2024 at 02:39:11PM +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> Instead of passing separate names and callbacks arrays
> to virtio_find_vqs(), have one of virtual_queue_info structs and
> pass it to virtio_find_vqs_info().
>
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
> arch/um/drivers/virt-pci.c | 8 ++++---
> drivers/bluetooth/virtio_bt.c | 14 ++++-------
> drivers/firmware/arm_scmi/virtio.c | 11 ++++-----
> drivers/gpio/gpio-virtio.c | 10 ++++----
> drivers/gpu/drm/virtio/virtgpu_kms.c | 9 ++++----
> drivers/iommu/virtio-iommu.c | 11 ++++-----
> drivers/net/wireless/virtual/mac80211_hwsim.c | 14 ++++-------
> drivers/rpmsg/virtio_rpmsg_bus.c | 8 ++++---
> drivers/virtio/virtio_input.c | 9 ++++----
> net/vmw_vsock/virtio_transport.c | 17 +++++---------
> sound/virtio/virtio_card.c | 23 ++++++++-----------
> 11 files changed, 59 insertions(+), 75 deletions(-)
>
> diff --git a/arch/um/drivers/virt-pci.c b/arch/um/drivers/virt-pci.c
> index 7cb503469bbd..3df2ea53471a 100644
> --- a/arch/um/drivers/virt-pci.c
> +++ b/arch/um/drivers/virt-pci.c
> @@ -567,12 +567,14 @@ struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus)
>
> static int um_pci_init_vqs(struct um_pci_device *dev)
> {
> + struct virtio_queue_info vqs_info[] = {
> + { "cmd", um_pci_cmd_vq_cb },
> + { "irq", um_pci_irq_vq_cb },
> + };
> struct virtqueue *vqs[2];
> - static const char *const names[2] = { "cmd", "irq" };
> - vq_callback_t *cbs[2] = { um_pci_cmd_vq_cb, um_pci_irq_vq_cb };
> int err, i;
>
> - err = virtio_find_vqs(dev->vdev, 2, vqs, cbs, names, NULL);
> + err = virtio_find_vqs_info(dev->vdev, 2, vqs, vqs_info, NULL);
> if (err)
> return err;
>
> diff --git a/drivers/bluetooth/virtio_bt.c b/drivers/bluetooth/virtio_bt.c
> index 463b49ca2492..6481f9fe24c4 100644
> --- a/drivers/bluetooth/virtio_bt.c
> +++ b/drivers/bluetooth/virtio_bt.c
> @@ -254,13 +254,9 @@ static void virtbt_rx_done(struct virtqueue *vq)
>
> static int virtbt_probe(struct virtio_device *vdev)
> {
> - vq_callback_t *callbacks[VIRTBT_NUM_VQS] = {
> - [VIRTBT_VQ_TX] = virtbt_tx_done,
> - [VIRTBT_VQ_RX] = virtbt_rx_done,
> - };
> - const char *names[VIRTBT_NUM_VQS] = {
> - [VIRTBT_VQ_TX] = "tx",
> - [VIRTBT_VQ_RX] = "rx",
> + struct virtio_queue_info vqs_info[VIRTBT_NUM_VQS] = {
> + [VIRTBT_VQ_TX] = { "tx", virtbt_tx_done },
> + [VIRTBT_VQ_RX] = { "rx", virtbt_rx_done },
> };
> struct virtio_bluetooth *vbt;
> struct hci_dev *hdev;
> @@ -289,8 +285,8 @@ static int virtbt_probe(struct virtio_device *vdev)
>
> INIT_WORK(&vbt->rx, virtbt_rx_work);
>
> - err = virtio_find_vqs(vdev, VIRTBT_NUM_VQS, vbt->vqs, callbacks,
> - names, NULL);
> + err = virtio_find_vqs_info(vdev, VIRTBT_NUM_VQS, vbt->vqs,
> + vqs_info, NULL);
> if (err)
> return err;
>
> diff --git a/drivers/firmware/arm_scmi/virtio.c b/drivers/firmware/arm_scmi/virtio.c
> index 4892058445ce..a6ae59c03308 100644
> --- a/drivers/firmware/arm_scmi/virtio.c
> +++ b/drivers/firmware/arm_scmi/virtio.c
> @@ -354,11 +354,9 @@ static void scmi_vio_deferred_tx_worker(struct work_struct *work)
> scmi_vio_channel_release(vioch);
> }
>
> -static const char *const scmi_vio_vqueue_names[] = { "tx", "rx" };
> -
> -static vq_callback_t *scmi_vio_complete_callbacks[] = {
> - scmi_vio_complete_cb,
> - scmi_vio_complete_cb
> +static struct virtio_queue_info scmi_vio_vqs_info[] = {
> + { "tx", scmi_vio_complete_cb },
> + { "rx", scmi_vio_complete_cb },
> };
>
> static unsigned int virtio_get_max_msg(struct scmi_chan_info *base_cinfo)
> @@ -831,8 +829,7 @@ static int scmi_vio_probe(struct virtio_device *vdev)
> if (have_vq_rx)
> channels[VIRTIO_SCMI_VQ_RX].is_rx = true;
>
> - ret = virtio_find_vqs(vdev, vq_cnt, vqs, scmi_vio_complete_callbacks,
> - scmi_vio_vqueue_names, NULL);
> + ret = virtio_find_vqs_info(vdev, vq_cnt, vqs, scmi_vio_vqs_info, NULL);
> if (ret) {
> dev_err(dev, "Failed to get %d virtqueue(s)\n", vq_cnt);
> return ret;
> diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c
> index 9fae8e396c58..a45b392358c4 100644
> --- a/drivers/gpio/gpio-virtio.c
> +++ b/drivers/gpio/gpio-virtio.c
> @@ -457,15 +457,15 @@ static void virtio_gpio_free_vqs(struct virtio_device *vdev)
> static int virtio_gpio_alloc_vqs(struct virtio_gpio *vgpio,
> struct virtio_device *vdev)
> {
> - const char * const names[] = { "requestq", "eventq" };
> - vq_callback_t *cbs[] = {
> - virtio_gpio_request_vq,
> - virtio_gpio_event_vq,
> + struct virtio_queue_info vqs_info[] = {
> + { "requestq", virtio_gpio_request_vq },
> + { "eventq", virtio_gpio_event_vq },
> };
I'd maybe do struct virtio_queue_info vqs_info[2]
so it's clear array is the right size.
Not a new issue so can be a separate cleanup.
> struct virtqueue *vqs[2] = { NULL, NULL };
> int ret;
>
> - ret = virtio_find_vqs(vdev, vgpio->irq_lines ? 2 : 1, vqs, cbs, names, NULL);
> + ret = virtio_find_vqs_info(vdev, vgpio->irq_lines ? 2 : 1, vqs,
> + vqs_info, NULL);
> if (ret) {
> dev_err(&vdev->dev, "failed to find vqs: %d\n", ret);
> return ret;
> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> index 5a3b5aaed1f3..938c08a707eb 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> @@ -116,11 +116,10 @@ static void virtio_gpu_get_capsets(struct virtio_gpu_device *vgdev,
>
> int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
> {
> - static vq_callback_t *callbacks[] = {
> - virtio_gpu_ctrl_ack, virtio_gpu_cursor_ack
> + struct virtio_queue_info vqs_info[] = {
> + { "control", virtio_gpu_ctrl_ack },
> + { "cursor", virtio_gpu_cursor_ack },
> };
I'd maybe do struct virtio_queue_info vqs_info[2]
so it's clear array is the right size.
Not a new issue so can be a separate cleanup.
> - static const char * const names[] = { "control", "cursor" };
> -
> struct virtio_gpu_device *vgdev;
> /* this will expand later */
> struct virtqueue *vqs[2];
> @@ -207,7 +206,7 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
> DRM_INFO("features: %ccontext_init\n",
> vgdev->has_context_init ? '+' : '-');
>
> - ret = virtio_find_vqs(vgdev->vdev, 2, vqs, callbacks, names, NULL);
> + ret = virtio_find_vqs_info(vgdev->vdev, 2, vqs, vqs_info, NULL);
> if (ret) {
> DRM_ERROR("failed to find virt queues\n");
> goto err_vqs;
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index 9ed8958a42bf..ef77c1efc767 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
> @@ -1094,14 +1094,13 @@ static struct iommu_ops viommu_ops = {
> static int viommu_init_vqs(struct viommu_dev *viommu)
> {
> struct virtio_device *vdev = dev_to_virtio(viommu->dev);
> - const char *names[] = { "request", "event" };
> - vq_callback_t *callbacks[] = {
> - NULL, /* No async requests */
> - viommu_event_handler,
> + struct virtio_queue_info vqs_info[] = {
I'd maybe do struct virtio_queue_info vqs_info[VIOMMU_NR_VQS]
so it's clear array is the right size.
Not a new issue so can be a separate cleanup.
> + { "request" },
Let's keep the comment here:
{ "request", NULL /* No async requests */ },
> + { "event", viommu_event_handler },
> };
>
> - return virtio_find_vqs(vdev, VIOMMU_NR_VQS, viommu->vqs, callbacks,
> - names, NULL);
> + return virtio_find_vqs_info(vdev, VIOMMU_NR_VQS, viommu->vqs,
> + vqs_info, NULL);
> }
>
> static int viommu_fill_evtq(struct viommu_dev *viommu)
> diff --git a/drivers/net/wireless/virtual/mac80211_hwsim.c b/drivers/net/wireless/virtual/mac80211_hwsim.c
> index 20fa21bb4d1c..565c091f1ba5 100644
> --- a/drivers/net/wireless/virtual/mac80211_hwsim.c
> +++ b/drivers/net/wireless/virtual/mac80211_hwsim.c
> @@ -6558,17 +6558,13 @@ static void hwsim_virtio_rx_done(struct virtqueue *vq)
>
> static int init_vqs(struct virtio_device *vdev)
> {
> - vq_callback_t *callbacks[HWSIM_NUM_VQS] = {
> - [HWSIM_VQ_TX] = hwsim_virtio_tx_done,
> - [HWSIM_VQ_RX] = hwsim_virtio_rx_done,
> - };
> - const char *names[HWSIM_NUM_VQS] = {
> - [HWSIM_VQ_TX] = "tx",
> - [HWSIM_VQ_RX] = "rx",
> + struct virtio_queue_info vqs_info[HWSIM_NUM_VQS] = {
> + [HWSIM_VQ_TX] = { "tx", hwsim_virtio_tx_done },
> + [HWSIM_VQ_RX] = { "rx", hwsim_virtio_rx_done },
> };
>
> - return virtio_find_vqs(vdev, HWSIM_NUM_VQS,
> - hwsim_vqs, callbacks, names, NULL);
> + return virtio_find_vqs_info(vdev, HWSIM_NUM_VQS,
> + hwsim_vqs, vqs_info, NULL);
> }
>
> static int fill_vq(struct virtqueue *vq)
> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
> index e9e8c1f7829f..440f1cc9157d 100644
> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> @@ -868,8 +868,10 @@ static void rpmsg_virtio_del_ctrl_dev(struct rpmsg_device *rpdev_ctrl)
>
> static int rpmsg_probe(struct virtio_device *vdev)
> {
> - vq_callback_t *vq_cbs[] = { rpmsg_recv_done, rpmsg_xmit_done };
> - static const char * const names[] = { "input", "output" };
> + struct virtio_queue_info vqs_info[] = {
I'd maybe do struct virtio_queue_info vqs_info[2]
so it's clear array is the right size.
Not a new issue so can be a separate cleanup.
> + { "input", rpmsg_recv_done },
> + { "output", rpmsg_xmit_done },
> + };
> struct virtqueue *vqs[2];
> struct virtproc_info *vrp;
> struct virtio_rpmsg_channel *vch = NULL;
> @@ -891,7 +893,7 @@ static int rpmsg_probe(struct virtio_device *vdev)
> init_waitqueue_head(&vrp->sendq);
>
> /* We expect two virtqueues, rx and tx (and in this order) */
> - err = virtio_find_vqs(vdev, 2, vqs, vq_cbs, names, NULL);
> + err = virtio_find_vqs_info(vdev, 2, vqs, vqs_info, NULL);
> if (err)
> goto free_vrp;
>
> diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c
> index 1a730d6c0b55..e1491ad9eced 100644
> --- a/drivers/virtio/virtio_input.c
> +++ b/drivers/virtio/virtio_input.c
> @@ -185,13 +185,14 @@ static void virtinput_cfg_abs(struct virtio_input *vi, int abs)
>
> static int virtinput_init_vqs(struct virtio_input *vi)
> {
> + struct virtio_queue_info vqs_info[] = {
> + { "events", virtinput_recv_events },
> + { "status", virtinput_recv_status },
> + };
> struct virtqueue *vqs[2];
> - vq_callback_t *cbs[] = { virtinput_recv_events,
> - virtinput_recv_status };
> - static const char * const names[] = { "events", "status" };
> int err;
>
> - err = virtio_find_vqs(vi->vdev, 2, vqs, cbs, names, NULL);
> + err = virtio_find_vqs_info(vi->vdev, 2, vqs, vqs_info, NULL);
ARRAY_SIZE(vqs_info) is now possible instead of 2.
Can be a separate cleanup, though.
> if (err)
> return err;
> vi->evt = vqs[0];
> diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> index 43d405298857..d303ef4d9898 100644
> --- a/net/vmw_vsock/virtio_transport.c
> +++ b/net/vmw_vsock/virtio_transport.c
> @@ -617,20 +617,15 @@ static void virtio_transport_rx_work(struct work_struct *work)
> static int virtio_vsock_vqs_init(struct virtio_vsock *vsock)
> {
> struct virtio_device *vdev = vsock->vdev;
> - static const char * const names[] = {
> - "rx",
> - "tx",
> - "event",
> - };
> - vq_callback_t *callbacks[] = {
> - virtio_vsock_rx_done,
> - virtio_vsock_tx_done,
> - virtio_vsock_event_done,
> + struct virtio_queue_info vqs_info[] = {
> + { "rx", virtio_vsock_rx_done },
> + { "tx", virtio_vsock_tx_done },
> + { "event", virtio_vsock_event_done },
> };
> int ret;
>
> - ret = virtio_find_vqs(vdev, VSOCK_VQ_MAX, vsock->vqs, callbacks, names,
> - NULL);
> + ret = virtio_find_vqs_info(vdev, VSOCK_VQ_MAX, vsock->vqs, vqs_info,
> + NULL);
> if (ret < 0)
> return ret;
>
> diff --git a/sound/virtio/virtio_card.c b/sound/virtio/virtio_card.c
> index 7805daea0102..a6b0da6790dc 100644
> --- a/sound/virtio/virtio_card.c
> +++ b/sound/virtio/virtio_card.c
> @@ -110,25 +110,22 @@ static void virtsnd_event_notify_cb(struct virtqueue *vqueue)
> static int virtsnd_find_vqs(struct virtio_snd *snd)
> {
> struct virtio_device *vdev = snd->vdev;
> - static vq_callback_t *callbacks[VIRTIO_SND_VQ_MAX] = {
> - [VIRTIO_SND_VQ_CONTROL] = virtsnd_ctl_notify_cb,
> - [VIRTIO_SND_VQ_EVENT] = virtsnd_event_notify_cb,
> - [VIRTIO_SND_VQ_TX] = virtsnd_pcm_tx_notify_cb,
> - [VIRTIO_SND_VQ_RX] = virtsnd_pcm_rx_notify_cb
> - };
> - static const char *names[VIRTIO_SND_VQ_MAX] = {
> - [VIRTIO_SND_VQ_CONTROL] = "virtsnd-ctl",
> - [VIRTIO_SND_VQ_EVENT] = "virtsnd-event",
> - [VIRTIO_SND_VQ_TX] = "virtsnd-tx",
> - [VIRTIO_SND_VQ_RX] = "virtsnd-rx"
> + struct virtio_queue_info vqs_info[] = {
Why not
struct virtio_queue_info vqs_info[VIRTIO_SND_VQ_MAX] ?
otherwise it's not clear all arrays are same size.
> + [VIRTIO_SND_VQ_CONTROL] = { "virtsnd-ctl",
> + virtsnd_ctl_notify_cb },
> + [VIRTIO_SND_VQ_EVENT] = { "virtsnd-event",
> + virtsnd_event_notify_cb },
> + [VIRTIO_SND_VQ_TX] = { "virtsnd-tx",
> + virtsnd_pcm_tx_notify_cb },
> + [VIRTIO_SND_VQ_RX] = { "virtsnd-rx",
> + virtsnd_pcm_rx_notify_cb },
> };
> struct virtqueue *vqs[VIRTIO_SND_VQ_MAX] = { 0 };
> unsigned int i;
> unsigned int n;
> int rc;
>
> - rc = virtio_find_vqs(vdev, VIRTIO_SND_VQ_MAX, vqs, callbacks, names,
> - NULL);
> + rc = virtio_find_vqs_info(vdev, VIRTIO_SND_VQ_MAX, vqs, vqs_info, NULL);
> if (rc) {
> dev_err(&vdev->dev, "failed to initialize virtqueues\n");
> return rc;
> --
> 2.45.2
next prev parent reply other threads:[~2024-07-03 13:12 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-03 12:38 [PATCH virtio 00/19] virtio: consolidate vq info args of find_vqs() Jiri Pirko
2024-07-03 12:38 ` [PATCH virtio 01/19] caif_virtio: use virtio_find_single_vq() for single virtqueue finding Jiri Pirko
2024-07-03 12:38 ` [PATCH virtio 02/19] virtio: make virtio_find_vqs() call virtio_find_vqs_ctx() Jiri Pirko
2024-07-03 12:38 ` [PATCH virtio 03/19] virtio: make virtio_find_single_vq() call virtio_find_vqs() Jiri Pirko
2024-07-03 12:38 ` [PATCH virtio 04/19] virtio: introduce virtio_queue_info struct and find_vqs_info() config op Jiri Pirko
2024-07-03 13:02 ` Michael S. Tsirkin
2024-07-03 13:23 ` Jiri Pirko
2024-07-03 12:38 ` [PATCH virtio 05/19] virtio_pci: convert vp_*find_vqs() ops to find_vqs_info() Jiri Pirko
2024-07-03 12:39 ` [PATCH virtio 06/19] virtio: convert find_vqs() op implementations " Jiri Pirko
2024-07-03 12:39 ` [PATCH virtio 07/19] virtio: call virtio_find_vqs_info() from virtio_find_single_vq() directly Jiri Pirko
2024-07-03 12:39 ` [PATCH virtio 08/19] virtio: remove the original find_vqs() op Jiri Pirko
2024-07-03 12:39 ` [PATCH virtio 09/19] virtio: rename find_vqs_info() op to find_vqs() Jiri Pirko
2024-07-03 12:39 ` [PATCH virtio 10/19] virtio_blk: convert to use virtio_find_vqs_info() Jiri Pirko
2024-07-03 12:39 ` [PATCH virtio 11/19] virtio_console: " Jiri Pirko
2024-07-03 12:39 ` [PATCH virtio 12/19] virtio_crypto: " Jiri Pirko
2024-07-03 12:39 ` [PATCH virtio 13/19] virtio_net: " Jiri Pirko
2024-07-03 12:39 ` [PATCH virtio 14/19] scsi: virtio_scsi: " Jiri Pirko
2024-07-03 12:39 ` [PATCH virtio 15/19] virtiofs: " Jiri Pirko
2024-07-03 12:39 ` [PATCH virtio 16/19] virtio_balloon: " Jiri Pirko
2024-07-03 12:39 ` [PATCH virtio 17/19] virtio: convert the rest virtio_find_vqs() users to virtio_find_vqs_info() Jiri Pirko
2024-07-03 13:11 ` Michael S. Tsirkin [this message]
2024-07-03 13:22 ` Jiri Pirko
2024-07-03 12:39 ` [PATCH virtio 18/19] virtio: remove unused virtio_find_vqs() and virtio_find_vqs_ctx() helpers Jiri Pirko
2024-07-03 12:39 ` [PATCH virtio 19/19] virtio: rename virtio_find_vqs_info() to virtio_find_vqs() Jiri Pirko
2024-07-03 12:50 ` [PATCH virtio 00/19] virtio: consolidate vq info args of find_vqs() Michael S. Tsirkin
2024-07-03 13:24 ` Jiri Pirko
2024-07-03 13:37 ` Michael S. Tsirkin
2024-07-04 11:38 ` Xuan Zhuo
2024-07-04 12:39 ` Michael S. Tsirkin
2024-07-04 12:55 ` Jiri Pirko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240703090519-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=eperezma@redhat.com \
--cc=feliu@nvidia.com \
--cc=hengqi@linux.alibaba.com \
--cc=jasowang@redhat.com \
--cc=jiri@resnulli.us \
--cc=parav@nvidia.com \
--cc=virtualization@lists.linux.dev \
--cc=xuanzhuo@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.