Re: futex: Allow FUTEX_CLOCK_REALTIME with FUTEX_WAIT op

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

Hi Darren,

On 06/23/2016 06:16 PM, Darren Hart wrote:
> On Thu, Jun 23, 2016 at 03:40:36PM +0200, Thomas Gleixner wrote:
>> On Thu, 23 Jun 2016, Michael Kerrisk (man-pages) wrote:
>>> On 06/23/2016 09:18 AM, Thomas Gleixner wrote:
>>> Once upon a time, you told me the following:
>>>
>>> On 15 May 2014 at 16:14, Thomas Gleixner <tglx@linutronix.de> wrote:
>>>> On Thu, 15 May 2014, Michael Kerrisk (man-pages) wrote:
>>>>> And that universe would love to have your documentation of
>>>>> FUTEX_WAKE_BITSET and FUTEX_WAIT_BITSET ;-),
>>>>
>>>> I give you almost the full treatment, but I leave REQUEUE_PI to Darren
>>>> and FUTEX_WAKE_OP to Jakub. :)
>>>> [...]
>>>> FUTEX_CLOCK_REALTIME
>>>>
>>>>         This option bit can be ored on the futex ops FUTEX_WAIT_BITSET
>>>>         and FUTEX_WAIT_REQUEUE_PI
>>>>
>>>>         If set the kernel treats the user space supplied timeout as
>>>>         absolute time based on CLOCK_REALTIME.
>>>>
>>>>         If not set the kernel treats the user space supplied timeout
>>>>         as relative time.
>>> Unfortunately, I should have checked the code more carefully...
>>
>> Me too :)
>
> Seems to be going around...
>
>>
>>> Looking more carefully at the code, I see understand the situation
>>> is the following:
>>>
>>> FUTEX_LOCK_PI
>>> 	Always uses CLOCK_REALTIME
>>> 	'timeout' is absolute
>>
>> Yes.
>>
>>> FUTEX_WAIT_REQUEUE_PI
>>> 	Choice of clock (CLOCK_REALTIME vs CLOCK_MONOTONIC) is
>>>         	determined by presence or absence of
>>> 		FUTEX_CLOCK_REALTIME flag
>>> 	'timeout' is absolute
>>
>> Yes
>>
>>> FUTEX_WAIT_BITSET
>>> 	Choice of clock (CLOCK_REALTIME vs CLOCK_MONOTONIC) is
>>>         	determined by presence or absence of
>>> 		FUTEX_CLOCK_REALTIME flag
>>> 	'timeout' is absolute
>>
>> Yes
>>
>>> FUTEX_WAIT
>>> 	Choice of clock (CLOCK_REALTIME vs CLOCK_MONOTONIC) is
>>>         	determined by presence or absence of
>>> 		FUTEX_CLOCK_REALTIME flag
>>> 	'timeout' is relative
>>
>> Yes.
>>
>>> I've amended the man page to describe those details.
>
> OK, that confirms my question, timeout interpretation as relative or absolute is
> based on the op code, not the CLOCK flag.
>
>>>
>>>> The flag was explicitely added to allow FUTEX_WAIT to hand in absolute time.
>>>
>>> When you say that the "flag was added", which flag do you mean? Or, did you
>>> mean: "applying Matthieu's patch will allow FUTEX_WAIT to hand in absolute
>>> time".
>>
>> I didn't express myself clearly. When Darren added the support for
>> CLOCK_REALTIME to FUTEX_WAIT I think he wanted to add absolute timeout
>> support. Anything else does not make sense.
>
> I sent that patch because reading the new man page it struck me as strange that
> FUTEX_WAIT was restricted to CLOCK_MONOTONIC and the other op codes were not,
> especially since FUTEX_WAIT is a just FUTEX_WAIT_BITSET with the mask set to
> ALL.
>
> I didn't realize the impact to relative/absolute interpretation of the timeout
> value at the time.
>
> I think it was a mistake to introduce a change that made FUTEX_WAIT interpret
> the timeout differently based on the CLOCK flag,

I'm missing something. Where does it do that? As far as I can tell FUTEX_WAIT
always interprets the clock as relative, regardless of presence/absence of
FUTEX_CLOCK_REALTIME? Am I missing something?

> while that interpretation is
> independent of the CLOCK flag for all other op codes.
>
> In my opinion, we should treat the timeout value as relative for FUTEX_WAIT
> regardless of the CLOCK used.

I realize it's historical, but it is really weird that FUTEX_WAIT interprets
time timeout (relative vs absolute) differently from all of the other
operations.
> That would require a change to the man page to eliminate the relative/absolute
> language in the FUTEX_CLOCK_REALTIME definition and explicit definitions of the
> interpretation for each op code (as Matthew explains above).
>
> Do we agree on that?

Yes.

The man page changes are already in Git. My earlier reply contained the
commit ref:
http://git.kernel.org/cgit/docs/man-pages/man-pages.git/commit/?id=8064bfa5369c6856f606004d02e48ab275e05bed

Cheers,

Michael


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