Re: [PATCH v3] futex_waitv.2: new page

[Alejandro Colomar <alx@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 6058 bytes --]

Hi,

On 2026-02-10T21:32:18+0100, наб wrote:
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> ---
>  man/man2/futex_waitv.2 | 407 +++++++++++++++++++++++++++++++++++++++++
>  man/man7/futex.7       |   9 +-
>  2 files changed, 414 insertions(+), 2 deletions(-)
>  create mode 100644 man/man2/futex_waitv.2
> 
> diff --git u/man/man2/futex_waitv.2 p/man/man2/futex_waitv.2
> new file mode 100644
> index 000000000..2ec6e4b50
> --- /dev/null
> +++ p/man/man2/futex_waitv.2
> @@ -0,0 +1,407 @@
[...]

> +.SH EXAMPLES
> +The program below executes a linear-time operation on 10 threads,
> +displaying the results in real time,
> +waiting at most 1 second for each new result.
> +The first 3 threads operate on the same data (complete in the same time).
> +.B !\&
> +indicates the futex that woke up each
> +.BR futex_waitv ().
> +.in +4
> +.EX
> +.RB $\~ ./futex_waitv
> +153	153	153	237	100	245	177	127	215	61
> +									122!
> +				200!
> +							254!
> +306	306!
> +		306!
> +						354!
> +								430!
> +			474!
> +					490!
> +Connection timed out
> +.EE
> +.P
> +.EX

Please wrap the program in comments so that it's checked by the build
system.  See for example:

	eventfd.2:381:.\" SRC BEGIN (eventfd.c)
	eventfd.2:436:.\" SRC END

> +#include <errno.h>
> +#include <inttypes.h>
> +#include <linux/futex.h>
> +#include <pthread.h>
> +#include <stdatomic.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/syscall.h>
> +#include <time.h>
> +#include <unistd.h>
> +\&
> +void *
> +worker(void *arg)
> +{
> +	_Atomic uint32_t *futex = arg;

Please use two spaces to separate the type from the identifier:

	_Atomic uint32_t  *futex;

> +\&
> +	usleep(*futex * 10000);
> +	*futex *= 2;
> +	syscall(SYS_futex, futex, FUTEX_WAKE_PRIVATE, 1);

Is this allowed?  FUTEX_WAIT(2const) documents a 5th parameter.

     long syscall(SYS_futex, uint32_t *uaddr, FUTEX_WAIT, uint32_t val,
                  const struct timespec *_Nullable timeout);

You may get away because uninitialized values get a NULL often, but
is this reliable?  And even if it were reliable on glibc, would it be
good to use it in a manual page?

Also, please define a wrapper function futex_wait_private() that has the
following prototype:

	static inline long
	my_futex_wait_private(uint32_t *uaddr, uint32_t val,
	                      const struct timespec *_Nullable timeout);

> +	return NULL;
> +}
> +\&
> +int
> +main(void)
> +{
> +#define WORKERS 10
> +	_Atomic uint32_t futexes[WORKERS];
> +\&
> +	uint8_t init[WORKERS];

Please separate declarations from code with a blank line (kernel style).

> +	getentropy(init, sizeof(init));
> +	init[0] = init[1] = init[2];
> +	for (int i = 0; i < WORKERS; ++i) {

I'd prefer using countof() in things like this.

> +		printf("%" PRIu8 "\[rs]t", init[i]);
> +		atomic_init(&futexes[i], init[i]);
> +		pthread_create(&(pthread_t){}, NULL, worker, &futexes[i]);

I've now pushed a patch documenting _Nullable in pthread_create(3).

> +	}
> +	putchar('\[rs]n');
> +\&
> +	struct futex_waitv waiters[WORKERS] = {};
> +	for (int i = 0; i < WORKERS; ++i) {
> +		waiters[i].val   = futexes[i];
> +		waiters[i].uaddr = (uintptr_t)&futexes[i];
> +		waiters[i].flags = FUTEX2_SIZE_U32 | FUTEX2_PRIVATE;
> +	}
> +	for (;;) {
> +		struct timespec timeout;
> +		clock_gettime(CLOCK_MONOTONIC, &timeout);
> +		timeout.tv_sec += 1;
> +\&
> +		int woke = syscall(SYS_futex_waitv, waiters, WORKERS, 0, &timeout, CLOCK_MONOTONIC);

Please write a my_futex_waitv() static inline wrapper.

Please separate declarations from code, and avoid initializations except
when really needed.

> +		if (woke == -1 && (errno != EAGAIN && errno != EWOULDBLOCK))
> +			break;
> +\&
> +		for (int i = 0; i < WORKERS; ++i) {
> +			if (futexes[i] != waiters[i].val)
> +				printf("%" PRIu32 "%.*s", futexes[i], i == woke, "!");

I'd prefer to avoid "%.*s" unless really needed.  That's something to be
used exclusively with [[gnu::nonstring]].  If I'm reading this
correctly, you really want "%s" with i==woke ? "!" : "".

> +			putchar('\[rs]t');
> +		}
> +		putchar('\[rs]n');
> +\&
> +		for (int i = 0; i < WORKERS; ++i)
> +			waiters[i].val = futexes[i];
> +	}
> +	printf("%s\[rs]n", strerror(errno));

I expect strerror(3) should go to stderr.  Do we really want stdout?


Cheers,
Alex

> +}
> +.EE
> +.SH SEE ALSO
> +.ad l
> +.BR futex (2),
> +.BR FUTEX_WAIT (2const),
> +.BR FUTEX_WAKE (2const),
> +.BR futex (7)
> +.P
> +The following kernel source files:
> +.IP \[bu]
> +.I Documentation/userspace-api/futex2.rst
> +.IP \[bu]
> +.I kernel/futex/syscall.c
> +.IP \[bu]
> +.I kernel/futex/waitwake.c
> +.IP \[bu]
> +.I kernel/futex/futex.h
> diff --git u/man/man7/futex.7 p/man/man7/futex.7
> index 51c5d5d9b..d271144ff 100644
> --- u/man/man7/futex.7
> +++ p/man/man7/futex.7
> @@ -45,7 +45,9 @@ .SS Semantics
>  Any futex operation starts in user space,
>  but it may be necessary to communicate with the kernel using the
>  .BR futex (2)
> -system call.
> +or
> +.BR futex_waitv (2)
> +system calls.
>  .P
>  To "up" a futex, execute the proper assembler instructions that
>  will cause the host CPU to atomically increment the integer.
> @@ -72,7 +74,9 @@ .SS Semantics
>  .P
>  The
>  .BR futex (2)
> -system call can optionally be passed a timeout specifying how long
> +and
> +.BR futex_waitv (2)
> +system calls can optionally be passed a timeout specifying how long
>  the kernel should
>  wait for the futex to be upped.
>  In this case, semantics are more complex and the programmer is referred
> @@ -107,6 +111,7 @@ .SH NOTES
>  .SH SEE ALSO
>  .BR clone (2),
>  .BR futex (2),
> +.BR futex_waitv (2),
>  .BR get_robust_list (2),
>  .BR set_robust_list (2),
>  .BR set_tid_address (2),
> -- 
> 2.39.5



-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]