All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dawei Li <dawei.li@linux.dev>
To: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: andersson@kernel.org, linux-remoteproc@vger.kernel.org,
	linux-kernel@vger.kernel.org, set_pte_at@outlook.com,
	dawei.li@linux.dev
Subject: Re: [PATCH v5 0/3] rpmsg: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI
Date: Fri, 17 Oct 2025 00:28:44 +0800	[thread overview]
Message-ID: <20251016162844.GA2725@wendao-VirtualBox> (raw)
In-Reply-To: <aPEP9EdZ8tIAVwyE@p14s>

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

  reply	other threads:[~2025-10-16 16:29 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2025-10-17 14:47     ` Mathieu Poirier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251016162844.GA2725@wendao-VirtualBox \
    --to=dawei.li@linux.dev \
    --cc=andersson@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=mathieu.poirier@linaro.org \
    --cc=set_pte_at@outlook.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.