casaroli opened a new pull request, #21421: URL: https://github.com/apache/nuttx/pull/21421
## Summary Fix eight deviations from POSIX and defects in the System V message queues, one commit each: - `msgget()` did not keep the key. It used the key as a queue identifier (the table index plus one). So `msgget(key, 0)` after `msgget(key, IPC_CREAT)` gave `ENOENT`, `IPC_CREAT | IPC_EXCL` never gave `EEXIST`, and `msgget(1, 0)` found any queue with the identifier 1. The queue now keeps its key, separate from its identifier. `IPC_PRIVATE` always makes a new queue. - `msgrcv()` with `msgtyp < 0` took the lowest type of the whole queue. It now takes the first message of the lowest type that is less than or equal to `|msgtyp|`. If no message fits, it blocks, or gives `ENOMSG` with `IPC_NOWAIT`. - `msgctl(IPC_STAT)` did not set `msg_perm`, `msg_stime`, `msg_rtime`, `msg_ctime`, `msg_lspid` and `msg_lrpid`, and `msg_cbytes` was the number of messages times `CONFIG_MQ_MAXMSGSIZE`. The queue now keeps these values: `msgget()` sets the mode and `msg_ctime`, `msgsnd()` and `msgrcv()` set the PID and the time, `IPC_SET` sets the mode and `msg_ctime`, and `msg_cbytes` counts the bytes in the queue. - `msgctl(IPC_SET)` changed `msg_qbytes` into a number of whole messages (33 became 32) and stored it in an `int16_t` without a check. `msg_qbytes` is now a byte limit: `msgsnd()` blocks, or gives `EAGAIN` with `IPC_NOWAIT`, when the message would make the queue larger than the limit. `IPC_STAT` reports `msg_qbytes` as `IPC_SET` set it, and `IPC_SET` gives `EINVAL` for 0. The default stays 16 messages of the maximum size. `msgrcv()` and `IPC_SET` wake up all waiting senders, because the free room can be enough for more than one smaller message. - `msgsnd()` in an interrupt handler with a full queue and without `IPC_NOWAIT` added the message beyond the limit. It now gives `EAGAIN`, as `mq_send()` does. - `msgsnd()` woke up only the first waiting receiver. If that receiver waited for another type, it blocked again, and a receiver that wanted the message stayed blocked while the message was in the queue. `msgsnd()` now wakes up all receivers, and each one checks again for a message of its type. - The size of the queue table was a `uint8_t` that grows by 10. With more than 250 queues it wrapped to 4: the queues above 4 gave `EINVAL`, and the next growth made the table smaller and cleared live entries. The size is now an `int`, and `msgget()` gives `ENOSPC` before the identifier passes `INT16_MAX`. - When the system-wide pool of `CONFIG_PREALLOC_MQ_MSGS` messages was empty, `msgsnd()` gave `ENOMEM`. It now waits for a free message, or gives `EAGAIN` with `IPC_NOWAIT` or in an interrupt handler. The waiters use a pool wait list (a `struct mqueue_cmn_s` with the state `TSTATE_WAIT_MQNOTFULL`), so a signal, a cancellation or a task restart removes them as for a queue. `msgrcv()` and `IPC_RMID` wake them up. After the wake-up, `msgsnd()` checks again that its queue exists (`EIDRM`) and has room. ## Impact Only the System V message queues (`sched/mqueue/msg*.c`) change, when `CONFIG_DISABLE_MQUEUE_SYSV` is not set. `struct msqid_ds`, `struct ipc_perm` and the system calls do not change. No in-tree code calls `msgget()`, `msgsnd()`, `msgrcv()` or `msgctl()`. Results that a program sees in all builds, flat too: `msgget()` of an existing key now returns that queue (was `ENOENT` or a new queue), and `IPC_CREAT | IPC_EXCL` gives `EEXIST`; `msgrcv()` with `msgtyp < 0` can now block or give `ENOMSG`; `IPC_STAT` fills all fields; `msg_qbytes` is in bytes and `IPC_SET` gives `EINVAL` for 0; `msgsnd()` from an interrupt handler with a full queue gives `EAGAIN`; `msgsnd()` with an empty message pool blocks, or gives `EAGAIN` with `IPC_NOWAIT` (was `ENOMEM`); `msgget()` can give `ENOSPC` at the table limit. The queue limit is now in bytes, so a queue of small messages can hold more than 16 messages. The system-wide pool of `CONFIG_PREALLOC_MQ_MSGS` messages stays the limit of the number of messages. Its default (4 or 8) is smaller than 16, so the old limit of 16 messages per queue had no effect in the default configuration. With `CONFIG_PREALLOC_MQ_MSGS=0` (no defconfig sets it), `msgsnd()` now blocks, where it gave `ENOMEM`. Size: `struct msgq_s` (kernel memory, one per queue, allocated by `msgget()`) grows from 40 to 80 bytes on 32-bit targets and from 72 to 128 bytes on 64-bit targets: the three times, the two PIDs, `struct ipc_perm` and the two byte counters. The code of `sched/mqueue/msg*.o` grows from 4454 to 4755 bytes of text on mps2-an500:nsh (+301) and from 2768 to 3277 bytes on qemu-armv8a:nsh (+509). The pool wait list and the wider table counter add 23 bytes (32-bit) and 43 bytes (64-bit) of BSS. ## Testing Test: the cmocka test `cmocka_msg_test` (nuttx-apps testing/testsuites, `CONFIG_CM_MSG_TEST`; the apps PR follows) with 25 cases. 15 cases pass on master. 10 cases test these bugs: `get_key`, `type_negative`, `ctl_fields`, `ctl_set`, `wait_types` (two receivers of different types), `pool_nowait`, `pool_wait` (blocked by an empty pool, woken by `msgrcv()` of another queue), `pool_eintr`, `pool_eidrm`, `get_many` (260 queues). ostest also runs. | Configuration | Build | master | this PR | ostest (master / PR) | |---|---|---|---|---| | qemu-armv8a:nsh (arm64, QEMU HVF) | flat | 15 PASS, 10 FAIL | 25/25 PASS | 0 / 0 | | qemu-armv8a:knsh (arm64, QEMU HVF) | kernel | 15 PASS, 10 FAIL | 25/25 PASS | 0 / 0 | | mps2-an500:knsh (Cortex-M7, QEMU TCG) | protected | 15 PASS, 10 FAIL | 25/25 PASS | 0 / 0 | | qemu-armv8a:nsh_smp (arm64, 2 CPUs, QEMU HVF), with #21393 | flat SMP | 15 PASS, 10 FAIL | 25/25 PASS | 0 / 0 | The SMP row includes the commits of #21393. Without them, ostest on master and on this PR stops in the spinlock test on this target, and #21393 fixes that hang; the msg test gives the same result without them (15 PASS, 10 FAIL on master, 25/25 PASS with this PR). Builds with `CONFIG_DISABLE_MQUEUE_SYSV` off and `EXTRAFLAGS="-Wno-cpp -Werror"`, no warnings: rv-virt:knsh64 (kernel), rv-virt:pnsh64 (protected), rv-virt:nsh (rv32 flat), mps2-an500:nsh (flat, `DEBUG_ASSERTIONS`), mps2-an500:knsh (protected), qemu-armv8a:nsh_smp (flat SMP, `DEBUG_ASSERTIONS`), qemu-armv8a:knsh (kernel). qemu-armv8a:nsh with `CONFIG_DISABLE_MQUEUE_SYSV=y` builds no `msg*.o`. Host clang (sim:nsh on macOS) compiles the five files with no warnings. `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]
