Re: [PATCHv2] rpmsg: compute number of buffers to allocate from vrings
On Thu, Nov 27, 2014 at 12:30 AM, Suman Anna s-a...@ti.com wrote: Yep, I have reviewed and verified the changes, it is good to go. Applied, thanks! -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] rpmsg: compute number of buffers to allocate from vrings
Hi Suman,
On Thu, Nov 13, 2014 at 7:46 PM, Suman Anna s-a...@ti.com wrote:
Hi Ohad,
On 09/16/2014 01:33 PM, Suman Anna wrote:
The buffers to be used for communication are allocated during the
rpmsg virtio driver's probe, and the total number of buffers is
currently hard-coded to 512. The vring configuration can vary from
one platform to another or between different remote processors. The
setup of the receive buffers will throw a WARN_ON if the associated
vrings are configured with less than 256 buffers (in each direction).
So, adjust this hard-coded value to rely on the number of buffers the
virtqueue vring is setup with, but also limit to use 256 buffers at
most in each direction to avoid wacky resource tables consuming up
unreasonable memory.
NOTE: The number of buffers is already assumed to be symmetrical
in each direction, and that logic is not unchanged.
Signed-off-by: Suman Anna s-a...@ti.com
---
v2 changes:
- add upper limit on buffers and update comments
- revise patch description
Any comments on this one, if not can you pick this up for 3.19?
Did some small changes - untested, not even compiled - can you please
make sure it works for you?
Thanks,
Ohad.
From b1b9891441fa33fd0d49b5cb3aa7f04bca1cc1db Mon Sep 17 00:00:00 2001
From: Suman Anna s-a...@ti.com
Date: Tue, 16 Sep 2014 13:33:07 -0500
Subject: [PATCH] rpmsg: use less buffers when vrings are small
Adjust the number of rpmsg buffers to rely on the size of the
vring, instead of using the hard coded value of 512 (256 per
direction).
This is needed when small vrings are being used, where 256
buffers are too much to fit in a vring.
While considering the vring size, keep using the 512 hard coded
value as an upper limit to avoid wacky resource tables consuming
unreasonable amount of memory.
NOTE: The number of buffers is already assumed to be symmetrical
in each direction, and that logic is unchanged.
Signed-off-by: Suman Anna s-a...@ti.com
[edit commit message, small code and comment simplification]
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
drivers/rpmsg/virtio_rpmsg_bus.c | 44 +++-
1 file changed, 30 insertions(+), 14 deletions(-)
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index b6135d4..92f6af6 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -41,6 +41,7 @@
* @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
* @last_sbuf: index of last tx buffer used
* @bufs_dma: dma base addr of the buffers
* @tx_lock: protects svq, sbufs and sleepers, to allow concurrent senders.
@@ -60,6 +61,7 @@ struct virtproc_info {
struct virtio_device *vdev;
struct virtqueue *rvq, *svq;
void *rbufs, *sbufs;
+ unsigned int num_bufs;
int last_sbuf;
dma_addr_t bufs_dma;
struct mutex tx_lock;
@@ -86,13 +88,14 @@ struct rpmsg_channel_info {
#define to_rpmsg_driver(d) container_of(d, struct rpmsg_driver, drv)
/*
- * We're allocating 512 buffers of 512 bytes for communications, and then
- * using the first 256 buffers for RX, and the last 256 buffers for TX.
+ * 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).
*
* Each buffer will have 16 bytes for the msg header and 496 bytes for
* the payload.
*
- * This will require a total space of 256KB for the buffers.
+ * This will utilize a maximum total space of 256KB for the buffers.
*
* We might also want to add support for user-provided buffers in time.
* This will allow bigger buffer size flexibility, and can also be used
@@ -102,9 +105,8 @@ struct rpmsg_channel_info {
* can change this without changing anything in the firmware of the remote
* processor.
*/
-#define RPMSG_NUM_BUFS (512)
+#define MAX_RPMSG_NUM_BUFS (512)
#define RPMSG_BUF_SIZE (512)
-#define RPMSG_TOTAL_BUF_SPACE (RPMSG_NUM_BUFS * RPMSG_BUF_SIZE)
/*
* Local addresses are dynamically allocated on-demand.
@@ -579,7 +581,7 @@ static void *get_a_tx_buf(struct virtproc_info *vrp)
* either pick the next unused tx buffer
* (half of our buffers are used for sending messages)
*/
- if (vrp-last_sbuf RPMSG_NUM_BUFS / 2)
+ if (vrp-last_sbuf vrp-num_bufs / 2)
ret = vrp-sbufs + RPMSG_BUF_SIZE * vrp-last_sbuf++;
/* or recycle a used one */
else
@@ -948,6 +950,7 @@ static int rpmsg_probe(struct virtio_device *vdev)
struct virtproc_info *vrp;
void *bufs_va;
int err = 0, i;
+ size_t total_buf_space;
vrp = kzalloc(sizeof(*vrp), GFP_KERNEL);
if (!vrp)
@@ -968,10 +971,22 @@ 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) !=
+
Re: [PATCHv2] rpmsg: compute number of buffers to allocate from vrings
Hi Ohad, On Thu, Nov 13, 2014 at 7:46 PM, Suman Anna s-a...@ti.com wrote: Hi Ohad, On 09/16/2014 01:33 PM, Suman Anna wrote: The buffers to be used for communication are allocated during the rpmsg virtio driver's probe, and the total number of buffers is currently hard-coded to 512. The vring configuration can vary from one platform to another or between different remote processors. The setup of the receive buffers will throw a WARN_ON if the associated vrings are configured with less than 256 buffers (in each direction). So, adjust this hard-coded value to rely on the number of buffers the virtqueue vring is setup with, but also limit to use 256 buffers at most in each direction to avoid wacky resource tables consuming up unreasonable memory. NOTE: The number of buffers is already assumed to be symmetrical in each direction, and that logic is not unchanged. Signed-off-by: Suman Anna s-a...@ti.com --- v2 changes: - add upper limit on buffers and update comments - revise patch description Any comments on this one, if not can you pick this up for 3.19? Did some small changes - untested, not even compiled - can you please make sure it works for you? Yep, I have reviewed and verified the changes, it is good to go. Thanks, Suman -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] rpmsg: compute number of buffers to allocate from vrings
Hi Ohad, On 09/16/2014 01:33 PM, Suman Anna wrote: The buffers to be used for communication are allocated during the rpmsg virtio driver's probe, and the total number of buffers is currently hard-coded to 512. The vring configuration can vary from one platform to another or between different remote processors. The setup of the receive buffers will throw a WARN_ON if the associated vrings are configured with less than 256 buffers (in each direction). So, adjust this hard-coded value to rely on the number of buffers the virtqueue vring is setup with, but also limit to use 256 buffers at most in each direction to avoid wacky resource tables consuming up unreasonable memory. NOTE: The number of buffers is already assumed to be symmetrical in each direction, and that logic is not unchanged. Signed-off-by: Suman Anna s-a...@ti.com --- v2 changes: - add upper limit on buffers and update comments - revise patch description Any comments on this one, if not can you pick this up for 3.19? regards Suman -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] rpmsg: compute number of buffers to allocate from vrings
The buffers to be used for communication are allocated during the
rpmsg virtio driver's probe, and the total number of buffers is
currently hard-coded to 512. The vring configuration can vary from
one platform to another or between different remote processors. The
setup of the receive buffers will throw a WARN_ON if the associated
vrings are configured with less than 256 buffers (in each direction).
So, adjust this hard-coded value to rely on the number of buffers the
virtqueue vring is setup with, but also limit to use 256 buffers at
most in each direction to avoid wacky resource tables consuming up
unreasonable memory.
NOTE: The number of buffers is already assumed to be symmetrical
in each direction, and that logic is not unchanged.
Signed-off-by: Suman Anna s-a...@ti.com
---
v2 changes:
- add upper limit on buffers and update comments
- revise patch description
drivers/rpmsg/virtio_rpmsg_bus.c | 46
1 file changed, 32 insertions(+), 14 deletions(-)
diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index b6135d4..9ea6c43 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -41,6 +41,7 @@
* @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
* @last_sbuf: index of last tx buffer used
* @bufs_dma: dma base addr of the buffers
* @tx_lock: protects svq, sbufs and sleepers, to allow concurrent senders.
@@ -60,6 +61,7 @@ struct virtproc_info {
struct virtio_device *vdev;
struct virtqueue *rvq, *svq;
void *rbufs, *sbufs;
+ unsigned int num_bufs;
int last_sbuf;
dma_addr_t bufs_dma;
struct mutex tx_lock;
@@ -86,13 +88,14 @@ struct rpmsg_channel_info {
#define to_rpmsg_driver(d) container_of(d, struct rpmsg_driver, drv)
/*
- * We're allocating 512 buffers of 512 bytes for communications, and then
- * using the first 256 buffers for RX, and the last 256 buffers for TX.
+ * We're allocating buffers of 512 bytes each for communications. The
+ * number of buffers are computed from the number of buffers supported by
+ * the virtqueue vring (upto a maximum of 256 buffers in each direction).
*
* Each buffer will have 16 bytes for the msg header and 496 bytes for
* the payload.
*
- * This will require a total space of 256KB for the buffers.
+ * This will utilize a maximum total space of 256KB for the buffers.
*
* We might also want to add support for user-provided buffers in time.
* This will allow bigger buffer size flexibility, and can also be used
@@ -102,9 +105,8 @@ struct rpmsg_channel_info {
* can change this without changing anything in the firmware of the remote
* processor.
*/
-#define RPMSG_NUM_BUFS (512)
+#define MAX_RPMSG_NUM_BUFS (256)
#define RPMSG_BUF_SIZE (512)
-#define RPMSG_TOTAL_BUF_SPACE (RPMSG_NUM_BUFS * RPMSG_BUF_SIZE)
/*
* Local addresses are dynamically allocated on-demand.
@@ -579,7 +581,7 @@ static void *get_a_tx_buf(struct virtproc_info *vrp)
* either pick the next unused tx buffer
* (half of our buffers are used for sending messages)
*/
- if (vrp-last_sbuf RPMSG_NUM_BUFS / 2)
+ if (vrp-last_sbuf vrp-num_bufs / 2)
ret = vrp-sbufs + RPMSG_BUF_SIZE * vrp-last_sbuf++;
/* or recycle a used one */
else
@@ -948,6 +950,7 @@ static int rpmsg_probe(struct virtio_device *vdev)
struct virtproc_info *vrp;
void *bufs_va;
int err = 0, i;
+ size_t total_buf_space;
vrp = kzalloc(sizeof(*vrp), GFP_KERNEL);
if (!vrp)
@@ -968,10 +971,24 @@ static int rpmsg_probe(struct virtio_device *vdev)
vrp-rvq = vqs[0];
vrp-svq = vqs[1];
+ vrp-num_bufs = MAX_RPMSG_NUM_BUFS * 2;
+ if (virtqueue_get_vring_size(vrp-rvq) MAX_RPMSG_NUM_BUFS) {
+ /*
+* Override if the virtqueues are configured with less number of
+* buffers. We expect equal number of buffers for each direction
+* as per current code, so throw a warning if the configuration
+* doesn't match.
+*/
+ vrp-num_bufs = virtqueue_get_vring_size(vrp-rvq) * 2;
+ WARN_ON(virtqueue_get_vring_size(vrp-svq) !=
+ (vrp-num_bufs / 2));
+ }
+ total_buf_space = vrp-num_bufs * RPMSG_BUF_SIZE;
+
/* allocate coherent memory for the buffers */
bufs_va = dma_alloc_coherent(vdev-dev.parent-parent,
- RPMSG_TOTAL_BUF_SPACE,
- vrp-bufs_dma, GFP_KERNEL);
+total_buf_space, vrp-bufs_dma,
+GFP_KERNEL);
if (!bufs_va) {
err = -ENOMEM;
goto vqs_del;
@@ -984,10 +1001,10 @@
