casaroli opened a new pull request, #21145:
URL: https://github.com/apache/nuttx/pull/21145

   ## Summary
   
   This PR fixes SysV shared memory (`CONFIG_MM_SHM`, kernel builds) and adds 
the key `IPC_PRIVATE`. On master, a user program of a kernel build that calls 
`shmget()` or `shmat()` does not link, and a large shared memory area makes a 
kernel that does not link.
   
   - **mm: Build the shared memory and VMA code only into the kernel.** In a 
kernel build, `mm/` is built twice: for the kernel and for the user-space 
library. `mm/shm/*.c` and `mm/map/vm_region.c` had no guard, so the user 
`libmm` also got the kernel functions. A user program links with `-lmm -lc 
-lproxies`, so it took the kernel code from `libmm` instead of the system call 
proxies, and the link failed (`undefined reference to mm_map_add`, 
`mm_map_find`, `nxsched_self`, `up_shmat`, ...). Now these files are compiled 
only for the kernel (`mm/map/vm_region.c` also for a flat build), as 
`mm/map/mm_map.c` does.
   - **mm/shm: Allocate the page list of a region from the kernel heap.** Each 
region had a static page list of `CONFIG_ARCH_SHM_NPAGES` entries. That option 
is also the size of the mapping area of each process, so a large area made a 
large table: with 65536 pages (256 MiB), `g_shminfo` was 524400 bytes for one 
region, in `.data`. With 32 regions qemu-armv8a:knsh did not link (`region 
'dram' overflowed by 14991360 bytes`). Now `shm_extend()` allocates the page 
list with the size of the request, and `shm_destroy()` frees it. A request 
larger than `CONFIG_ARCH_SHM_NPAGES` pages fails with `EINVAL` (before: it 
allocated `CONFIG_ARCH_SHM_NPAGES` pages, then failed with `ENOMEM`).
   - **mm/shm: Return EINVAL for an invalid shared memory identifier.** 
`shmat()` and `shmctl()` checked the `shmid` from user space only with 
`DEBUGASSERT()`. With assertions, a bad identifier stopped the kernel; without 
them, the kernel used an entry outside the table or a free entry.
   - **mm/shm: Create a new region for the key IPC_PRIVATE.** 
`shmget(IPC_PRIVATE, ...)` failed with `ENOSYS`. POSIX says that `IPC_PRIVATE` 
always creates a new segment. Now it creates a new region every time, also 
without `IPC_CREAT`, and no key lookup finds it (a free region also has the key 
0).
   - One whitespace commit first fixes an older nxstyle error in `shmctl.c`, 
and one commit updates `Documentation/os/memory/shm.rst` (the meaning of 
`CONFIG_ARCH_SHM_MAXREGIONS` and `CONFIG_ARCH_SHM_NPAGES`).
   
   ## Impact
   
   Only `CONFIG_MM_SHM=y`, which depends on `CONFIG_BUILD_KERNEL`. Flat and 
protected builds do not build this code, so there is no change for them. 
`struct shm_region_s` is internal.
   
   Memory: the static cost is now about 96 bytes per region and does not depend 
on `CONFIG_ARCH_SHM_NPAGES`. On qemu-armv8a:knsh (`nm -S`), `g_shminfo` is 120 
bytes with 1 region and 3096 bytes with 32 regions. On master with 
`CONFIG_ARCH_SHM_NPAGES=65536` it is 524400 bytes with 1 region, in the kernel 
and again in the user `libmm`, and 16780056 bytes with 32 regions. The page 
list of a region comes from the kernel heap (8 bytes per page on 64-bit 
targets). The user `libmm` has no shared memory code now.
   
   Behaviour: `shmget()` with a size above `CONFIG_ARCH_SHM_NPAGES` pages fails 
with `EINVAL` (was `ENOMEM`); `shmat()`/`shmctl()` of an invalid identifier 
fail with `EINVAL` (was an assertion); `shmget(IPC_PRIVATE, ...)` works (was 
`ENOSYS`).
   
   ## Testing
   
   Test programs `shmsysv.c` and `shmprivate.c` on qemu-armv8a:knsh (kernel 
build, QEMU with HVF) with `CONFIG_MM_SHM=y`, 
`CONFIG_ARCH_SHM_VBASE=0xF0000000`, `CONFIG_ARCH_SHM_NPAGES=65536`, 
`CONFIG_ARCH_SHM_MAXREGIONS=32`:
   
   | Case | master | this PR |
   |---|---|---|
   | the kernel links (32 regions, 65536 pages) | no: `region 'dram' overflowed 
by 14999552 bytes` | yes |
   | a program that calls `shmget()`/`shmat()` links (master with 1024 pages, 
so that the kernel links) | no: undefined `mm_map_add`, `mm_map_find`, 
`mm_map_lock`, `mm_map_remove`, `mm_map_unlock`, `nxsched_self`, `up_shmat`, 
`up_shmdt` | yes |
   | 1 `shmget(IPC_CREAT)` + `shmat()` of 3 pages | - | PASS |
   | 2 `IPC_STAT`: `shm_segsz`, `shm_nattch` | - | PASS |
   | 3 a second process attaches the key and sees the data | - | PASS |
   | 4 `shmdt()` + `IPC_RMID`, then `ENOENT` | - | PASS |
   | 5 64-page region, 20 create/remove cycles | - | PASS |
   | 6 `shmat(1000)`, `shmctl(-1)`, `shmctl()` of a free id: `EINVAL` | - | 
PASS |
   | 8 a `fork()` child attaches the key and writes | - | PASS |
   | 9 32 regions, the 33rd fails with `ENOSPC` | - | PASS |
   | `shmprivate`: two `shmget(IPC_PRIVATE)` give two ids (the second without 
`IPC_CREAT`) | - | PASS (without the last commit: `ENOSYS`) |
   | `shmprivate`: a `fork()` child writes through the segment, the parent 
reads it | - | PASS |
   | `shmprivate`: a key lookup does not find a private segment (`ENOENT`) | - 
| PASS |
   | `shmprivate`: `IPC_RMID` of both | - | PASS |
   
   ostest exits with status 0 on qemu-armv8a:knsh (with the options above) and 
qemu-armv8a:nsh, on master and with this PR.
   
   Build, each OK: qemu-armv8a:knsh with `CONFIG_MM_SHM` (the default area and 
the options above); icicle:knsh (kernel, rv64, has `CONFIG_MM_SHM=y`); 
rv-virt:knsh64 (kernel, rv64), also with `CONFIG_MM_SHM` and with 
`CONFIG_FS_SHMFS` (both with `CONFIG_ARCH_SHM_VBASE=0xC2000000`); 
qemu-armv8a:nsh (flat, arm64); mps2-an500:nsh (flat, Cortex-M7); 
mps2-an500:knsh (protected, Cortex-M7).
   
   `tools/checkpatch.sh -c -u -m -g` passes.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to