Hi Tanmay (Apologies for the late review)
On Wed, Apr 29, 2026 at 09:10:51AM -0700, Tanmay Shah wrote: > Current design allocates memory for tx and rx buffers equally. The > throughput can be increased if the user is allowed to configure number > of tx and rx buffers as required. Hence, do not split number of tx & rx > buffers into half, but decide based on respective vring size. > > Signed-off-by: Tanmay Shah <[email protected]> > --- > > Test performed: > - Test this patch with existing firmware as it is, rpmsg working. > > Changes in v2: > - Change author > - fix commit message with better explanation > - %s/sbuf/tx_buf > - %s/rbuf/rx_buf > - %s/num_rbuf/num_rx_buf/ > - %s/num_sbuf/num_tx_buf/ Please split this patch in two parts - one to do the refactoring of the tx/rx_buf and another one for the varying size. > > drivers/rpmsg/virtio_rpmsg_bus.c | 68 ++++++++++++++++---------------- > 1 file changed, 34 insertions(+), 34 deletions(-) > > diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c > b/drivers/rpmsg/virtio_rpmsg_bus.c > index 5ae15111fb4f..e59d8cf9b975 100644 > --- a/drivers/rpmsg/virtio_rpmsg_bus.c > +++ b/drivers/rpmsg/virtio_rpmsg_bus.c > @@ -35,13 +35,14 @@ > * @vdev: the virtio device > * @rvq: rx virtqueue > * @svq: tx virtqueue > - * @rbufs: kernel address of rx buffers > - * @sbufs: kernel address of tx buffers > - * @num_bufs: total number of buffers for rx and tx > - * @buf_size: size of one rx or tx buffer > + * @rx_bufs: kernel address of rx buffers > + * @tx_bufs: kernel address of tx buffers > + * @num_rx_buf: total number of buffers for rx > + * @num_tx_buf: total number of buffers for tx > + * @buf_size: size of one rx or tx buffer > * @last_sbuf: index of last tx buffer used > * @bufs_dma: dma base addr of the buffers > - * @tx_lock: protects svq and sbufs, to allow concurrent senders. > + * @tx_lock: protects svq and tx_bufs, to allow concurrent senders. > * sending a message might require waking up a dozing remote > * processor, which involves sleeping, hence the mutex. > * @endpoints: idr of local endpoints, allows fast retrieval > @@ -55,8 +56,9 @@ > struct virtproc_info { > struct virtio_device *vdev; > struct virtqueue *rvq, *svq; > - void *rbufs, *sbufs; > - unsigned int num_bufs; > + void *rx_bufs, *tx_bufs; > + unsigned int num_rx_buf; > + unsigned int num_tx_buf; > unsigned int buf_size; > int last_sbuf; > dma_addr_t bufs_dma; > @@ -110,7 +112,7 @@ struct virtio_rpmsg_channel { > /* > * We're allocating buffers of 512 bytes each for communications. The > * number of buffers will be computed from the number of buffers supported > - * by the vring, upto a maximum of 512 buffers (256 in each direction). > + * by the vring, up to a maximum of 256 in each direction. > * > * Each buffer will have 16 bytes for the msg header and 496 bytes for > * the payload. > @@ -125,7 +127,7 @@ struct virtio_rpmsg_channel { > * can change this without changing anything in the firmware of the remote > * processor. > */ > -#define MAX_RPMSG_NUM_BUFS (512) > +#define MAX_RPMSG_NUM_BUFS (256) > #define MAX_RPMSG_BUF_SIZE (512) > > /* > @@ -440,12 +442,9 @@ static void *get_a_tx_buf(struct virtproc_info *vrp) > > mutex_lock(&vrp->tx_lock); > > - /* > - * either pick the next unused tx buffer > - * (half of our buffers are used for sending messages) > - */ > - if (vrp->last_sbuf < vrp->num_bufs / 2) > - ret = vrp->sbufs + vrp->buf_size * vrp->last_sbuf++; > + /* either pick the next unused tx buffer */ > + if (vrp->last_sbuf < vrp->num_tx_buf) > + ret = vrp->tx_bufs + vrp->buf_size * vrp->last_sbuf++; > /* or recycle a used one */ > else > ret = virtqueue_get_buf(vrp->svq, &len); > @@ -631,11 +630,10 @@ static __poll_t virtio_rpmsg_poll(struct rpmsg_endpoint > *ept, struct file *filp, > > /* > * check for a free buffer, either: > - * - we haven't used all of the available transmit buffers (half of the > - * allocated buffers are used for transmit, hence num_bufs / 2), or, > + * - we haven't used all of the available transmit buffers or, > * - we ask the virtqueue if there's a buffer available > */ > - if (vrp->last_sbuf < vrp->num_bufs / 2 || > + if (vrp->last_sbuf < vrp->num_tx_buf || > !virtqueue_enable_cb(vrp->svq)) > mask |= EPOLLOUT; > > @@ -846,19 +844,20 @@ static int rpmsg_probe(struct virtio_device *vdev) > vrp->rvq = vqs[0]; > vrp->svq = vqs[1]; > > - /* we expect symmetric tx/rx vrings */ > - WARN_ON(virtqueue_get_vring_size(vrp->rvq) != > - virtqueue_get_vring_size(vrp->svq)); > - > /* we need less buffers if vrings are small */ > - if (virtqueue_get_vring_size(vrp->rvq) < MAX_RPMSG_NUM_BUFS / 2) > - vrp->num_bufs = virtqueue_get_vring_size(vrp->rvq) * 2; > + if (virtqueue_get_vring_size(vrp->rvq) < MAX_RPMSG_NUM_BUFS) > + vrp->num_rx_buf = virtqueue_get_vring_size(vrp->rvq); > + else > + vrp->num_rx_buf = MAX_RPMSG_NUM_BUFS; > + > + if (virtqueue_get_vring_size(vrp->svq) < MAX_RPMSG_NUM_BUFS) > + vrp->num_tx_buf = virtqueue_get_vring_size(vrp->svq); > else > - vrp->num_bufs = MAX_RPMSG_NUM_BUFS; > + vrp->num_tx_buf = MAX_RPMSG_NUM_BUFS; > > vrp->buf_size = MAX_RPMSG_BUF_SIZE; > > - total_buf_space = vrp->num_bufs * vrp->buf_size; > + total_buf_space = (vrp->num_rx_buf + vrp->num_tx_buf) * vrp->buf_size; > > /* allocate coherent memory for the buffers */ > bufs_va = dma_alloc_coherent(vdev->dev.parent, > @@ -872,16 +871,16 @@ static int rpmsg_probe(struct virtio_device *vdev) > dev_dbg(&vdev->dev, "buffers: va %p, dma %pad\n", > bufs_va, &vrp->bufs_dma); > > - /* half of the buffers is dedicated for RX */ > - vrp->rbufs = bufs_va; > + /* first part of the buffers is dedicated for RX */ > + vrp->rx_bufs = bufs_va; > > - /* and half is dedicated for TX */ > - vrp->sbufs = bufs_va + total_buf_space / 2; > + /* and second part is dedicated for TX */ > + vrp->tx_bufs = bufs_va + vrp->num_rx_buf * vrp->buf_size; > > /* set up the receive buffers */ > - for (i = 0; i < vrp->num_bufs / 2; i++) { > + for (i = 0; i < vrp->num_rx_buf; i++) { > struct scatterlist sg; > - void *cpu_addr = vrp->rbufs + i * vrp->buf_size; > + void *cpu_addr = vrp->rx_bufs + i * vrp->buf_size; > > rpmsg_sg_init(&sg, cpu_addr, vrp->buf_size); > > @@ -966,7 +965,8 @@ static int rpmsg_remove_device(struct device *dev, void > *data) > static void rpmsg_remove(struct virtio_device *vdev) > { > struct virtproc_info *vrp = vdev->priv; > - size_t total_buf_space = vrp->num_bufs * vrp->buf_size; > + unsigned int num_bufs = vrp->num_rx_buf + vrp->num_tx_buf; > + size_t total_buf_space = num_bufs * vrp->buf_size; > int ret; > > virtio_reset_device(vdev); > @@ -980,7 +980,7 @@ static void rpmsg_remove(struct virtio_device *vdev) > vdev->config->del_vqs(vrp->vdev); > > dma_free_coherent(vdev->dev.parent, total_buf_space, > - vrp->rbufs, vrp->bufs_dma); > + vrp->rx_bufs, vrp->bufs_dma); > > kfree(vrp); > } > -- > 2.34.1 >
