[PATCH v9 1/6] rpmsg: Process all available messages in virtqueue callback

[Tivy, Robert]

Just one small fix needed (see below) and it's good-to-go.

> -----Original Message-----
> From: Ohad Ben-Cohen [mailto:ohad at wizery.com]
> Sent: Tuesday, April 09, 2013 1:26 AM
> 
> On Tue, Apr 9, 2013 at 1:56 AM, Tivy, Robert <rtivy at ti.com> wrote:
> > Shouldn't the virtqueue_kick() be called only when we successfully
> added a buffer with virtqueue_add_buf()?
> 
> Definitely, thanks for noticing!
> 
> Take 2:
> 
> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c
> b/drivers/rpmsg/virtio_rpmsg_bus.c
> index a59684b..4ade672 100644
> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> @@ -776,22 +776,11 @@ out:
>  }
>  EXPORT_SYMBOL(rpmsg_send_offchannel_raw);
> 
> -/* called when an rx buffer is used, and it's time to digest a message
> */
> -static void rpmsg_recv_done(struct virtqueue *rvq)
> +static int rpmsg_recv_single(struct virtproc_info *vrp, struct device
> *dev,
> +                            struct rpmsg_hdr *msg, unsigned int len)
>  {
> -       struct rpmsg_hdr *msg;
> -       unsigned int len;
>         struct rpmsg_endpoint *ept;
>         struct scatterlist sg;
> -       struct virtproc_info *vrp = rvq->vdev->priv;
> -       struct device *dev = &rvq->vdev->dev;
> -       int err;

This new function also uses an 'int err;', so the above line should not be removed.

> -
> -       msg = virtqueue_get_buf(rvq, &len);
> -       if (!msg) {
> -               dev_err(dev, "uhm, incoming signal, but no used buffer
> ?\n");
> -               return;
> -       }
> 
>         dev_dbg(dev, "From: 0x%x, To: 0x%x, Len: %d, Flags: %d,
> Reserved: %d\n",
>                                         msg->src, msg->dst, msg->len,
> @@ -806,7 +795,7 @@ static void rpmsg_recv_done(struct virtqueue *rvq)
>         if (len > RPMSG_BUF_SIZE ||
>                 msg->len > (len - sizeof(struct rpmsg_hdr))) {
>                 dev_warn(dev, "inbound msg too big: (%d, %d)\n", len,
> msg->len);
> -               return;
> +               return -EINVAL;
>         }
> 
>         /* use the dst addr to fetch the callback of the appropriate
> user */
> @@ -842,11 +831,42 @@ static void rpmsg_recv_done(struct virtqueue
> *rvq)
>         err = virtqueue_add_buf(vrp->rvq, &sg, 0, 1, msg, GFP_KERNEL);
>         if (err < 0) {
>                 dev_err(dev, "failed to add a virtqueue buffer: %d\n",
> err);
> +               return err;
> +       }
> +
> +       return 0;
> +}
> +
> +/* called when an rx buffer is used, and it's time to digest a message
> */
> +static void rpmsg_recv_done(struct virtqueue *rvq)
> +{
> +       struct virtproc_info *vrp = rvq->vdev->priv;
> +       struct device *dev = &rvq->vdev->dev;
> +       struct rpmsg_hdr *msg;
> +       unsigned int len, msgs_received = 0;
> +       int err;
> +
> +       msg = virtqueue_get_buf(rvq, &len);
> +       if (!msg) {
> +               dev_err(dev, "uhm, incoming signal, but no used buffer
> ?\n");
>                 return;
>         }
> 
> +       while (msg) {
> +               err = rpmsg_recv_single(vrp, dev, msg, len);
> +               if (err)
> +                       break;
> +
> +               msgs_received++;
> +
> +               msg = virtqueue_get_buf(rvq, &len);
> +       };
> +
> +       dev_dbg(dev, "Received %u messages\n", msgs_received);
> +
>         /* tell the remote processor we added another available rx
> buffer */
> -       virtqueue_kick(vrp->rvq);
> +       if (msgs_received)
> +               virtqueue_kick(vrp->rvq);
>  }
> 
>  /*
> 
> > Thanks for the rewrite, looks better.  I'll give it a try and let you
> know how it goes.
> 
> Thanks!
> Ohad.

I added the above-mentioned 'int err;' to my tree's rpmsg_recv_single() and tested it out, it worked well.  So once that is corrected, you can add:
Acked-by: Robert Tivy <rtivy at ti.com>

Regards,

- Rob