Linux-man Archive on lore.kernel.org
 help / color / Atom feed
From: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
To: "devi R.K" <devi.feb27@gmail.com>
Cc: mtk.manpages@gmail.com, linux-man@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	lkml <linux-kernel@vger.kernel.org>,
	arul.jeniston@gmail.com
Subject: Re: [PATCH] timerfd_create.2: Included return value 0
Date: Sun, 29 Mar 2020 23:06:49 +0200
Message-ID: <3cbd0919-c82a-cb21-c10f-0498433ba5d1@gmail.com> (raw)
In-Reply-To: <CAJymdby8pDASq5Xv7DeTJ5cq1NXPe1jUWwApxZ5o-huaEXUrjw@mail.gmail.com>

[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@gmail.com> 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@gmail.com>
>>
>> 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@nanos.tec.linutronix.de/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@nanos.tec.linutronix.de/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@nanos.tec.linutronix.de/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/

  parent reply index

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAJymdbxJNag1W0vR9Ysm7_y91HwLAu4QaSMKZbed4emT1DYvww@mail.gmail.com>
2020-03-26  8:45 ` Michael Kerrisk (man-pages)
     [not found]   ` <CAJymdbwfm7EypQfXRqWZHbfj2SKk7PCP7SMccz6bmJWSauDqPQ@mail.gmail.com>
     [not found]     ` <CAJymdby8pDASq5Xv7DeTJ5cq1NXPe1jUWwApxZ5o-huaEXUrjw@mail.gmail.com>
2020-03-29 21:06       ` Michael Kerrisk (man-pages) [this message]
2020-03-29 22:50         ` Thomas Gleixner
2020-03-30 21:09           ` Michael Kerrisk (man-pages)
2020-03-30 21:29             ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3cbd0919-c82a-cb21-c10f-0498433ba5d1@gmail.com \
    --to=mtk.manpages@gmail.com \
    --cc=arul.jeniston@gmail.com \
    --cc=devi.feb27@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-man@vger.kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-man Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-man/0 linux-man/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-man linux-man/ https://lore.kernel.org/linux-man \
		linux-man@vger.kernel.org
	public-inbox-index linux-man

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-man


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git