| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aPEP9EdZ8tIAVwyE@p14s>
Date: Thu, 16 Oct 2025 09:32:04 -0600
From: Mathieu Poirier <mathieu.poirier@...aro.org>
To: Dawei Li <dawei.li@...ux.dev>
Cc: andersson@...nel.org, linux-remoteproc@...r.kernel.org,
linux-kernel@...r.kernel.org, set_pte_at@...look.com
Subject: Re: [PATCH v5 0/3] rpmsg: Introduce RPMSG_CREATE_EPT_FD_IOCTL uAPI
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
>
Powered by blists - more mailing lists