Re: [PATCH v5 0/3] rpmsg: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI

[Mathieu Poirier <mathieu.poirier@linaro.org>]

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