diff mbox

Add configure clock_gettime() monotonic time test

Message ID 20130403192223.GC326@rox.home.comstyle.com
State New
Headers show

Commit Message

Brad Smith April 3, 2013, 7:22 p.m. UTC 
On Wed, Apr 03, 2013 at 10:18:51AM +0200, Paolo Bonzini wrote:
> Il 03/04/2013 00:52, Peter Maydell ha scritto:
> > On 2 April 2013 17:45, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >> Il 02/04/2013 17:42, Brad Smith ha scritto:
> >>> Replace the hardcoded list of OS's utilizing clock_gettime() for monotonic
> >>> time with a configure test. This is to fix the use of monotonic time on
> >>> OpenBSD but allows for other POSIX compliant OS's such as NetBSD to also
> >>> utilize clock_gettime().
> >>
> >> I thought the list of OSes was supposed to filter out those that somehow
> >> had a broken CLOCK_MONOTONIC.
> > 
> > Judging from wading through git history, it's mostly just
> > historic accretion from an initial #ifdef __linux__ which was
> > put in by Fabrice way back when configure was barely doing
> > compile-this-code checks at all.
> > 
> > Google does suggest that some OSes do provide a CLOCK_MONOTONIC
> > but clock_gettime(CLOCK_MONOTONIC) always fails, eg Centos 3.
> 
> It would already fail, and no one reported it.
> 
> CentOS 3 has a 2.4 kernel.  I doubt anyone is using it with a recent QEMU.
> 
> Paolo
> 
> >> Otherwise, you might as well use "#ifdef CLOCK_MONOTONIC" and skip the
> >> configure test completely.
> > 
> > Tempting.
> > 
> > -- PMM

Then how about the following? This looks like it should be good
for Linux, *BSD's, Solaris.

Comments

Paolo Bonzini April 3, 2013, 8 p.m. UTC | #1 
> On Wed, Apr 03, 2013 at 10:18:51AM +0200, Paolo Bonzini wrote:
> > Il 03/04/2013 00:52, Peter Maydell ha scritto:
> > > On 2 April 2013 17:45, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > >> Il 02/04/2013 17:42, Brad Smith ha scritto:
> > >>> Replace the hardcoded list of OS's utilizing clock_gettime() for
> > >>> monotonic
> > >>> time with a configure test. This is to fix the use of monotonic time on
> > >>> OpenBSD but allows for other POSIX compliant OS's such as NetBSD to
> > >>> also
> > >>> utilize clock_gettime().
> > >>
> > >> I thought the list of OSes was supposed to filter out those that somehow
> > >> had a broken CLOCK_MONOTONIC.
> > > 
> > > Judging from wading through git history, it's mostly just
> > > historic accretion from an initial #ifdef __linux__ which was
> > > put in by Fabrice way back when configure was barely doing
> > > compile-this-code checks at all.
> > > 
> > > Google does suggest that some OSes do provide a CLOCK_MONOTONIC
> > > but clock_gettime(CLOCK_MONOTONIC) always fails, eg Centos 3.
> > 
> > It would already fail, and no one reported it.
> > 
> > CentOS 3 has a 2.4 kernel.  I doubt anyone is using it with a recent QEMU.
> > 
> > Paolo
> > 
> > >> Otherwise, you might as well use "#ifdef CLOCK_MONOTONIC" and skip the
> > >> configure test completely.
> > > 
> > > Tempting.
> > > 
> > > -- PMM
> 
> Then how about the following? This looks like it should be good
> for Linux, *BSD's, Solaris.
> 
> 
> diff --git a/include/qemu/timer.h b/include/qemu/timer.h
> index 1766b2d..c363190 100644
> --- a/include/qemu/timer.h
> +++ b/include/qemu/timer.h
> @@ -117,8 +117,7 @@ extern int use_rt_clock;
>  
>  static inline int64_t get_clock(void)
>  {
> -#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >=
> 500000) \
> -    || defined(__DragonFly__) || defined(__FreeBSD_kernel__)
> +#ifdef CLOCK_MONOTONIC
>      if (use_rt_clock) {
>          struct timespec ts;
>          clock_gettime(CLOCK_MONOTONIC, &ts);
> diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
> index 16f5e75..95e0847 100644
> --- a/util/qemu-timer-common.c
> +++ b/util/qemu-timer-common.c
> @@ -49,9 +49,7 @@ int use_rt_clock;
>  static void __attribute__((constructor)) init_get_clock(void)
>  {
>      use_rt_clock = 0;
> -#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >=
> 500000) \
> -    || defined(__DragonFly__) || defined(__FreeBSD_kernel__) \
> -    || defined(__OpenBSD__)
> +#ifdef CLOCK_MONOTONIC
>      {
>          struct timespec ts;
>          if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo
Brad Smith April 3, 2013, 8:07 p.m. UTC | #2 
On Wed, Apr 03, 2013 at 03:22:24PM -0400, Brad Smith wrote:
> On Wed, Apr 03, 2013 at 10:18:51AM +0200, Paolo Bonzini wrote:
> > Il 03/04/2013 00:52, Peter Maydell ha scritto:
> > > On 2 April 2013 17:45, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > >> Il 02/04/2013 17:42, Brad Smith ha scritto:
> > >>> Replace the hardcoded list of OS's utilizing clock_gettime() for monotonic
> > >>> time with a configure test. This is to fix the use of monotonic time on
> > >>> OpenBSD but allows for other POSIX compliant OS's such as NetBSD to also
> > >>> utilize clock_gettime().
> > >>
> > >> I thought the list of OSes was supposed to filter out those that somehow
> > >> had a broken CLOCK_MONOTONIC.
> > > 
> > > Judging from wading through git history, it's mostly just
> > > historic accretion from an initial #ifdef __linux__ which was
> > > put in by Fabrice way back when configure was barely doing
> > > compile-this-code checks at all.
> > > 
> > > Google does suggest that some OSes do provide a CLOCK_MONOTONIC
> > > but clock_gettime(CLOCK_MONOTONIC) always fails, eg Centos 3.
> > 
> > It would already fail, and no one reported it.
> > 
> > CentOS 3 has a 2.4 kernel.  I doubt anyone is using it with a recent QEMU.
> > 
> > Paolo
> > 
> > >> Otherwise, you might as well use "#ifdef CLOCK_MONOTONIC" and skip the
> > >> configure test completely.
> > > 
> > > Tempting.
> > > 
> > > -- PMM
> 
> Then how about the following? This looks like it should be good
> for Linux, *BSD's, Solaris.


Signed-off-by: Brad Smith <brad@comstyle.com>

> diff --git a/include/qemu/timer.h b/include/qemu/timer.h
> index 1766b2d..c363190 100644
> --- a/include/qemu/timer.h
> +++ b/include/qemu/timer.h
> @@ -117,8 +117,7 @@ extern int use_rt_clock;
>  
>  static inline int64_t get_clock(void)
>  {
> -#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 500000) \
> -    || defined(__DragonFly__) || defined(__FreeBSD_kernel__)
> +#ifdef CLOCK_MONOTONIC
>      if (use_rt_clock) {
>          struct timespec ts;
>          clock_gettime(CLOCK_MONOTONIC, &ts);
> diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
> index 16f5e75..95e0847 100644
> --- a/util/qemu-timer-common.c
> +++ b/util/qemu-timer-common.c
> @@ -49,9 +49,7 @@ int use_rt_clock;
>  static void __attribute__((constructor)) init_get_clock(void)
>  {
>      use_rt_clock = 0;
> -#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 500000) \
> -    || defined(__DragonFly__) || defined(__FreeBSD_kernel__) \
> -    || defined(__OpenBSD__)
> +#ifdef CLOCK_MONOTONIC
>      {
>          struct timespec ts;
>          if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
Brad Smith April 4, 2013, 5:21 p.m. UTC | #3 
On Wed, Apr 03, 2013 at 03:22:24PM -0400, Brad Smith wrote:
> On Wed, Apr 03, 2013 at 10:18:51AM +0200, Paolo Bonzini wrote:
> > Il 03/04/2013 00:52, Peter Maydell ha scritto:
> > > On 2 April 2013 17:45, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > >> Il 02/04/2013 17:42, Brad Smith ha scritto:
> > >>> Replace the hardcoded list of OS's utilizing clock_gettime() for monotonic
> > >>> time with a configure test. This is to fix the use of monotonic time on
> > >>> OpenBSD but allows for other POSIX compliant OS's such as NetBSD to also
> > >>> utilize clock_gettime().
> > >>
> > >> I thought the list of OSes was supposed to filter out those that somehow
> > >> had a broken CLOCK_MONOTONIC.
> > > 
> > > Judging from wading through git history, it's mostly just
> > > historic accretion from an initial #ifdef __linux__ which was
> > > put in by Fabrice way back when configure was barely doing
> > > compile-this-code checks at all.
> > > 
> > > Google does suggest that some OSes do provide a CLOCK_MONOTONIC
> > > but clock_gettime(CLOCK_MONOTONIC) always fails, eg Centos 3.
> > 
> > It would already fail, and no one reported it.
> > 
> > CentOS 3 has a 2.4 kernel.  I doubt anyone is using it with a recent QEMU.
> > 
> > Paolo
> > 
> > >> Otherwise, you might as well use "#ifdef CLOCK_MONOTONIC" and skip the
> > >> configure test completely.
> > > 
> > > Tempting.
> > > 
> > > -- PMM
> 
> Then how about the following? This looks like it should be good
> for Linux, *BSD's, Solaris.

Any further comments?

> diff --git a/include/qemu/timer.h b/include/qemu/timer.h
> index 1766b2d..c363190 100644
> --- a/include/qemu/timer.h
> +++ b/include/qemu/timer.h
> @@ -117,8 +117,7 @@ extern int use_rt_clock;
>  
>  static inline int64_t get_clock(void)
>  {
> -#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 500000) \
> -    || defined(__DragonFly__) || defined(__FreeBSD_kernel__)
> +#ifdef CLOCK_MONOTONIC
>      if (use_rt_clock) {
>          struct timespec ts;
>          clock_gettime(CLOCK_MONOTONIC, &ts);
> diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
> index 16f5e75..95e0847 100644
> --- a/util/qemu-timer-common.c
> +++ b/util/qemu-timer-common.c
> @@ -49,9 +49,7 @@ int use_rt_clock;
>  static void __attribute__((constructor)) init_get_clock(void)
>  {
>      use_rt_clock = 0;
> -#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 500000) \
> -    || defined(__DragonFly__) || defined(__FreeBSD_kernel__) \
> -    || defined(__OpenBSD__)
> +#ifdef CLOCK_MONOTONIC
>      {
>          struct timespec ts;
>          if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
> 
> -- 
> This message has been scanned for viruses and
> dangerous content by MailScanner, and is
> believed to be clean.
> 
>
Anthony Liguori April 5, 2013, 12:22 a.m. UTC | #4 
Brad Smith <brad@comstyle.com> writes:

> On Wed, Apr 03, 2013 at 03:22:24PM -0400, Brad Smith wrote:
>> On Wed, Apr 03, 2013 at 10:18:51AM +0200, Paolo Bonzini wrote:
>> > Il 03/04/2013 00:52, Peter Maydell ha scritto:
>> > > On 2 April 2013 17:45, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> > >> Il 02/04/2013 17:42, Brad Smith ha scritto:
>> > >>> Replace the hardcoded list of OS's utilizing clock_gettime() for monotonic
>> > >>> time with a configure test. This is to fix the use of monotonic time on
>> > >>> OpenBSD but allows for other POSIX compliant OS's such as NetBSD to also
>> > >>> utilize clock_gettime().
>> > >>
>> > >> I thought the list of OSes was supposed to filter out those that somehow
>> > >> had a broken CLOCK_MONOTONIC.
>> > > 
>> > > Judging from wading through git history, it's mostly just
>> > > historic accretion from an initial #ifdef __linux__ which was
>> > > put in by Fabrice way back when configure was barely doing
>> > > compile-this-code checks at all.
>> > > 
>> > > Google does suggest that some OSes do provide a CLOCK_MONOTONIC
>> > > but clock_gettime(CLOCK_MONOTONIC) always fails, eg Centos 3.
>> > 
>> > It would already fail, and no one reported it.
>> > 
>> > CentOS 3 has a 2.4 kernel.  I doubt anyone is using it with a recent QEMU.
>> > 
>> > Paolo
>> > 
>> > >> Otherwise, you might as well use "#ifdef CLOCK_MONOTONIC" and skip the
>> > >> configure test completely.
>> > > 
>> > > Tempting.
>> > > 
>> > > -- PMM
>> 
>> Then how about the following? This looks like it should be good
>> for Linux, *BSD's, Solaris.
>
> Any further comments?

Please submit a top-level patch and I'll apply it.

Regards,

Anthony Liguori

>
>> diff --git a/include/qemu/timer.h b/include/qemu/timer.h
>> index 1766b2d..c363190 100644
>> --- a/include/qemu/timer.h
>> +++ b/include/qemu/timer.h
>> @@ -117,8 +117,7 @@ extern int use_rt_clock;
>>  
>>  static inline int64_t get_clock(void)
>>  {
>> -#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 500000) \
>> -    || defined(__DragonFly__) || defined(__FreeBSD_kernel__)
>> +#ifdef CLOCK_MONOTONIC
>>      if (use_rt_clock) {
>>          struct timespec ts;
>>          clock_gettime(CLOCK_MONOTONIC, &ts);
>> diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
>> index 16f5e75..95e0847 100644
>> --- a/util/qemu-timer-common.c
>> +++ b/util/qemu-timer-common.c
>> @@ -49,9 +49,7 @@ int use_rt_clock;
>>  static void __attribute__((constructor)) init_get_clock(void)
>>  {
>>      use_rt_clock = 0;
>> -#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 500000) \
>> -    || defined(__DragonFly__) || defined(__FreeBSD_kernel__) \
>> -    || defined(__OpenBSD__)
>> +#ifdef CLOCK_MONOTONIC
>>      {
>>          struct timespec ts;
>>          if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
>> 
>> -- 
>> This message has been scanned for viruses and
>> dangerous content by MailScanner, and is
>> believed to be clean.
>> 
>> 
>
> -- 
> This message has been scanned for viruses and
> dangerous content by MailScanner, and is
> believed to be clean.
diff mbox

Patch

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 1766b2d..c363190 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -117,8 +117,7 @@  extern int use_rt_clock;
 
 static inline int64_t get_clock(void)
 {
-#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 500000) \
-    || defined(__DragonFly__) || defined(__FreeBSD_kernel__)
+#ifdef CLOCK_MONOTONIC
     if (use_rt_clock) {
         struct timespec ts;
         clock_gettime(CLOCK_MONOTONIC, &ts);
diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
index 16f5e75..95e0847 100644
--- a/util/qemu-timer-common.c
+++ b/util/qemu-timer-common.c
@@ -49,9 +49,7 @@  int use_rt_clock;
 static void __attribute__((constructor)) init_get_clock(void)
 {
     use_rt_clock = 0;
-#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 500000) \
-    || defined(__DragonFly__) || defined(__FreeBSD_kernel__) \
-    || defined(__OpenBSD__)
+#ifdef CLOCK_MONOTONIC
     {
         struct timespec ts;
         if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {