Add support into the rpmsg char driver to skip copying the data into an
skb if the endpoint supports rpmsg_rx_done. If the endpoint supports
the rx_done operation, allocate a zero sized skb and set the data to
the buffer returned in the rx callback. When the packet is read from
the character device, release the memory by calling rpmsg_rx_done().
Signed-off-by: Chris Lew <[email protected]>
---
drivers/rpmsg/rpmsg_char.c | 50 ++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 48 insertions(+), 2 deletions(-)
diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
index b6183d4f62a2..be62ddcf356c 100644
--- a/drivers/rpmsg/rpmsg_char.c
+++ b/drivers/rpmsg/rpmsg_char.c
@@ -91,8 +91,8 @@ int rpmsg_chrdev_eptdev_destroy(struct device *dev, void *data)
}
EXPORT_SYMBOL(rpmsg_chrdev_eptdev_destroy);
-static int rpmsg_ept_cb(struct rpmsg_device *rpdev, void *buf, int len,
- void *priv, u32 addr)
+static int rpmsg_ept_copy_cb(struct rpmsg_device *rpdev, void *buf, int len,
+ void *priv, u32 addr)
{
struct rpmsg_eptdev *eptdev = priv;
struct sk_buff *skb;
@@ -113,6 +113,43 @@ static int rpmsg_ept_cb(struct rpmsg_device *rpdev, void *buf, int len,
return 0;
}
+static int rpmsg_ept_no_copy_cb(struct rpmsg_device *rpdev, void *buf, int len,
+ void *priv, u32 addr)
+{
+ struct rpmsg_eptdev *eptdev = priv;
+ struct sk_buff *skb;
+
+ skb = alloc_skb(0, GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ skb->head = buf;
+ skb->data = buf;
+ skb_reset_tail_pointer(skb);
+ skb_set_end_offset(skb, len);
+ skb_put(skb, len);
+
+ spin_lock(&eptdev->queue_lock);
+ skb_queue_tail(&eptdev->queue, skb);
+ spin_unlock(&eptdev->queue_lock);
+
+ /* wake up any blocking processes, waiting for new data */
+ wake_up_interruptible(&eptdev->readq);
+
+ return RPMSG_DEFER;
+}
+
+static int rpmsg_ept_cb(struct rpmsg_device *rpdev, void *buf, int len,
+ void *priv, u32 addr)
+{
+ struct rpmsg_eptdev *eptdev = priv;
+ rpmsg_rx_cb_t cb;
+
+ cb = (eptdev->ept->rx_done) ? rpmsg_ept_no_copy_cb : rpmsg_ept_copy_cb;
+
+ return cb(rpdev, buf, len, priv, addr);
+}
+
static int rpmsg_eptdev_open(struct inode *inode, struct file *filp)
{
struct rpmsg_eptdev *eptdev = cdev_to_eptdev(inode->i_cdev);
@@ -210,6 +247,15 @@ static ssize_t rpmsg_eptdev_read_iter(struct kiocb *iocb, struct iov_iter *to)
if (copy_to_iter(skb->data, use, to) != use)
use = -EFAULT;
+ if (eptdev->ept->rx_done) {
+ rpmsg_rx_done(eptdev->ept, skb->data);
+ /*
+ * Data memory is freed by rpmsg_rx_done(), reset the skb data
+ * pointers so kfree_skb() does not try to free a second time.
+ */
+ skb->head = NULL;
+ skb->data = NULL;
+ }
kfree_skb(skb);
return use;
--
2.7.4
On Tue, Jun 07, 2022 at 06:16:43PM -0700, Chris Lew wrote:
> Add support into the rpmsg char driver to skip copying the data into an
> skb if the endpoint supports rpmsg_rx_done. If the endpoint supports
> the rx_done operation, allocate a zero sized skb and set the data to
> the buffer returned in the rx callback. When the packet is read from
> the character device, release the memory by calling rpmsg_rx_done().
>
> Signed-off-by: Chris Lew <[email protected]>
> ---
> drivers/rpmsg/rpmsg_char.c | 50 ++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
> index b6183d4f62a2..be62ddcf356c 100644
> --- a/drivers/rpmsg/rpmsg_char.c
> +++ b/drivers/rpmsg/rpmsg_char.c
> @@ -91,8 +91,8 @@ int rpmsg_chrdev_eptdev_destroy(struct device *dev, void *data)
> }
> EXPORT_SYMBOL(rpmsg_chrdev_eptdev_destroy);
>
> -static int rpmsg_ept_cb(struct rpmsg_device *rpdev, void *buf, int len,
> - void *priv, u32 addr)
> +static int rpmsg_ept_copy_cb(struct rpmsg_device *rpdev, void *buf, int len,
> + void *priv, u32 addr)
> {
> struct rpmsg_eptdev *eptdev = priv;
> struct sk_buff *skb;
> @@ -113,6 +113,43 @@ static int rpmsg_ept_cb(struct rpmsg_device *rpdev, void *buf, int len,
> return 0;
> }
>
> +static int rpmsg_ept_no_copy_cb(struct rpmsg_device *rpdev, void *buf, int len,
> + void *priv, u32 addr)
> +{
> + struct rpmsg_eptdev *eptdev = priv;
> + struct sk_buff *skb;
> +
> + skb = alloc_skb(0, GFP_ATOMIC);
> + if (!skb)
> + return -ENOMEM;
> +
> + skb->head = buf;
> + skb->data = buf;
> + skb_reset_tail_pointer(skb);
> + skb_set_end_offset(skb, len);
> + skb_put(skb, len);
> +
I was worried about all that open ended code but looking at the sk_buff API I
don't think it is possible to do otherwise. As such:
Reviewed-by: Mathieu Poirier <[email protected]>
> + spin_lock(&eptdev->queue_lock);
> + skb_queue_tail(&eptdev->queue, skb);
> + spin_unlock(&eptdev->queue_lock);
> +
> + /* wake up any blocking processes, waiting for new data */
> + wake_up_interruptible(&eptdev->readq);
> +
> + return RPMSG_DEFER;
> +}
> +
> +static int rpmsg_ept_cb(struct rpmsg_device *rpdev, void *buf, int len,
> + void *priv, u32 addr)
> +{
> + struct rpmsg_eptdev *eptdev = priv;
> + rpmsg_rx_cb_t cb;
> +
> + cb = (eptdev->ept->rx_done) ? rpmsg_ept_no_copy_cb : rpmsg_ept_copy_cb;
> +
> + return cb(rpdev, buf, len, priv, addr);
> +}
> +
> static int rpmsg_eptdev_open(struct inode *inode, struct file *filp)
> {
> struct rpmsg_eptdev *eptdev = cdev_to_eptdev(inode->i_cdev);
> @@ -210,6 +247,15 @@ static ssize_t rpmsg_eptdev_read_iter(struct kiocb *iocb, struct iov_iter *to)
> if (copy_to_iter(skb->data, use, to) != use)
> use = -EFAULT;
>
> + if (eptdev->ept->rx_done) {
> + rpmsg_rx_done(eptdev->ept, skb->data);
> + /*
> + * Data memory is freed by rpmsg_rx_done(), reset the skb data
> + * pointers so kfree_skb() does not try to free a second time.
> + */
> + skb->head = NULL;
> + skb->data = NULL;
> + }
> kfree_skb(skb);
>
> return use;
> --
> 2.7.4
>