public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/3] rpmsg: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI
@ 2025-10-15 15:17 Dawei Li
  2025-10-15 15:17 ` [PATCH v5 1/3] rpmsg: char: Reuse eptdev logic for anonymous device Dawei Li
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Dawei Li @ 2025-10-15 15:17 UTC (permalink / raw)
  To: andersson, mathieu.poirier
  Cc: linux-remoteproc, linux-kernel, dawei.li, set_pte_at

Hi,

This is V5 of series which introduce new uAPI(RPMSG_CREATE_EPT_FD_IOCTL)
for rpmsg subsystem.

Current uAPI implementation for rpmsg ctrl & char device manipulation is
abstracted in procedures below:
- fd = open("/dev/rpmsg_ctrlX")
- ioctl(fd, RPMSG_CREATE_EPT_IOCTL, &info); /dev/rpmsgY devnode is
  generated.
- fd_ep = open("/dev/rpmsgY", O_RDWR)
- operations on fd_ep(write, read, poll ioctl)
- ioctl(fd_ep, RPMSG_DESTROY_EPT_IOCTL)
- close(fd_ep)
- close(fd)

This /dev/rpmsgY abstraction is less favorable for:
- Performance issue: It's time consuming for some operations are
involved:
  - Device node creation.
    Depends on specific config, especially CONFIG_DEVTMPFS, the overall
    overhead is based on coordination between DEVTMPFS and userspace
    tools such as udev and mdev.
  - Extra kernel-userspace switch cost.
  - Other major costs brought by heavy-weight logic like device_add().

- /dev/rpmsgY node can be opened only once. It doesn't make much sense
    that a dynamically created device node can be opened only once.

- For some container application such as docker, a client can't access
  host's dev unless specified explicitly. But in case of /dev/rpmsgY, which
  is generated dynamically and whose existence is unknown for clients in
  advance, this uAPI based on device node doesn't fit well.

An anonymous inode based approach is introduced to address the issues
above. Rather than generating device node and opening it, rpmsg code just
creates an anonymous inode representing eptdev and return the fd to
userspace.

Performance demo

A simple C application is tested to verify performance of new uAPI.
Please be noted that all '#' in code are preceded with space to suppress
checkpatch complaints.

$ cat test.c

 #include <linux/rpmsg.h>

 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/ioctl.h>
 #include <fcntl.h>
 #include <string.h>
 #include <stdio.h>
 #include <unistd.h>
 #include <stdlib.h>
 #include <errno.h>
 #include <sys/time.h>

 #define N (1 << 20)

int main(int argc, char *argv[])
{
	int ret, fd, ep_fd, loop;
	struct rpmsg_endpoint_info info;
	struct rpmsg_endpoint_fd_info fd_info;
	struct timeval start, end;
	int i = 0;
	double t1, t2;

	fd = -1;
	ep_fd = -1;
	loop = N;

	if (argc == 1) {
		loop = N;
	} else if (argc > 1) {
		loop = atoi(argv[1]);
	}

	printf("loop[%d]\n", loop);

	strcpy(info.name, "epx");
	info.src = -1;
	info.dst = -1;

	strcpy(fd_info.name, "epx");
	fd_info.src = -1;
	fd_info.dst = -1;
	fd_info.fd = -1;

	while (fd < 0) {
		fd = open("/dev/rpmsg_ctrl0", O_RDWR);
		if (fd < 0) {
			printf("open rpmsg_ctrl0 failed, fd[%d]\n", fd);
		}
	}

	gettimeofday(&start, NULL);

	while (loop--) {
		ret = ioctl(fd, RPMSG_CREATE_EPT_IOCTL, &info);
		if (ret < 0) {
			printf("ioctl[RPMSG_CREATE_EPT_IOCTL] failed,
			ret[%d]\n", ret);
		}

		ep_fd = -1;
		i = 0;

		while (ep_fd < 0) {
			ep_fd = open("/dev/rpmsg0", O_RDWR);
			if (ep_fd < 0) {
				i++;
				printf("open rpmsg0 failed, epfd[%d]\n", ep_fd);
			}
		}

		ret = ioctl(ep_fd, RPMSG_DESTROY_EPT_IOCTL, &info);
		if (ret < 0) {
			printf("old RPMSG_DESTROY_EPT_IOCTL failed, ret[%d], errno[%d]\n",
			ret, errno);
		}

		close(ep_fd);
	}
	
	gettimeofday(&end, NULL);

	printf("time for old way: [%ld] us\n",
		1000000 * (end.tv_sec - start.tv_sec) + end.tv_usec - start.tv_usec);
	t1 = 1000000 * (end.tv_sec - start.tv_sec) + end.tv_usec - start.tv_usec;

	if (argc == 1) {
		loop = N;
	} else if (argc > 1) {
		loop = atoi(argv[1]);
	}

	printf("loop[%d]\n", loop);

	gettimeofday(&start, NULL);

	while (loop--) {
		fd_info.fd = -1;
		fd_info.flags = O_RDWR | O_CLOEXEC | O_NONBLOCK;
		ret = ioctl(fd, RPMSG_CREATE_EPT_FD_IOCTL, &fd_info);
		if (ret < 0 || fd_info.fd < 0) {
			printf("ioctl[RPMSG_CREATE_EPT_FD_IOCTL] failed, ret[%d]\n", ret);
		}

		ret = ioctl(fd_info.fd, RPMSG_DESTROY_EPT_IOCTL, &info);
		if (ret < 0) {
			printf("new ioctl[RPMSG_DESTROY_EPT_IOCTL] failed, ret[%d]\n", ret);
		}

		close(fd_info.fd);
	}
	
	gettimeofday(&end, NULL);

	printf("time for new way: [%ld] us\n",
	1000000 * (end.tv_sec - start.tv_sec) + end.tv_usec - start.tv_usec);
	t2 = 1000000 * (end.tv_sec - start.tv_sec) + end.tv_usec - start.tv_usec;

	printf("t1(old) / t2(new) = %f\n", t1 / t2);

	close(fd);
}

Performance benchmark

- Legacy means benchmark based on old uAPI
- New means benchmark based on new uAPI(the one this series introduce)
- Time are in units of us(10^-6 s)

Test	loops	Total time(legacy)	Total time(new)	legacy/new	
1	1000	148362			1153		128.674761	
2	1000	145640			1121		129.919715	
3	1000	145303			1174		123.767462	
4	1000	150294			1142		131.605954	
5	1000	160877			1175		136.916596	
6	1000	154400			1134		136.155203	
7	1000	143252			1163		123.174549	
8	1000	148758			1161		128.129199
9	1000	149044			1112		134.032374
10	1000	146895			1192		123.234060
11	10000	1428967			11627		122.900748
12	10000	1367015			10557		129.488965
13	10000	1371919			11663		117.630027
14	10000	1358447			11080		122.603520
15	10000	1375463			11245		122.317741
16	10000	1364901			11153		122.379718
17	10000	1352665			10735		126.005123
18	10000	1400873			11341		123.522882
19	10000	1391276			10892		127.733750
20	10000	1394367			11110		125.505581
21	100000	14069671		115569		121.742604
22	100000	13663364		117074		116.707074
23	100000	13735740		115638		118.782234
24	100000	13714441		119362		114.897882
25	100000	13904366		118282		117.552679
26	100000	13870560		117717		117.829710
27	100000	13713605		118312		115.910516
28	100000	13872852		114347		121.322396
29	100000	13777964		119072		115.711200
30	100000	13725654		116296		118.023440

Changelog:

Changes in v5:
- Rebased on v6.18.rc1.
- Fix checkpatch warning on commit msg on patch[1/3].
- Other minor commit msg tweaks.
- Update performance testing results.
- Link to v4:
  https://lore.kernel.org/all/20250609151531.22621-1-dawei.li@linux.dev/

Changes in v4:
- Build warning of copy_to_user (Dan).
- ioctl() branches reorder (Beleswar).
- Remove local variable fd and pass &ept_fd_info.fd to
  rpmsg_anonymous_eptdev_create().
- Link to v3:
  https://lore.kernel.org/all/20250519150823.62350-1-dawei.li@linux.dev/

Changes in v3:
- s/anon/anonymous (Mathieu)
- API naming adjustment (Mathieu)
  - __rpmsg_chrdev_eptdev_alloc ->  rpmsg_eptdev_alloc
  - __rpmsg_chrdev_eptdev_add ->  rpmsg_eptdev_add
- Add parameter 'flags' to uAPI so user can specify file flags
  explicitly on creating anonymous inode.
- Link to v2:
  https://lore.kernel.org/all/20250509155927.109258-1-dawei.li@linux.dev/

Changes in v2:
- Fix compilation error for !CONFIG_RPMSG_CHAR config(Test robot).
- Link to v1:
  https://lore.kernel.org/all/20250507141712.4276-1-dawei.li@linux.dev/

Dawei Li (3):
  rpmsg: char: Reuse eptdev logic for anonymous device
  rpmsg: char: Implement eptdev based on anonymous inode
  rpmsg: ctrl: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI

 drivers/rpmsg/rpmsg_char.c | 129 ++++++++++++++++++++++++++++++-------
 drivers/rpmsg/rpmsg_char.h |  23 +++++++
 drivers/rpmsg/rpmsg_ctrl.c |  35 ++++++++--
 include/uapi/linux/rpmsg.h |  27 +++++++-
 4 files changed, 182 insertions(+), 32 deletions(-)

---

Thanks,

	Dawei

-- 
2.25.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v5 1/3] rpmsg: char: Reuse eptdev logic for anonymous device
  2025-10-15 15:17 [PATCH v5 0/3] rpmsg: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI Dawei Li
@ 2025-10-15 15:17 ` Dawei Li
  2025-10-15 15:17 ` [PATCH v5 2/3] rpmsg: char: Implement eptdev based on anonymous inode Dawei Li
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Dawei Li @ 2025-10-15 15:17 UTC (permalink / raw)
  To: andersson, mathieu.poirier
  Cc: linux-remoteproc, linux-kernel, dawei.li, set_pte_at

Current uAPI implementation for rpmsg ctrl & char device manipulation is
abstracted in procedures below:
- fd = open("/dev/rpmsg_ctrlX")
- ioctl(fd, RPMSG_CREATE_EPT_IOCTL, &info); /dev/rpmsgY devnode is
  generated.
- fd_ep = open("/dev/rpmsgY", O_RDWR)
- operations on fd_ep(write, read, poll ioctl)
- ioctl(fd_ep, RPMSG_DESTROY_EPT_IOCTL)
- close(fd_ep)
- close(fd)

This /dev/rpmsgY abstraction is less favorable for:
- Performance issue: It's time consuming for some operations are involved:
  - Device node creation.
    Depends on specific config, especially CONFIG_DEVTMPFS, the overall
    overhead is based on coordination between DEVTMPFS and userspace
    tools such as udev and mdev.
  - Extra kernel-userspace switch cost.
  - Other major costs brought by heavy-weight logic like device_add().

- /dev/rpmsgY node can be opened only once. It doesn't make much sense
    that a dynamically created device node can be opened only once.

- For some container application such as docker, a client can't access
  host's device node unless specified explicitly. But in case of
  /dev/rpmsgY, which is generated dynamically and whose existence is
  unknown for clients in advance, this uAPI based on device node doesn't
  fit well.

An anonymous inode based approach is introduced to address the issues
above. Rather than generating device node and opening it, rpmsg code just
creates an anonymous inode representing eptdev and return the fd to
userspace.

The legacy abstraction based on struct device and struct cdev is honored:
- Avoid legacy uAPI break(RPMSG_CREATE_EPT_IOCTL)
- Reuse existing logic:
  - dev_err() and friends.
  - Life cycle management of struct device.

Signed-off-by: Dawei Li <dawei.li@linux.dev>
---
 drivers/rpmsg/rpmsg_char.c | 80 ++++++++++++++++++++++++++------------
 1 file changed, 56 insertions(+), 24 deletions(-)

diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
index 96fcdd2d7093..85154a422e9d 100644
--- a/drivers/rpmsg/rpmsg_char.c
+++ b/drivers/rpmsg/rpmsg_char.c
@@ -91,7 +91,8 @@ int rpmsg_chrdev_eptdev_destroy(struct device *dev, void *data)
 	/* wake up any blocked readers */
 	wake_up_interruptible(&eptdev->readq);
 
-	cdev_device_del(&eptdev->cdev, &eptdev->dev);
+	if (eptdev->dev.devt)
+		cdev_device_del(&eptdev->cdev, &eptdev->dev);
 	put_device(&eptdev->dev);
 
 	return 0;
@@ -132,21 +133,17 @@ static int rpmsg_ept_flow_cb(struct rpmsg_device *rpdev, void *priv, bool enable
 	return 0;
 }
 
-static int rpmsg_eptdev_open(struct inode *inode, struct file *filp)
+static int __rpmsg_eptdev_open(struct rpmsg_eptdev *eptdev)
 {
-	struct rpmsg_eptdev *eptdev = cdev_to_eptdev(inode->i_cdev);
 	struct rpmsg_endpoint *ept;
 	struct rpmsg_device *rpdev = eptdev->rpdev;
 	struct device *dev = &eptdev->dev;
 
-	mutex_lock(&eptdev->ept_lock);
 	if (eptdev->ept) {
-		mutex_unlock(&eptdev->ept_lock);
 		return -EBUSY;
 	}
 
 	if (!eptdev->rpdev) {
-		mutex_unlock(&eptdev->ept_lock);
 		return -ENETRESET;
 	}
 
@@ -164,21 +161,32 @@ static int rpmsg_eptdev_open(struct inode *inode, struct file *filp)
 	if (!ept) {
 		dev_err(dev, "failed to open %s\n", eptdev->chinfo.name);
 		put_device(dev);
-		mutex_unlock(&eptdev->ept_lock);
 		return -EINVAL;
 	}
 
 	ept->flow_cb = rpmsg_ept_flow_cb;
 	eptdev->ept = ept;
-	filp->private_data = eptdev;
-	mutex_unlock(&eptdev->ept_lock);
 
 	return 0;
 }
 
-static int rpmsg_eptdev_release(struct inode *inode, struct file *filp)
+static int rpmsg_eptdev_open(struct inode *inode, struct file *filp)
 {
 	struct rpmsg_eptdev *eptdev = cdev_to_eptdev(inode->i_cdev);
+	int ret;
+
+	mutex_lock(&eptdev->ept_lock);
+	ret = __rpmsg_eptdev_open(eptdev);
+	if (!ret)
+		filp->private_data = eptdev;
+	mutex_unlock(&eptdev->ept_lock);
+
+	return ret;
+}
+
+static int rpmsg_eptdev_release(struct inode *inode, struct file *filp)
+{
+	struct rpmsg_eptdev *eptdev = filp->private_data;
 	struct device *dev = &eptdev->dev;
 
 	/* Close the endpoint, if it's not already destroyed by the parent */
@@ -400,12 +408,13 @@ static void rpmsg_eptdev_release_device(struct device *dev)
 	struct rpmsg_eptdev *eptdev = dev_to_eptdev(dev);
 
 	ida_free(&rpmsg_ept_ida, dev->id);
-	ida_free(&rpmsg_minor_ida, MINOR(eptdev->dev.devt));
+	if (eptdev->dev.devt)
+		ida_free(&rpmsg_minor_ida, MINOR(eptdev->dev.devt));
 	kfree(eptdev);
 }
 
-static struct rpmsg_eptdev *rpmsg_chrdev_eptdev_alloc(struct rpmsg_device *rpdev,
-						      struct device *parent)
+static struct rpmsg_eptdev *rpmsg_eptdev_alloc(struct rpmsg_device *rpdev,
+					       struct device *parent, bool cdev)
 {
 	struct rpmsg_eptdev *eptdev;
 	struct device *dev;
@@ -428,33 +437,50 @@ static struct rpmsg_eptdev *rpmsg_chrdev_eptdev_alloc(struct rpmsg_device *rpdev
 	dev->groups = rpmsg_eptdev_groups;
 	dev_set_drvdata(dev, eptdev);
 
-	cdev_init(&eptdev->cdev, &rpmsg_eptdev_fops);
-	eptdev->cdev.owner = THIS_MODULE;
+	if (cdev) {
+		cdev_init(&eptdev->cdev, &rpmsg_eptdev_fops);
+		eptdev->cdev.owner = THIS_MODULE;
+	}
 
 	return eptdev;
 }
 
-static int rpmsg_chrdev_eptdev_add(struct rpmsg_eptdev *eptdev, struct rpmsg_channel_info chinfo)
+static struct rpmsg_eptdev *rpmsg_chrdev_eptdev_alloc(struct rpmsg_device *rpdev,
+						      struct device *parent)
+{
+	return rpmsg_eptdev_alloc(rpdev, parent, true);
+}
+
+static int rpmsg_eptdev_add(struct rpmsg_eptdev *eptdev,
+			    struct rpmsg_channel_info chinfo, bool cdev)
 {
 	struct device *dev = &eptdev->dev;
 	int ret;
 
 	eptdev->chinfo = chinfo;
 
-	ret = ida_alloc_max(&rpmsg_minor_ida, RPMSG_DEV_MAX - 1, GFP_KERNEL);
-	if (ret < 0)
-		goto free_eptdev;
-	dev->devt = MKDEV(MAJOR(rpmsg_major), ret);
+	if (cdev) {
+		ret = ida_alloc_max(&rpmsg_minor_ida, RPMSG_DEV_MAX - 1, GFP_KERNEL);
+		if (ret < 0)
+			goto free_eptdev;
 
+		dev->devt = MKDEV(MAJOR(rpmsg_major), ret);
+	}
+
+	/* Anonymous inode device still need device name for dev_err() and friends */
 	ret = ida_alloc(&rpmsg_ept_ida, GFP_KERNEL);
 	if (ret < 0)
 		goto free_minor_ida;
 	dev->id = ret;
 	dev_set_name(dev, "rpmsg%d", ret);
 
-	ret = cdev_device_add(&eptdev->cdev, &eptdev->dev);
-	if (ret)
-		goto free_ept_ida;
+	ret = 0;
+
+	if (cdev) {
+		ret = cdev_device_add(&eptdev->cdev, &eptdev->dev);
+		if (ret)
+			goto free_ept_ida;
+	}
 
 	/* We can now rely on the release function for cleanup */
 	dev->release = rpmsg_eptdev_release_device;
@@ -464,7 +490,8 @@ static int rpmsg_chrdev_eptdev_add(struct rpmsg_eptdev *eptdev, struct rpmsg_cha
 free_ept_ida:
 	ida_free(&rpmsg_ept_ida, dev->id);
 free_minor_ida:
-	ida_free(&rpmsg_minor_ida, MINOR(dev->devt));
+	if (cdev)
+		ida_free(&rpmsg_minor_ida, MINOR(dev->devt));
 free_eptdev:
 	put_device(dev);
 	kfree(eptdev);
@@ -472,6 +499,11 @@ static int rpmsg_chrdev_eptdev_add(struct rpmsg_eptdev *eptdev, struct rpmsg_cha
 	return ret;
 }
 
+static int rpmsg_chrdev_eptdev_add(struct rpmsg_eptdev *eptdev, struct rpmsg_channel_info chinfo)
+{
+	return rpmsg_eptdev_add(eptdev, chinfo, true);
+}
+
 int rpmsg_chrdev_eptdev_create(struct rpmsg_device *rpdev, struct device *parent,
 			       struct rpmsg_channel_info chinfo)
 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v5 2/3] rpmsg: char: Implement eptdev based on anonymous inode
  2025-10-15 15:17 [PATCH v5 0/3] rpmsg: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI Dawei Li
  2025-10-15 15:17 ` [PATCH v5 1/3] rpmsg: char: Reuse eptdev logic for anonymous device Dawei Li
@ 2025-10-15 15:17 ` Dawei Li
  2025-10-15 15:17 ` [PATCH v5 3/3] rpmsg: ctrl: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI Dawei Li
  2025-10-16 15:32 ` [PATCH v5 0/3] rpmsg: " Mathieu Poirier
  3 siblings, 0 replies; 7+ messages in thread
From: Dawei Li @ 2025-10-15 15:17 UTC (permalink / raw)
  To: andersson, mathieu.poirier
  Cc: linux-remoteproc, linux-kernel, dawei.li, set_pte_at

Introduce new eptdev abstraction based on anonymous inode. The new API
is exactly same with legacy one except:

- It's anonymous and devnode/path free.
- Its fops->open() is empty.

Signed-off-by: Dawei Li <dawei.li@linux.dev>
---
 drivers/rpmsg/rpmsg_char.c | 49 ++++++++++++++++++++++++++++++++++++++
 drivers/rpmsg/rpmsg_char.h | 23 ++++++++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c
index 85154a422e9d..34b35ea74aab 100644
--- a/drivers/rpmsg/rpmsg_char.c
+++ b/drivers/rpmsg/rpmsg_char.c
@@ -13,6 +13,7 @@
 
 #define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
 
+#include <linux/anon_inodes.h>
 #include <linux/cdev.h>
 #include <linux/device.h>
 #include <linux/fs.h>
@@ -517,6 +518,54 @@ int rpmsg_chrdev_eptdev_create(struct rpmsg_device *rpdev, struct device *parent
 }
 EXPORT_SYMBOL(rpmsg_chrdev_eptdev_create);
 
+static const struct file_operations rpmsg_anonymous_eptdev_fops = {
+	.owner = THIS_MODULE,
+	.release = rpmsg_eptdev_release,
+	.read_iter = rpmsg_eptdev_read_iter,
+	.write_iter = rpmsg_eptdev_write_iter,
+	.poll = rpmsg_eptdev_poll,
+	.unlocked_ioctl = rpmsg_eptdev_ioctl,
+	.compat_ioctl = compat_ptr_ioctl,
+};
+
+int rpmsg_anonymous_eptdev_create(struct rpmsg_device *rpdev, struct device *parent,
+				  struct rpmsg_channel_info chinfo, unsigned int flags,
+				  int *pfd)
+{
+	struct rpmsg_eptdev *eptdev;
+	int ret, fd;
+
+	/* Anonymous inode only supports these file flags */
+	if (flags & ~(O_ACCMODE | O_NONBLOCK | O_CLOEXEC))
+		return -EINVAL;
+
+	eptdev = rpmsg_eptdev_alloc(rpdev, parent, false);
+	if (IS_ERR(eptdev))
+		return PTR_ERR(eptdev);
+
+	ret =  rpmsg_eptdev_add(eptdev, chinfo, false);
+	if (ret) {
+		dev_err(&eptdev->dev, "failed to add %s\n", eptdev->chinfo.name);
+		return ret;
+	}
+
+	fd = anon_inode_getfd("rpmsg-eptdev", &rpmsg_anonymous_eptdev_fops, eptdev, flags);
+	if (fd < 0) {
+		put_device(&eptdev->dev);
+		return fd;
+	}
+
+	mutex_lock(&eptdev->ept_lock);
+	ret = __rpmsg_eptdev_open(eptdev);
+	mutex_unlock(&eptdev->ept_lock);
+
+	if (!ret)
+		*pfd = fd;
+
+	return ret;
+}
+EXPORT_SYMBOL(rpmsg_anonymous_eptdev_create);
+
 static int rpmsg_chrdev_probe(struct rpmsg_device *rpdev)
 {
 	struct rpmsg_channel_info chinfo;
diff --git a/drivers/rpmsg/rpmsg_char.h b/drivers/rpmsg/rpmsg_char.h
index 117d9cbc52f0..70ce2c511f13 100644
--- a/drivers/rpmsg/rpmsg_char.h
+++ b/drivers/rpmsg/rpmsg_char.h
@@ -19,6 +19,22 @@
 int rpmsg_chrdev_eptdev_create(struct rpmsg_device *rpdev, struct device *parent,
 			       struct rpmsg_channel_info chinfo);
 
+/**
+ * rpmsg_anonymous_eptdev_create() - register anonymous device and its associated
+ *                                   fd based on an endpoint
+ * @rpdev:  prepared rpdev to be used for creating endpoints
+ * @parent: parent device
+ * @chinfo: associated endpoint channel information.
+ * @flag: file flag
+ * @pfd: fd in represent of endpoint device
+ *
+ * This function create a new rpmsg endpoint device and its associated fd to instantiate a new
+ * endpoint based on chinfo information.
+ */
+int rpmsg_anonymous_eptdev_create(struct rpmsg_device *rpdev, struct device *parent,
+				  struct rpmsg_channel_info chinfo, unsigned int flags,
+				  int *pfd);
+
 /**
  * rpmsg_chrdev_eptdev_destroy() - destroy created char device endpoint.
  * @data: private data associated to the endpoint device
@@ -36,6 +52,13 @@ static inline int rpmsg_chrdev_eptdev_create(struct rpmsg_device *rpdev, struct
 	return -ENXIO;
 }
 
+static inline int rpmsg_anonymous_eptdev_create(struct rpmsg_device *rpdev, struct device *parent,
+						struct rpmsg_channel_info chinfo,
+						unsigned int flags, int *pfd)
+{
+	return -ENXIO;
+}
+
 static inline int rpmsg_chrdev_eptdev_destroy(struct device *dev, void *data)
 {
 	return -ENXIO;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v5 3/3] rpmsg: ctrl: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI
  2025-10-15 15:17 [PATCH v5 0/3] rpmsg: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI Dawei Li
  2025-10-15 15:17 ` [PATCH v5 1/3] rpmsg: char: Reuse eptdev logic for anonymous device Dawei Li
  2025-10-15 15:17 ` [PATCH v5 2/3] rpmsg: char: Implement eptdev based on anonymous inode Dawei Li
@ 2025-10-15 15:17 ` Dawei Li
  2025-10-16 15:32 ` [PATCH v5 0/3] rpmsg: " Mathieu Poirier
  3 siblings, 0 replies; 7+ messages in thread
From: Dawei Li @ 2025-10-15 15:17 UTC (permalink / raw)
  To: andersson, mathieu.poirier
  Cc: linux-remoteproc, linux-kernel, dawei.li, set_pte_at

Implement RPMSG_CREATE_EPT_FD_IOCTL, new uAPI for rpmsg ctrl, which
shares most of operations of RPMSG_CREATE_EPT_IOCTL except that it
returns fd representing eptdev to userspace directly.

Possible calling procedures for userspace are:
- fd = open("/dev/rpmsg_ctrlX")
- ioctl(fd, RPMSG_CREATE_EPT_FD_IOCTL, &info);
- fd_ep = info.fd
- operations on fd_ep(write, read, poll ioctl)
- ioctl(fd_ep, RPMSG_DESTROY_EPT_IOCTL)
- close(fd_ep)
- close(fd)

Signed-off-by: Dawei Li <dawei.li@linux.dev>
---
 drivers/rpmsg/rpmsg_ctrl.c | 35 ++++++++++++++++++++++++++++-------
 include/uapi/linux/rpmsg.h | 27 ++++++++++++++++++++++++++-
 2 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/drivers/rpmsg/rpmsg_ctrl.c b/drivers/rpmsg/rpmsg_ctrl.c
index 28f57945ccd9..efb207506e5c 100644
--- a/drivers/rpmsg/rpmsg_ctrl.c
+++ b/drivers/rpmsg/rpmsg_ctrl.c
@@ -75,19 +75,30 @@ static long rpmsg_ctrldev_ioctl(struct file *fp, unsigned int cmd,
 				unsigned long arg)
 {
 	struct rpmsg_ctrldev *ctrldev = fp->private_data;
+	struct rpmsg_endpoint_fd_info ept_fd_info;
 	void __user *argp = (void __user *)arg;
 	struct rpmsg_endpoint_info eptinfo;
 	struct rpmsg_channel_info chinfo;
 	struct rpmsg_device *rpdev;
 	int ret = 0;
 
-	if (copy_from_user(&eptinfo, argp, sizeof(eptinfo)))
-		return -EFAULT;
-
-	memcpy(chinfo.name, eptinfo.name, RPMSG_NAME_SIZE);
-	chinfo.name[RPMSG_NAME_SIZE - 1] = '\0';
-	chinfo.src = eptinfo.src;
-	chinfo.dst = eptinfo.dst;
+	if (cmd == RPMSG_CREATE_EPT_FD_IOCTL) {
+		if (copy_from_user(&ept_fd_info, argp, sizeof(ept_fd_info)))
+			return -EFAULT;
+
+		memcpy(chinfo.name, ept_fd_info.name, RPMSG_NAME_SIZE);
+		chinfo.name[RPMSG_NAME_SIZE - 1] = '\0';
+		chinfo.src = ept_fd_info.src;
+		chinfo.dst = ept_fd_info.dst;
+	} else {
+		if (copy_from_user(&eptinfo, argp, sizeof(eptinfo)))
+			return -EFAULT;
+
+		memcpy(chinfo.name, eptinfo.name, RPMSG_NAME_SIZE);
+		chinfo.name[RPMSG_NAME_SIZE - 1] = '\0';
+		chinfo.src = eptinfo.src;
+		chinfo.dst = eptinfo.dst;
+	}
 
 	mutex_lock(&ctrldev->ctrl_lock);
 	switch (cmd) {
@@ -110,6 +121,16 @@ static long rpmsg_ctrldev_ioctl(struct file *fp, unsigned int cmd,
 				chinfo.name, ret);
 		break;
 
+	case RPMSG_CREATE_EPT_FD_IOCTL:
+		ret = rpmsg_anonymous_eptdev_create(ctrldev->rpdev, &ctrldev->dev, chinfo,
+						    ept_fd_info.flags, &ept_fd_info.fd);
+		if (ret)
+			break;
+
+		if (copy_to_user(argp, &ept_fd_info, sizeof(ept_fd_info)))
+			ret = -EFAULT;
+		break;
+
 	default:
 		ret = -EINVAL;
 	}
diff --git a/include/uapi/linux/rpmsg.h b/include/uapi/linux/rpmsg.h
index f0c8da2b185b..02befb298f71 100644
--- a/include/uapi/linux/rpmsg.h
+++ b/include/uapi/linux/rpmsg.h
@@ -29,7 +29,8 @@ struct rpmsg_endpoint_info {
 #define RPMSG_CREATE_EPT_IOCTL	_IOW(0xb5, 0x1, struct rpmsg_endpoint_info)
 
 /**
- * Destroy a rpmsg char device endpoint created by the RPMSG_CREATE_EPT_IOCTL.
+ * Destroy a rpmsg char device endpoint created by the RPMSG_CREATE_EPT_IOCTL
+ * or RPMSG_CREATE_EPT_FD_IOCTL.
  */
 #define RPMSG_DESTROY_EPT_IOCTL	_IO(0xb5, 0x2)
 
@@ -53,4 +54,28 @@ struct rpmsg_endpoint_info {
  */
 #define RPMSG_SET_INCOMING_FLOWCONTROL _IOR(0xb5, 0x6, int)
 
+/**
+ * struct rpmsg_endpoint_fd_info - endpoint & fd info representation
+ * @name: name of service
+ * @src: local address. To set to RPMSG_ADDR_ANY if not used.
+ * @dst: destination address. To set to RPMSG_ADDR_ANY if not used.
+ * @flags: file flags of endpoint device, valid flags:
+ *         O_RDONLY/O_WRONLY/O_RDWR
+ *         O_NONBLOCK
+ *         O_CLOEXEC
+ * @fd: fd returned from driver
+ */
+struct rpmsg_endpoint_fd_info {
+	char name[32];
+	__u32 src;
+	__u32 dst;
+	__u32 flags;
+	__s32 fd;
+};
+
+/**
+ * Instantiate a new rmpsg endpoint which is represented by fd
+ */
+#define RPMSG_CREATE_EPT_FD_IOCTL _IOWR(0xb5, 0x7, struct rpmsg_endpoint_fd_info)
+
 #endif
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 0/3] rpmsg: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI
  2025-10-15 15:17 [PATCH v5 0/3] rpmsg: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI Dawei Li
                   ` (2 preceding siblings ...)
  2025-10-15 15:17 ` [PATCH v5 3/3] rpmsg: ctrl: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI Dawei Li
@ 2025-10-16 15:32 ` Mathieu Poirier
  2025-10-16 16:28   ` Dawei Li
  3 siblings, 1 reply; 7+ messages in thread
From: Mathieu Poirier @ 2025-10-16 15:32 UTC (permalink / raw)
  To: Dawei Li; +Cc: andersson, linux-remoteproc, linux-kernel, set_pte_at

I have applied this set.

Thanks,
Mathieu

On Wed, Oct 15, 2025 at 11:17:15PM +0800, Dawei Li wrote:
> Hi,
> 
> This is V5 of series which introduce new uAPI(RPMSG_CREATE_EPT_FD_IOCTL)
> for rpmsg subsystem.
> 
> Current uAPI implementation for rpmsg ctrl & char device manipulation is
> abstracted in procedures below:
> - fd = open("/dev/rpmsg_ctrlX")
> - ioctl(fd, RPMSG_CREATE_EPT_IOCTL, &info); /dev/rpmsgY devnode is
>   generated.
> - fd_ep = open("/dev/rpmsgY", O_RDWR)
> - operations on fd_ep(write, read, poll ioctl)
> - ioctl(fd_ep, RPMSG_DESTROY_EPT_IOCTL)
> - close(fd_ep)
> - close(fd)
> 
> This /dev/rpmsgY abstraction is less favorable for:
> - Performance issue: It's time consuming for some operations are
> involved:
>   - Device node creation.
>     Depends on specific config, especially CONFIG_DEVTMPFS, the overall
>     overhead is based on coordination between DEVTMPFS and userspace
>     tools such as udev and mdev.
>   - Extra kernel-userspace switch cost.
>   - Other major costs brought by heavy-weight logic like device_add().
> 
> - /dev/rpmsgY node can be opened only once. It doesn't make much sense
>     that a dynamically created device node can be opened only once.
> 
> - For some container application such as docker, a client can't access
>   host's dev unless specified explicitly. But in case of /dev/rpmsgY, which
>   is generated dynamically and whose existence is unknown for clients in
>   advance, this uAPI based on device node doesn't fit well.
> 
> An anonymous inode based approach is introduced to address the issues
> above. Rather than generating device node and opening it, rpmsg code just
> creates an anonymous inode representing eptdev and return the fd to
> userspace.
> 
> Performance demo
> 
> A simple C application is tested to verify performance of new uAPI.
> Please be noted that all '#' in code are preceded with space to suppress
> checkpatch complaints.
> 
> $ cat test.c
> 
>  #include <linux/rpmsg.h>
> 
>  #include <sys/types.h>
>  #include <sys/stat.h>
>  #include <sys/ioctl.h>
>  #include <fcntl.h>
>  #include <string.h>
>  #include <stdio.h>
>  #include <unistd.h>
>  #include <stdlib.h>
>  #include <errno.h>
>  #include <sys/time.h>
> 
>  #define N (1 << 20)
> 
> int main(int argc, char *argv[])
> {
> 	int ret, fd, ep_fd, loop;
> 	struct rpmsg_endpoint_info info;
> 	struct rpmsg_endpoint_fd_info fd_info;
> 	struct timeval start, end;
> 	int i = 0;
> 	double t1, t2;
> 
> 	fd = -1;
> 	ep_fd = -1;
> 	loop = N;
> 
> 	if (argc == 1) {
> 		loop = N;
> 	} else if (argc > 1) {
> 		loop = atoi(argv[1]);
> 	}
> 
> 	printf("loop[%d]\n", loop);
> 
> 	strcpy(info.name, "epx");
> 	info.src = -1;
> 	info.dst = -1;
> 
> 	strcpy(fd_info.name, "epx");
> 	fd_info.src = -1;
> 	fd_info.dst = -1;
> 	fd_info.fd = -1;
> 
> 	while (fd < 0) {
> 		fd = open("/dev/rpmsg_ctrl0", O_RDWR);
> 		if (fd < 0) {
> 			printf("open rpmsg_ctrl0 failed, fd[%d]\n", fd);
> 		}
> 	}
> 
> 	gettimeofday(&start, NULL);
> 
> 	while (loop--) {
> 		ret = ioctl(fd, RPMSG_CREATE_EPT_IOCTL, &info);
> 		if (ret < 0) {
> 			printf("ioctl[RPMSG_CREATE_EPT_IOCTL] failed,
> 			ret[%d]\n", ret);
> 		}
> 
> 		ep_fd = -1;
> 		i = 0;
> 
> 		while (ep_fd < 0) {
> 			ep_fd = open("/dev/rpmsg0", O_RDWR);
> 			if (ep_fd < 0) {
> 				i++;
> 				printf("open rpmsg0 failed, epfd[%d]\n", ep_fd);
> 			}
> 		}
> 
> 		ret = ioctl(ep_fd, RPMSG_DESTROY_EPT_IOCTL, &info);
> 		if (ret < 0) {
> 			printf("old RPMSG_DESTROY_EPT_IOCTL failed, ret[%d], errno[%d]\n",
> 			ret, errno);
> 		}
> 
> 		close(ep_fd);
> 	}
> 	
> 	gettimeofday(&end, NULL);
> 
> 	printf("time for old way: [%ld] us\n",
> 		1000000 * (end.tv_sec - start.tv_sec) + end.tv_usec - start.tv_usec);
> 	t1 = 1000000 * (end.tv_sec - start.tv_sec) + end.tv_usec - start.tv_usec;
> 
> 	if (argc == 1) {
> 		loop = N;
> 	} else if (argc > 1) {
> 		loop = atoi(argv[1]);
> 	}
> 
> 	printf("loop[%d]\n", loop);
> 
> 	gettimeofday(&start, NULL);
> 
> 	while (loop--) {
> 		fd_info.fd = -1;
> 		fd_info.flags = O_RDWR | O_CLOEXEC | O_NONBLOCK;
> 		ret = ioctl(fd, RPMSG_CREATE_EPT_FD_IOCTL, &fd_info);
> 		if (ret < 0 || fd_info.fd < 0) {
> 			printf("ioctl[RPMSG_CREATE_EPT_FD_IOCTL] failed, ret[%d]\n", ret);
> 		}
> 
> 		ret = ioctl(fd_info.fd, RPMSG_DESTROY_EPT_IOCTL, &info);
> 		if (ret < 0) {
> 			printf("new ioctl[RPMSG_DESTROY_EPT_IOCTL] failed, ret[%d]\n", ret);
> 		}
> 
> 		close(fd_info.fd);
> 	}
> 	
> 	gettimeofday(&end, NULL);
> 
> 	printf("time for new way: [%ld] us\n",
> 	1000000 * (end.tv_sec - start.tv_sec) + end.tv_usec - start.tv_usec);
> 	t2 = 1000000 * (end.tv_sec - start.tv_sec) + end.tv_usec - start.tv_usec;
> 
> 	printf("t1(old) / t2(new) = %f\n", t1 / t2);
> 
> 	close(fd);
> }
> 
> Performance benchmark
> 
> - Legacy means benchmark based on old uAPI
> - New means benchmark based on new uAPI(the one this series introduce)
> - Time are in units of us(10^-6 s)
> 
> Test	loops	Total time(legacy)	Total time(new)	legacy/new	
> 1	1000	148362			1153		128.674761	
> 2	1000	145640			1121		129.919715	
> 3	1000	145303			1174		123.767462	
> 4	1000	150294			1142		131.605954	
> 5	1000	160877			1175		136.916596	
> 6	1000	154400			1134		136.155203	
> 7	1000	143252			1163		123.174549	
> 8	1000	148758			1161		128.129199
> 9	1000	149044			1112		134.032374
> 10	1000	146895			1192		123.234060
> 11	10000	1428967			11627		122.900748
> 12	10000	1367015			10557		129.488965
> 13	10000	1371919			11663		117.630027
> 14	10000	1358447			11080		122.603520
> 15	10000	1375463			11245		122.317741
> 16	10000	1364901			11153		122.379718
> 17	10000	1352665			10735		126.005123
> 18	10000	1400873			11341		123.522882
> 19	10000	1391276			10892		127.733750
> 20	10000	1394367			11110		125.505581
> 21	100000	14069671		115569		121.742604
> 22	100000	13663364		117074		116.707074
> 23	100000	13735740		115638		118.782234
> 24	100000	13714441		119362		114.897882
> 25	100000	13904366		118282		117.552679
> 26	100000	13870560		117717		117.829710
> 27	100000	13713605		118312		115.910516
> 28	100000	13872852		114347		121.322396
> 29	100000	13777964		119072		115.711200
> 30	100000	13725654		116296		118.023440
> 
> Changelog:
> 
> Changes in v5:
> - Rebased on v6.18.rc1.
> - Fix checkpatch warning on commit msg on patch[1/3].
> - Other minor commit msg tweaks.
> - Update performance testing results.
> - Link to v4:
>   https://lore.kernel.org/all/20250609151531.22621-1-dawei.li@linux.dev/
> 
> Changes in v4:
> - Build warning of copy_to_user (Dan).
> - ioctl() branches reorder (Beleswar).
> - Remove local variable fd and pass &ept_fd_info.fd to
>   rpmsg_anonymous_eptdev_create().
> - Link to v3:
>   https://lore.kernel.org/all/20250519150823.62350-1-dawei.li@linux.dev/
> 
> Changes in v3:
> - s/anon/anonymous (Mathieu)
> - API naming adjustment (Mathieu)
>   - __rpmsg_chrdev_eptdev_alloc ->  rpmsg_eptdev_alloc
>   - __rpmsg_chrdev_eptdev_add ->  rpmsg_eptdev_add
> - Add parameter 'flags' to uAPI so user can specify file flags
>   explicitly on creating anonymous inode.
> - Link to v2:
>   https://lore.kernel.org/all/20250509155927.109258-1-dawei.li@linux.dev/
> 
> Changes in v2:
> - Fix compilation error for !CONFIG_RPMSG_CHAR config(Test robot).
> - Link to v1:
>   https://lore.kernel.org/all/20250507141712.4276-1-dawei.li@linux.dev/
> 
> Dawei Li (3):
>   rpmsg: char: Reuse eptdev logic for anonymous device
>   rpmsg: char: Implement eptdev based on anonymous inode
>   rpmsg: ctrl: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI
> 
>  drivers/rpmsg/rpmsg_char.c | 129 ++++++++++++++++++++++++++++++-------
>  drivers/rpmsg/rpmsg_char.h |  23 +++++++
>  drivers/rpmsg/rpmsg_ctrl.c |  35 ++++++++--
>  include/uapi/linux/rpmsg.h |  27 +++++++-
>  4 files changed, 182 insertions(+), 32 deletions(-)
> 
> ---
> 
> Thanks,
> 
> 	Dawei
> 
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 0/3] rpmsg: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI
  2025-10-16 15:32 ` [PATCH v5 0/3] rpmsg: " Mathieu Poirier
@ 2025-10-16 16:28   ` Dawei Li
  2025-10-17 14:47     ` Mathieu Poirier
  0 siblings, 1 reply; 7+ messages in thread
From: Dawei Li @ 2025-10-16 16:28 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: andersson, linux-remoteproc, linux-kernel, set_pte_at, dawei.li

Hi, Mathieu, 

On Thu, Oct 16, 2025 at 09:32:04AM -0600, Mathieu Poirier wrote:
> I have applied this set.
> 
> Thanks,
> Mathieu

It seems that it is v4 being applied? [1]

The only difference between v4 and v5 is commit message, though.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git/commit/?h=rpmsg-next&id=00af63201cbb7903e5deb2a9fdebd97f979492e5

> 
> On Wed, Oct 15, 2025 at 11:17:15PM +0800, Dawei Li wrote:
> > Hi,
> > 
> > This is V5 of series which introduce new uAPI(RPMSG_CREATE_EPT_FD_IOCTL)
> > for rpmsg subsystem.
> > 
> > Current uAPI implementation for rpmsg ctrl & char device manipulation is
> > abstracted in procedures below:
> > - fd = open("/dev/rpmsg_ctrlX")
> > - ioctl(fd, RPMSG_CREATE_EPT_IOCTL, &info); /dev/rpmsgY devnode is
> >   generated.
> > - fd_ep = open("/dev/rpmsgY", O_RDWR)
> > - operations on fd_ep(write, read, poll ioctl)
> > - ioctl(fd_ep, RPMSG_DESTROY_EPT_IOCTL)
> > - close(fd_ep)
> > - close(fd)
> > 
> > This /dev/rpmsgY abstraction is less favorable for:
> > - Performance issue: It's time consuming for some operations are
> > involved:
> >   - Device node creation.
> >     Depends on specific config, especially CONFIG_DEVTMPFS, the overall
> >     overhead is based on coordination between DEVTMPFS and userspace
> >     tools such as udev and mdev.
> >   - Extra kernel-userspace switch cost.
> >   - Other major costs brought by heavy-weight logic like device_add().
> > 
> > - /dev/rpmsgY node can be opened only once. It doesn't make much sense
> >     that a dynamically created device node can be opened only once.
> > 
> > - For some container application such as docker, a client can't access
> >   host's dev unless specified explicitly. But in case of /dev/rpmsgY, which
> >   is generated dynamically and whose existence is unknown for clients in
> >   advance, this uAPI based on device node doesn't fit well.
> > 
> > An anonymous inode based approach is introduced to address the issues
> > above. Rather than generating device node and opening it, rpmsg code just
> > creates an anonymous inode representing eptdev and return the fd to
> > userspace.
> > 
> > Performance demo
> > 
> > A simple C application is tested to verify performance of new uAPI.
> > Please be noted that all '#' in code are preceded with space to suppress
> > checkpatch complaints.
> > 
> > $ cat test.c
> > 
> >  #include <linux/rpmsg.h>
> > 
> >  #include <sys/types.h>
> >  #include <sys/stat.h>
> >  #include <sys/ioctl.h>
> >  #include <fcntl.h>
> >  #include <string.h>
> >  #include <stdio.h>
> >  #include <unistd.h>
> >  #include <stdlib.h>
> >  #include <errno.h>
> >  #include <sys/time.h>
> > 
> >  #define N (1 << 20)
> > 
> > int main(int argc, char *argv[])
> > {
> > 	int ret, fd, ep_fd, loop;
> > 	struct rpmsg_endpoint_info info;
> > 	struct rpmsg_endpoint_fd_info fd_info;
> > 	struct timeval start, end;
> > 	int i = 0;
> > 	double t1, t2;
> > 
> > 	fd = -1;
> > 	ep_fd = -1;
> > 	loop = N;
> > 
> > 	if (argc == 1) {
> > 		loop = N;
> > 	} else if (argc > 1) {
> > 		loop = atoi(argv[1]);
> > 	}
> > 
> > 	printf("loop[%d]\n", loop);
> > 
> > 	strcpy(info.name, "epx");
> > 	info.src = -1;
> > 	info.dst = -1;
> > 
> > 	strcpy(fd_info.name, "epx");
> > 	fd_info.src = -1;
> > 	fd_info.dst = -1;
> > 	fd_info.fd = -1;
> > 
> > 	while (fd < 0) {
> > 		fd = open("/dev/rpmsg_ctrl0", O_RDWR);
> > 		if (fd < 0) {
> > 			printf("open rpmsg_ctrl0 failed, fd[%d]\n", fd);
> > 		}
> > 	}
> > 
> > 	gettimeofday(&start, NULL);
> > 
> > 	while (loop--) {
> > 		ret = ioctl(fd, RPMSG_CREATE_EPT_IOCTL, &info);
> > 		if (ret < 0) {
> > 			printf("ioctl[RPMSG_CREATE_EPT_IOCTL] failed,
> > 			ret[%d]\n", ret);
> > 		}
> > 
> > 		ep_fd = -1;
> > 		i = 0;
> > 
> > 		while (ep_fd < 0) {
> > 			ep_fd = open("/dev/rpmsg0", O_RDWR);
> > 			if (ep_fd < 0) {
> > 				i++;
> > 				printf("open rpmsg0 failed, epfd[%d]\n", ep_fd);
> > 			}
> > 		}
> > 
> > 		ret = ioctl(ep_fd, RPMSG_DESTROY_EPT_IOCTL, &info);
> > 		if (ret < 0) {
> > 			printf("old RPMSG_DESTROY_EPT_IOCTL failed, ret[%d], errno[%d]\n",
> > 			ret, errno);
> > 		}
> > 
> > 		close(ep_fd);
> > 	}
> > 	
> > 	gettimeofday(&end, NULL);
> > 
> > 	printf("time for old way: [%ld] us\n",
> > 		1000000 * (end.tv_sec - start.tv_sec) + end.tv_usec - start.tv_usec);
> > 	t1 = 1000000 * (end.tv_sec - start.tv_sec) + end.tv_usec - start.tv_usec;
> > 
> > 	if (argc == 1) {
> > 		loop = N;
> > 	} else if (argc > 1) {
> > 		loop = atoi(argv[1]);
> > 	}
> > 
> > 	printf("loop[%d]\n", loop);
> > 
> > 	gettimeofday(&start, NULL);
> > 
> > 	while (loop--) {
> > 		fd_info.fd = -1;
> > 		fd_info.flags = O_RDWR | O_CLOEXEC | O_NONBLOCK;
> > 		ret = ioctl(fd, RPMSG_CREATE_EPT_FD_IOCTL, &fd_info);
> > 		if (ret < 0 || fd_info.fd < 0) {
> > 			printf("ioctl[RPMSG_CREATE_EPT_FD_IOCTL] failed, ret[%d]\n", ret);
> > 		}
> > 
> > 		ret = ioctl(fd_info.fd, RPMSG_DESTROY_EPT_IOCTL, &info);
> > 		if (ret < 0) {
> > 			printf("new ioctl[RPMSG_DESTROY_EPT_IOCTL] failed, ret[%d]\n", ret);
> > 		}
> > 
> > 		close(fd_info.fd);
> > 	}
> > 	
> > 	gettimeofday(&end, NULL);
> > 
> > 	printf("time for new way: [%ld] us\n",
> > 	1000000 * (end.tv_sec - start.tv_sec) + end.tv_usec - start.tv_usec);
> > 	t2 = 1000000 * (end.tv_sec - start.tv_sec) + end.tv_usec - start.tv_usec;
> > 
> > 	printf("t1(old) / t2(new) = %f\n", t1 / t2);
> > 
> > 	close(fd);
> > }
> > 
> > Performance benchmark
> > 
> > - Legacy means benchmark based on old uAPI
> > - New means benchmark based on new uAPI(the one this series introduce)
> > - Time are in units of us(10^-6 s)
> > 
> > Test	loops	Total time(legacy)	Total time(new)	legacy/new	
> > 1	1000	148362			1153		128.674761	
> > 2	1000	145640			1121		129.919715	
> > 3	1000	145303			1174		123.767462	
> > 4	1000	150294			1142		131.605954	
> > 5	1000	160877			1175		136.916596	
> > 6	1000	154400			1134		136.155203	
> > 7	1000	143252			1163		123.174549	
> > 8	1000	148758			1161		128.129199
> > 9	1000	149044			1112		134.032374
> > 10	1000	146895			1192		123.234060
> > 11	10000	1428967			11627		122.900748
> > 12	10000	1367015			10557		129.488965
> > 13	10000	1371919			11663		117.630027
> > 14	10000	1358447			11080		122.603520
> > 15	10000	1375463			11245		122.317741
> > 16	10000	1364901			11153		122.379718
> > 17	10000	1352665			10735		126.005123
> > 18	10000	1400873			11341		123.522882
> > 19	10000	1391276			10892		127.733750
> > 20	10000	1394367			11110		125.505581
> > 21	100000	14069671		115569		121.742604
> > 22	100000	13663364		117074		116.707074
> > 23	100000	13735740		115638		118.782234
> > 24	100000	13714441		119362		114.897882
> > 25	100000	13904366		118282		117.552679
> > 26	100000	13870560		117717		117.829710
> > 27	100000	13713605		118312		115.910516
> > 28	100000	13872852		114347		121.322396
> > 29	100000	13777964		119072		115.711200
> > 30	100000	13725654		116296		118.023440
> > 
> > Changelog:
> > 
> > Changes in v5:
> > - Rebased on v6.18.rc1.
> > - Fix checkpatch warning on commit msg on patch[1/3].
> > - Other minor commit msg tweaks.
> > - Update performance testing results.
> > - Link to v4:
> >   https://lore.kernel.org/all/20250609151531.22621-1-dawei.li@linux.dev/
> > 
> > Changes in v4:
> > - Build warning of copy_to_user (Dan).
> > - ioctl() branches reorder (Beleswar).
> > - Remove local variable fd and pass &ept_fd_info.fd to
> >   rpmsg_anonymous_eptdev_create().
> > - Link to v3:
> >   https://lore.kernel.org/all/20250519150823.62350-1-dawei.li@linux.dev/
> > 
> > Changes in v3:
> > - s/anon/anonymous (Mathieu)
> > - API naming adjustment (Mathieu)
> >   - __rpmsg_chrdev_eptdev_alloc ->  rpmsg_eptdev_alloc
> >   - __rpmsg_chrdev_eptdev_add ->  rpmsg_eptdev_add
> > - Add parameter 'flags' to uAPI so user can specify file flags
> >   explicitly on creating anonymous inode.
> > - Link to v2:
> >   https://lore.kernel.org/all/20250509155927.109258-1-dawei.li@linux.dev/
> > 
> > Changes in v2:
> > - Fix compilation error for !CONFIG_RPMSG_CHAR config(Test robot).
> > - Link to v1:
> >   https://lore.kernel.org/all/20250507141712.4276-1-dawei.li@linux.dev/
> > 
> > Dawei Li (3):
> >   rpmsg: char: Reuse eptdev logic for anonymous device
> >   rpmsg: char: Implement eptdev based on anonymous inode
> >   rpmsg: ctrl: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI
> > 
> >  drivers/rpmsg/rpmsg_char.c | 129 ++++++++++++++++++++++++++++++-------
> >  drivers/rpmsg/rpmsg_char.h |  23 +++++++
> >  drivers/rpmsg/rpmsg_ctrl.c |  35 ++++++++--
> >  include/uapi/linux/rpmsg.h |  27 +++++++-
> >  4 files changed, 182 insertions(+), 32 deletions(-)
> > 
> > ---
> > 
> > Thanks,
> > 
> > 	Dawei
> > 
> > -- 
> > 2.25.1
> > 

Thanks,

	Dawei

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 0/3] rpmsg: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI
  2025-10-16 16:28   ` Dawei Li
@ 2025-10-17 14:47     ` Mathieu Poirier
  0 siblings, 0 replies; 7+ messages in thread
From: Mathieu Poirier @ 2025-10-17 14:47 UTC (permalink / raw)
  To: Dawei Li; +Cc: andersson, linux-remoteproc, linux-kernel, set_pte_at

On Fri, Oct 17, 2025 at 12:28:44AM +0800, Dawei Li wrote:
> Hi, Mathieu, 
> 
> On Thu, Oct 16, 2025 at 09:32:04AM -0600, Mathieu Poirier wrote:
> > I have applied this set.
> > 
> > Thanks,
> > Mathieu
> 
> It seems that it is v4 being applied? [1]

I don't know how it came to that but it is fixed now.

Thanks for bringing this to my attention,
Mathieu

> 
> The only difference between v4 and v5 is commit message, though.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git/commit/?h=rpmsg-next&id=00af63201cbb7903e5deb2a9fdebd97f979492e5
> 
> > 
> > On Wed, Oct 15, 2025 at 11:17:15PM +0800, Dawei Li wrote:
> > > Hi,
> > > 
> > > This is V5 of series which introduce new uAPI(RPMSG_CREATE_EPT_FD_IOCTL)
> > > for rpmsg subsystem.
> > > 
> > > Current uAPI implementation for rpmsg ctrl & char device manipulation is
> > > abstracted in procedures below:
> > > - fd = open("/dev/rpmsg_ctrlX")
> > > - ioctl(fd, RPMSG_CREATE_EPT_IOCTL, &info); /dev/rpmsgY devnode is
> > >   generated.
> > > - fd_ep = open("/dev/rpmsgY", O_RDWR)
> > > - operations on fd_ep(write, read, poll ioctl)
> > > - ioctl(fd_ep, RPMSG_DESTROY_EPT_IOCTL)
> > > - close(fd_ep)
> > > - close(fd)
> > > 
> > > This /dev/rpmsgY abstraction is less favorable for:
> > > - Performance issue: It's time consuming for some operations are
> > > involved:
> > >   - Device node creation.
> > >     Depends on specific config, especially CONFIG_DEVTMPFS, the overall
> > >     overhead is based on coordination between DEVTMPFS and userspace
> > >     tools such as udev and mdev.
> > >   - Extra kernel-userspace switch cost.
> > >   - Other major costs brought by heavy-weight logic like device_add().
> > > 
> > > - /dev/rpmsgY node can be opened only once. It doesn't make much sense
> > >     that a dynamically created device node can be opened only once.
> > > 
> > > - For some container application such as docker, a client can't access
> > >   host's dev unless specified explicitly. But in case of /dev/rpmsgY, which
> > >   is generated dynamically and whose existence is unknown for clients in
> > >   advance, this uAPI based on device node doesn't fit well.
> > > 
> > > An anonymous inode based approach is introduced to address the issues
> > > above. Rather than generating device node and opening it, rpmsg code just
> > > creates an anonymous inode representing eptdev and return the fd to
> > > userspace.
> > > 
> > > Performance demo
> > > 
> > > A simple C application is tested to verify performance of new uAPI.
> > > Please be noted that all '#' in code are preceded with space to suppress
> > > checkpatch complaints.
> > > 
> > > $ cat test.c
> > > 
> > >  #include <linux/rpmsg.h>
> > > 
> > >  #include <sys/types.h>
> > >  #include <sys/stat.h>
> > >  #include <sys/ioctl.h>
> > >  #include <fcntl.h>
> > >  #include <string.h>
> > >  #include <stdio.h>
> > >  #include <unistd.h>
> > >  #include <stdlib.h>
> > >  #include <errno.h>
> > >  #include <sys/time.h>
> > > 
> > >  #define N (1 << 20)
> > > 
> > > int main(int argc, char *argv[])
> > > {
> > > 	int ret, fd, ep_fd, loop;
> > > 	struct rpmsg_endpoint_info info;
> > > 	struct rpmsg_endpoint_fd_info fd_info;
> > > 	struct timeval start, end;
> > > 	int i = 0;
> > > 	double t1, t2;
> > > 
> > > 	fd = -1;
> > > 	ep_fd = -1;
> > > 	loop = N;
> > > 
> > > 	if (argc == 1) {
> > > 		loop = N;
> > > 	} else if (argc > 1) {
> > > 		loop = atoi(argv[1]);
> > > 	}
> > > 
> > > 	printf("loop[%d]\n", loop);
> > > 
> > > 	strcpy(info.name, "epx");
> > > 	info.src = -1;
> > > 	info.dst = -1;
> > > 
> > > 	strcpy(fd_info.name, "epx");
> > > 	fd_info.src = -1;
> > > 	fd_info.dst = -1;
> > > 	fd_info.fd = -1;
> > > 
> > > 	while (fd < 0) {
> > > 		fd = open("/dev/rpmsg_ctrl0", O_RDWR);
> > > 		if (fd < 0) {
> > > 			printf("open rpmsg_ctrl0 failed, fd[%d]\n", fd);
> > > 		}
> > > 	}
> > > 
> > > 	gettimeofday(&start, NULL);
> > > 
> > > 	while (loop--) {
> > > 		ret = ioctl(fd, RPMSG_CREATE_EPT_IOCTL, &info);
> > > 		if (ret < 0) {
> > > 			printf("ioctl[RPMSG_CREATE_EPT_IOCTL] failed,
> > > 			ret[%d]\n", ret);
> > > 		}
> > > 
> > > 		ep_fd = -1;
> > > 		i = 0;
> > > 
> > > 		while (ep_fd < 0) {
> > > 			ep_fd = open("/dev/rpmsg0", O_RDWR);
> > > 			if (ep_fd < 0) {
> > > 				i++;
> > > 				printf("open rpmsg0 failed, epfd[%d]\n", ep_fd);
> > > 			}
> > > 		}
> > > 
> > > 		ret = ioctl(ep_fd, RPMSG_DESTROY_EPT_IOCTL, &info);
> > > 		if (ret < 0) {
> > > 			printf("old RPMSG_DESTROY_EPT_IOCTL failed, ret[%d], errno[%d]\n",
> > > 			ret, errno);
> > > 		}
> > > 
> > > 		close(ep_fd);
> > > 	}
> > > 	
> > > 	gettimeofday(&end, NULL);
> > > 
> > > 	printf("time for old way: [%ld] us\n",
> > > 		1000000 * (end.tv_sec - start.tv_sec) + end.tv_usec - start.tv_usec);
> > > 	t1 = 1000000 * (end.tv_sec - start.tv_sec) + end.tv_usec - start.tv_usec;
> > > 
> > > 	if (argc == 1) {
> > > 		loop = N;
> > > 	} else if (argc > 1) {
> > > 		loop = atoi(argv[1]);
> > > 	}
> > > 
> > > 	printf("loop[%d]\n", loop);
> > > 
> > > 	gettimeofday(&start, NULL);
> > > 
> > > 	while (loop--) {
> > > 		fd_info.fd = -1;
> > > 		fd_info.flags = O_RDWR | O_CLOEXEC | O_NONBLOCK;
> > > 		ret = ioctl(fd, RPMSG_CREATE_EPT_FD_IOCTL, &fd_info);
> > > 		if (ret < 0 || fd_info.fd < 0) {
> > > 			printf("ioctl[RPMSG_CREATE_EPT_FD_IOCTL] failed, ret[%d]\n", ret);
> > > 		}
> > > 
> > > 		ret = ioctl(fd_info.fd, RPMSG_DESTROY_EPT_IOCTL, &info);
> > > 		if (ret < 0) {
> > > 			printf("new ioctl[RPMSG_DESTROY_EPT_IOCTL] failed, ret[%d]\n", ret);
> > > 		}
> > > 
> > > 		close(fd_info.fd);
> > > 	}
> > > 	
> > > 	gettimeofday(&end, NULL);
> > > 
> > > 	printf("time for new way: [%ld] us\n",
> > > 	1000000 * (end.tv_sec - start.tv_sec) + end.tv_usec - start.tv_usec);
> > > 	t2 = 1000000 * (end.tv_sec - start.tv_sec) + end.tv_usec - start.tv_usec;
> > > 
> > > 	printf("t1(old) / t2(new) = %f\n", t1 / t2);
> > > 
> > > 	close(fd);
> > > }
> > > 
> > > Performance benchmark
> > > 
> > > - Legacy means benchmark based on old uAPI
> > > - New means benchmark based on new uAPI(the one this series introduce)
> > > - Time are in units of us(10^-6 s)
> > > 
> > > Test	loops	Total time(legacy)	Total time(new)	legacy/new	
> > > 1	1000	148362			1153		128.674761	
> > > 2	1000	145640			1121		129.919715	
> > > 3	1000	145303			1174		123.767462	
> > > 4	1000	150294			1142		131.605954	
> > > 5	1000	160877			1175		136.916596	
> > > 6	1000	154400			1134		136.155203	
> > > 7	1000	143252			1163		123.174549	
> > > 8	1000	148758			1161		128.129199
> > > 9	1000	149044			1112		134.032374
> > > 10	1000	146895			1192		123.234060
> > > 11	10000	1428967			11627		122.900748
> > > 12	10000	1367015			10557		129.488965
> > > 13	10000	1371919			11663		117.630027
> > > 14	10000	1358447			11080		122.603520
> > > 15	10000	1375463			11245		122.317741
> > > 16	10000	1364901			11153		122.379718
> > > 17	10000	1352665			10735		126.005123
> > > 18	10000	1400873			11341		123.522882
> > > 19	10000	1391276			10892		127.733750
> > > 20	10000	1394367			11110		125.505581
> > > 21	100000	14069671		115569		121.742604
> > > 22	100000	13663364		117074		116.707074
> > > 23	100000	13735740		115638		118.782234
> > > 24	100000	13714441		119362		114.897882
> > > 25	100000	13904366		118282		117.552679
> > > 26	100000	13870560		117717		117.829710
> > > 27	100000	13713605		118312		115.910516
> > > 28	100000	13872852		114347		121.322396
> > > 29	100000	13777964		119072		115.711200
> > > 30	100000	13725654		116296		118.023440
> > > 
> > > Changelog:
> > > 
> > > Changes in v5:
> > > - Rebased on v6.18.rc1.
> > > - Fix checkpatch warning on commit msg on patch[1/3].
> > > - Other minor commit msg tweaks.
> > > - Update performance testing results.
> > > - Link to v4:
> > >   https://lore.kernel.org/all/20250609151531.22621-1-dawei.li@linux.dev/
> > > 
> > > Changes in v4:
> > > - Build warning of copy_to_user (Dan).
> > > - ioctl() branches reorder (Beleswar).
> > > - Remove local variable fd and pass &ept_fd_info.fd to
> > >   rpmsg_anonymous_eptdev_create().
> > > - Link to v3:
> > >   https://lore.kernel.org/all/20250519150823.62350-1-dawei.li@linux.dev/
> > > 
> > > Changes in v3:
> > > - s/anon/anonymous (Mathieu)
> > > - API naming adjustment (Mathieu)
> > >   - __rpmsg_chrdev_eptdev_alloc ->  rpmsg_eptdev_alloc
> > >   - __rpmsg_chrdev_eptdev_add ->  rpmsg_eptdev_add
> > > - Add parameter 'flags' to uAPI so user can specify file flags
> > >   explicitly on creating anonymous inode.
> > > - Link to v2:
> > >   https://lore.kernel.org/all/20250509155927.109258-1-dawei.li@linux.dev/
> > > 
> > > Changes in v2:
> > > - Fix compilation error for !CONFIG_RPMSG_CHAR config(Test robot).
> > > - Link to v1:
> > >   https://lore.kernel.org/all/20250507141712.4276-1-dawei.li@linux.dev/
> > > 
> > > Dawei Li (3):
> > >   rpmsg: char: Reuse eptdev logic for anonymous device
> > >   rpmsg: char: Implement eptdev based on anonymous inode
> > >   rpmsg: ctrl: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI
> > > 
> > >  drivers/rpmsg/rpmsg_char.c | 129 ++++++++++++++++++++++++++++++-------
> > >  drivers/rpmsg/rpmsg_char.h |  23 +++++++
> > >  drivers/rpmsg/rpmsg_ctrl.c |  35 ++++++++--
> > >  include/uapi/linux/rpmsg.h |  27 +++++++-
> > >  4 files changed, 182 insertions(+), 32 deletions(-)
> > > 
> > > ---
> > > 
> > > Thanks,
> > > 
> > > 	Dawei
> > > 
> > > -- 
> > > 2.25.1
> > > 
> 
> Thanks,
> 
> 	Dawei

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-10-17 14:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-15 15:17 [PATCH v5 0/3] rpmsg: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI Dawei Li
2025-10-15 15:17 ` [PATCH v5 1/3] rpmsg: char: Reuse eptdev logic for anonymous device Dawei Li
2025-10-15 15:17 ` [PATCH v5 2/3] rpmsg: char: Implement eptdev based on anonymous inode Dawei Li
2025-10-15 15:17 ` [PATCH v5 3/3] rpmsg: ctrl: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI Dawei Li
2025-10-16 15:32 ` [PATCH v5 0/3] rpmsg: " Mathieu Poirier
2025-10-16 16:28   ` Dawei Li
2025-10-17 14:47     ` Mathieu Poirier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox