git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 9/9] Use timer_settime for new platforms

From
Keller, Jacob E <jacob.e.keller@intel.com>
Date
Aug 29, 2014, 16:11 UTC
Message-ID
<1409328711.18778.12.camel@jekeller-desk1.amr.corp.intel.com>
In-Reply-To
<xmqq61hc9z94.fsf@gitster.dls.corp.google.com>
On Thu, 2014-08-28 at 12:43 -0700, Junio C Hamano wrote:
Show 41 quoted lines
> Jonas 'Sortie' Termansen <sortie@maxsi.org> writes:
> 
> > setitimer() is an obsolescent XSI interface and may be removed in a
> > future standard. Applications should use the core POSIX timer_settime()
> > instead.
> >
> > This patch cleans up the progress reporting and changes it to try using
> > timer_settime, or if that fails, setitimer. If either function is not
> > provided by the system, then git-compat-util.h provides replacements
> > that always fail with ENOSYS.
> >
> > It's important that code doesn't simply check if timer_settime is
> > available as it can give false positives. Some systems like contemporary
> > OpenBSD provides the function, but it unconditionally fails with ENOSYS
> > at runtime.
> >
> > This approach allows the code using timer_settime() and setitimer() to
> > be simple and readable. My first attempt used #ifdef around each use of
> > timer_settime(), this quickly turned a into unmaintainable maze of
> > preprocessor conditionals.
> >
> > Signed-off-by: Jonas 'Sortie' Termansen <sortie@maxsi.org>
> > ---
> >  builtin/log.c | 47 ++++++++++++++++++++++++++++++++++++++++-------
> >  progress.c    | 34 +++++++++++++++++++++++++++-------
> >  2 files changed, 67 insertions(+), 14 deletions(-)
> 
> Yuck.  I didn't look at the change very carefully, but are the two
> interface so vastly different that you cannot emulate one in terms
> of the other, and use a single API at the callsites, isolating the
> knowledge of which kind of API is used to interact with the system
> timer in one place (perhaps in compat/itimer.c)?
> 
> Having to sprinkle "if (is_using_timer_settime)" around means we
> need to support two APIs at each and every callsite that wants timer
> interrupt actions.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Agreed! The biggest difference is that timer_settime() uses nanoseconds value, which is very easy to convert microseconds into nanoseconds.

In fact, you could probably easily provide a wrapper for timer_settime that works for settimer by simply converting it. This is a much better approach. (though settimer "could" lose some precision since you ahve to divide by 1000)...

The other big difference is abstracting away the timer_create aspect which is ok, since we would always call timer create, and use the correct timer, but we just fall back to setitimer to use the default timer.

I'll submit a patch series which does what I think should work, and will result in less if checks.

Regards, Jake

Previous: Junio C Hamano
Message 16 of 16 in “Use timer_settime for new platforms”
  1. 0/9 Use timer_settime for new platformsJonas 'Sortie' Termansen, Aug 28, 2014
  2. 1/9 git-compat-util.h: Add missing semicolon after struct itimervalJonas 'Sortie' Termansen, Aug 28, 2014
  3. 2/9 autoconf: Check for timer_tJonas 'Sortie' Termansen, Aug 28, 2014
  4. Jonas 'Sortie' TermansenAug 28, 2014
  5. 3/9 autoconf: Check for struct timespecJonas 'Sortie' Termansen, Aug 28, 2014
  6. 4/9 autoconf: Check for struct sigeventJonas 'Sortie' Termansen, Aug 28, 2014
  7. 5/9 autoconf: Check for struct itimervalJonas 'Sortie' Termansen, Aug 28, 2014
  8. Junio C HamanoAug 28, 2014
  9. 6/9 autoconf: Check for struct itimerspecJonas 'Sortie' Termansen, Aug 28, 2014
  10. 7/9 autoconf: Check for setitimerJonas 'Sortie' Termansen, Aug 28, 2014
  11. 8/9 autoconf: Check for timer_settimeJonas 'Sortie' Termansen, Aug 28, 2014
  12. Keller, Jacob EAug 29, 2014
  13. Jonas 'Sortie' TermansenAug 29, 2014
  14. 9/9 Use timer_settime for new platformsJonas 'Sortie' Termansen, Aug 28, 2014
  15. Junio C HamanoAug 28, 2014
  16. Keller, Jacob EAug 29, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.