Re: [PATCH] timerfd_create.2: Included return value 0

[Michael Kerrisk (man-pages)]

[CC widened]

Hello Devi,

[It's best not to top post. I've rearranged the reply 
for readability.]

[Greetings, Thomas; now I recall a conversation we had in Lyon :-) ]


On 3/27/20 5:29 AM, devi R.K wrote:
> 
> On Thu, Mar 26, 2020 at 2:16 PM Michael Kerrisk (man-pages) <
> mtk.manpages@xxxxxxxxx> wrote:
> 
>> Hello Devi,
>>
>> On 3/18/20 3:04 PM, devi R.K wrote:
>>> Added a return value 0 for timerfd_read which happens when there is a
>>> bigger backward time drift*.*
>>>
>>> Signed-off-by: DEVI RK <devi.feb27@xxxxxxxxx>
>>
>> Can you say some more please about how you verified this and/or
>> point me at the relevant kernel source code? At a simple attempt,
>> I can't replicate the behavior you describe.

> We have written a program using real time clock and it has been raised to
> the community.
> 
> https://lore.kernel.org/lkml/alpine.DEB.2.21.1908191943280.1796@xxxxxxxxxxxxxxxxxxxxxxx/T/

It would be helpful if you had pointed me to this in the first
place, and also CCed the people from that earlier discussion.
I've widened the CC list.

Thanks for pointing me at that thread. In particular, the test
program at
https://lore.kernel.org/lkml/alpine.DEB.2.21.1908191943280.1796@xxxxxxxxxxxxxxxxxxxxxxx/T/#m489d81abdfbb2699743e18c37657311f8d52a4cd

I've now replicated this behavior with a program of my own.

>>> ---
>>>  man2/timerfd_create.2 | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/man2/timerfd_create.2 b/man2/timerfd_create.2
>>> index 066e0be..ccced98 100644
>>> --- a/man2/timerfd_create.2
>>> +++ b/man2/timerfd_create.2
>>> @@ -317,6 +317,10 @@ fails with the error
>>>  if the real-time clock undergoes a discontinuous change.
>>>  (This allows the reading application to discover
>>>  such discontinuous changes to the clock.)
>>> +.IP
>>> +A
>>> +.BR read (2)
>>> +may return 0 if the system clock undergoes a discontinuous change.
>>>  .TP
>>>  .BR poll "(2), " select "(2) (and similar)"
>>>  The file descriptor is readable

I think this patch does not really capture the details
properly. The immediately preceding paragraph says:

         If  the  associated  clock  is  either  CLOCK_REALTIME   or
         CLOCK_REALTIME_ALARM,     the     timer     is     absolute
         (TFD_TIMER_ABSTIME), and the  flag  TFD_TIMER_CANCEL_ON_SET
         was  specified when calling timerfd_settime(), then read(2)
         fails with the  error  ECANCELED  if  the  real-time  clock
         undergoes a discontinuous change.  (This allows the reading
         application to discover such discontinuous changes  to  the
         clock.)

Following on from that, I think we should have a pargraph that says
something like:

         If  the  associated  clock  is  either  CLOCK_REALTIME   or
         CLOCK_REALTIME_ALARM,     the     timer     is     absolute
         (TFD_TIMER_ABSTIME), and the  flag  TFD_TIMER_CANCEL_ON_SET
         was not specified when calling timerfd_settime(), then a
         discontinuous negative change to the clock 
         (e.g., clock_settime(2)) may cause read(2) to unblock, but
         return a value of 0 (i.e., no bytes read), if the clock
         change occurs after the time expired, but before the
         read(2) on the timerfd file descriptor.

This seems consistent with Thomas's observations in
https://lore.kernel.org/lkml/alpine.DEB.2.21.1908191943280.1796@xxxxxxxxxxxxxxxxxxxxxxx/T/#m49b78122b573a2749a05b720dc9fa036546db490

Do you agree?

Thomas, if you had a moment, your input would, as always,
be appreciated.

Cheers,

Michael

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