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

["Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>]

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/