Re: [PATCH 06/23] futex.2: Document FUTEX_LOCK_PI2

Michael Kerrisk (man-pages)
2021-08-09T00:06:18Z

hello Kurt and Alex,

On 8/8/21 10:41 AM, Alejandro Colomar wrote:
> From: Kurt Kanzenbach <kurt@linutronix.de>
>
> FUTEX_LOCK_PI2 is a new futex operation which was recently introduced into the
> Linux kernel. It works exactly like FUTEX_LOCK_PI. However, it has support for
> selectable clocks for timeouts. By default CLOCK_MONOTONIC is used. If
> FUTEX_CLOCK_REALTIME is specified then the timeout is measured against
> CLOCK_REALTIME.
>
> This new operation addresses an inconsistency in the futex interface:
> FUTEX_LOCK_PI only works with timeouts based on CLOCK_REALTIME in contrast to
> all the other PI operations.
>
> Document the FUTEX_LOCK_PI2 command.
>
> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>

Thanks. Patch applied.

But I have a note below.

> ---
> man2/futex.2 | 53 ++++++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 49 insertions(+), 4 deletions(-)
>
> diff --git a/man2/futex.2 b/man2/futex.2
> index ada96c517..2f340e0e0 100644
> --- a/man2/futex.2
> +++ b/man2/futex.2
> @@ -241,10 +241,13 @@ and so on.
> This option bit can be employed only with the
> .BR FUTEX_WAIT_BITSET ,
> .BR FUTEX_WAIT_REQUEUE_PI ,
> -and
> (since Linux 4.5)
> .\" commit 337f13046ff03717a9e99675284a817527440a49
> -.BR FUTEX_WAIT
> +.BR FUTEX_WAIT ,
> +and
> +(since Linux v5.14.0)
> +.\" commit bf22a6976897977b0a3f1aeba6823c959fc4fdae
> +.BR FUTEX_LOCK_PI2
> operations.
> .IP
> If this option is set, the kernel measures the
> @@ -904,7 +907,9 @@ value to 0 if the previous value was the expected TID.
> If a futex is already acquired (i.e., has a nonzero value),
> waiters must employ the
> .B FUTEX_LOCK_PI
> -operation to acquire the lock.
> +or
> +.B FUTEX_LOCK_PI2
> +operations to acquire the lock.
> If other threads are waiting for the lock, then the
> .B FUTEX_WAITERS
> bit is set in the futex value;
> @@ -964,6 +969,8 @@ Note that the PI futex operations must be used as paired operations
> and are subject to some additional requirements:
> .IP * 3
> .B FUTEX_LOCK_PI
> +,
> +.B FUTEX_LOCK_PI2
> and
> .B FUTEX_TRYLOCK_PI
> pair with
> @@ -1122,6 +1129,27 @@ arguments are ignored.
> .\""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
> .\"
> .TP
> +.BR FUTEX_LOCK_PI2 " (since Linux 5.14.0)"
> +.\" commit bf22a6976897977b0a3f1aeba6823c959fc4fdae
> +This operation works similar like
> +.BR FUTEX_LOCK_PI .
> +The only difference is the
> +timeout argument.
> +.BR FUTEX_LOCK_PI2
> +has support for selectable clocks.

I think the page could help the reader by reminding them how the
clock is selected, as is done in the description of FUTEX_WAIT.

> +.IP
> +If
> +.I timeout
> +is not NULL, the structure it points to specifies
> +an absolute timeout.
> +If
> +.I timeout
> +is NULL, the operation can block indefinitely.

The above is the same as FUTEX_LOCK_PI, right? So, it
probably doesn't need repeating.

I've reworked this description to be:

FUTEX_LOCK_PI2 (since Linux 5.14)
This operation is the same as FUTEX_LOCK_PI, except that
the clock against which timeout is measured is selectable.
By default, the (absolute) timeout specified in timeout is
measured againt the CLOCK_MONOTONIC clock, but if the
FUTEX_CLOCK_REALTIME flag is specified in futex_op, then
the timeout is measured against the CLOCK_REALTIME clock.

Is it okay?

Thanks,

Michael

--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/