On Wed, Jul 19, 2023 at 9:54 AM Hawkins Jiawei <yin31...@gmail.com> wrote:
>
> This patch moves vhost_svq_poll() to the caller of
> vhost_vdpa_net_cvq_add() and introduces a helper funtion.
>
> By making this change, next patches in this series is
> able to refactor vhost_vdpa_net_load_x() only to delay
> the polling and checking process until either the SVQ
> is full or control commands shadow buffers are full.
>
> Signed-off-by: Hawkins Jiawei <yin31...@gmail.com>
> ---
> net/vhost-vdpa.c | 50 ++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 40 insertions(+), 10 deletions(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index fe0ba19724..d06f38403f 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -609,15 +609,21 @@ static ssize_t vhost_vdpa_net_cvq_add(VhostVDPAState *s,
> qemu_log_mask(LOG_GUEST_ERROR, "%s: No space on device queue\n",
> __func__);
> }
> - return r;
> }
>
> - /*
> - * We can poll here since we've had BQL from the time we sent the
> - * descriptor. Also, we need to take the answer before SVQ pulls by
> itself,
> - * when BQL is released
> - */
> - return vhost_svq_poll(svq, 1);
> + return r;
> +}
> +
> +/*
> + * Convenience wrapper to poll SVQ for multiple control commands.
> + *
> + * Caller should hold the BQL when invoking this function, and should take
> + * the answer before SVQ pulls by itself when BQL is released.
> + */
> +static ssize_t vhost_vdpa_net_svq_poll(VhostVDPAState *s, size_t
> cmds_in_flight)
> +{
> + VhostShadowVirtqueue *svq = g_ptr_array_index(s->vhost_vdpa.shadow_vqs,
> 0);
> + return vhost_svq_poll(svq, cmds_in_flight);
> }
>
> /* Convenience wrapper to get number of available SVQ descriptors */
> @@ -645,6 +651,7 @@ static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState *s,
> uint8_t class,
> .iov_base = s->status,
> .iov_len = sizeof(*s->status),
> };
> + ssize_t r;
>
> assert(data_size < vhost_vdpa_net_cvq_cmd_page_len() - sizeof(ctrl));
> /* Each CVQ command has one out descriptor and one in descriptor */
> @@ -657,7 +664,16 @@ static ssize_t vhost_vdpa_net_load_cmd(VhostVDPAState
> *s, uint8_t class,
> iov_to_buf(data_sg, data_num, 0,
> s->cvq_cmd_out_buffer + sizeof(ctrl), data_size);
>
> - return vhost_vdpa_net_cvq_add(s, &out, 1, &in, 1);
> + r = vhost_vdpa_net_cvq_add(s, &out, 1, &in, 1);
> + if (unlikely(r < 0)) {
> + return r;
> + }
> +
> + /*
> + * We can poll here since we've had BQL from the time
> + * we sent the descriptor.
> + */
> + return vhost_vdpa_net_svq_poll(s, 1);
> }
>
> static int vhost_vdpa_net_load_mac(VhostVDPAState *s, const VirtIONet *n)
> @@ -1152,6 +1168,12 @@ static int
> vhost_vdpa_net_excessive_mac_filter_cvq_add(VhostVDPAState *s,
> if (unlikely(r < 0)) {
> return r;
> }
> +
> + /*
> + * We can poll here since we've had BQL from the time
> + * we sent the descriptor.
> + */
> + vhost_vdpa_net_svq_poll(s, 1);
Don't we need to check the return value of vhost_vdpa_net_svq_poll here?
> if (*s->status != VIRTIO_NET_OK) {
> return sizeof(*s->status);
> }
> @@ -1266,10 +1288,18 @@ static int
> vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
> goto out;
> }
> } else {
> - dev_written = vhost_vdpa_net_cvq_add(s, &out, 1, &in, 1);
> - if (unlikely(dev_written < 0)) {
> + ssize_t r;
> + r = vhost_vdpa_net_cvq_add(s, &out, 1, &in, 1);
> + if (unlikely(r < 0)) {
> + dev_written = r;
> goto out;
> }
> +
> + /*
> + * We can poll here since we've had BQL from the time
> + * we sent the descriptor.
> + */
> + dev_written = vhost_vdpa_net_svq_poll(s, 1);
> }
>
> if (unlikely(dev_written < sizeof(status))) {
> --
> 2.25.1
>
