Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
> From: ykzhao > Date: Fri, 27 Aug 2010 08:35:07 +0800 > > Thanks for the comments. > OK. I will limit it to linux platform and won't create it if it is not > defined in header file. Don't limit it to just Linux. We need less #ifdef __MyOS__ in the tree, not more. Just adding #ifdef CLOCK_MONOTONIC_COARSE should be enough. You'll need that anyway to support older versions of Linux. ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
On Thu, 2010-08-26 at 17:55 +0800, Pauli Nieminen wrote: > On Thu, Aug 26, 2010 at 8:48 AM, ykzhao wrote: > > On Tue, 2010-08-24 at 17:02 +0800, Mikhail Gusarov wrote: > >> Twas brillig at 16:45:34 24.08.2010 UTC+08 when yakui.z...@intel.com did > >> gyre and gimble: > >> > >> y> What side effect will it bring if we define it explicitly and use it > >> y> when it is not defined in system header? > >> > >> Bad side-effect is X.org becoming OS again :) It isn't hard to require > >> newer linux-libc-dev headers for CLOCK_MONOTONIC_COARSE definition. > > > > How about limiting it to linux platform as mentioned by Samuel > > Thibault? > > > > The change of this patch happens in the file of os/utils.c, which has > > the dependency on the OS. Not sure whether it will be too strict that > > the glibc header file needs to be updated in order to use the > > CLOCK_MONOTONIC_COARSE posix timer. > > > > Now some latest linux distribution already adds the definition of > > CLOCK_MONOTONIC_COARSE. But on the previous distribution, the user still > > can enjoy the benefit of using CLOCK_MONOTONIC_COARSE posix timer after > > updating the linux kernel while the gcc tool doesn't need to be updated. > > > > That use case is very unlikely to happen excluding developers. Nearly > everyone else will be using this patch in system with new enough > headers that do define CLOCK_MONOTIC_COARSE. Thanks for the comments. OK. I will limit it to linux platform and won't create it if it is not defined in header file. Thanks. Yakui > > > Thanks. > > yakui > >> > > > > ___ > > xorg-devel@lists.x.org: X.Org development > > Archives: http://lists.x.org/archives/xorg-devel > > Info: http://lists.x.org/mailman/listinfo/xorg-devel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
On Thu, Aug 26, 2010 at 8:48 AM, ykzhao wrote: > On Tue, 2010-08-24 at 17:02 +0800, Mikhail Gusarov wrote: >> Twas brillig at 16:45:34 24.08.2010 UTC+08 when yakui.z...@intel.com did >> gyre and gimble: >> >> y> What side effect will it bring if we define it explicitly and use it >> y> when it is not defined in system header? >> >> Bad side-effect is X.org becoming OS again :) It isn't hard to require >> newer linux-libc-dev headers for CLOCK_MONOTONIC_COARSE definition. > > How about limiting it to linux platform as mentioned by Samuel > Thibault? > > The change of this patch happens in the file of os/utils.c, which has > the dependency on the OS. Not sure whether it will be too strict that > the glibc header file needs to be updated in order to use the > CLOCK_MONOTONIC_COARSE posix timer. > > Now some latest linux distribution already adds the definition of > CLOCK_MONOTONIC_COARSE. But on the previous distribution, the user still > can enjoy the benefit of using CLOCK_MONOTONIC_COARSE posix timer after > updating the linux kernel while the gcc tool doesn't need to be updated. > That use case is very unlikely to happen excluding developers. Nearly everyone else will be using this patch in system with new enough headers that do define CLOCK_MONOTIC_COARSE. > Thanks. > yakui >> > > ___ > xorg-devel@lists.x.org: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
On Tue, 2010-08-24 at 17:02 +0800, Mikhail Gusarov wrote: > Twas brillig at 16:45:34 24.08.2010 UTC+08 when yakui.z...@intel.com did gyre > and gimble: > > y> What side effect will it bring if we define it explicitly and use it > y> when it is not defined in system header? > > Bad side-effect is X.org becoming OS again :) It isn't hard to require > newer linux-libc-dev headers for CLOCK_MONOTONIC_COARSE definition. How about limiting it to linux platform as mentioned by Samuel Thibault? The change of this patch happens in the file of os/utils.c, which has the dependency on the OS. Not sure whether it will be too strict that the glibc header file needs to be updated in order to use the CLOCK_MONOTONIC_COARSE posix timer. Now some latest linux distribution already adds the definition of CLOCK_MONOTONIC_COARSE. But on the previous distribution, the user still can enjoy the benefit of using CLOCK_MONOTONIC_COARSE posix timer after updating the linux kernel while the gcc tool doesn't need to be updated. Thanks. yakui > ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
On Tue, 2010-08-24 at 18:59 +0800, Julien Cristau wrote: > On Tue, Aug 24, 2010 at 16:15:44 +0800, ykzhao wrote: > > > I also agree that it is very pretty to get the time by using mentioned > > order. But the configure script already helps us to test whether the > > MONOTONIC_CLOCK is supported(It uses the CLOCK_MONOTONIC as the argument > > of clock_gettime). > >If so, maybe we can avoid some redundant check. > > > The configure check tells you whether CLOCK_MONOTONIC is defined on the > *build* system. That's not the same thing as telling you whether it > *works* on the *target*. Understand. And it looks more reasonable to check the CLOCK_MONOTONIC on the *target* again. Is it better that the check of CLOCK_MONOTONIC posix timer is put into another separated patch? Otherwise it will be mixed with the usage of CLOCK_MONOTONIC_COARSE posix timer. Thanks. > > Cheers, > Julien ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
ykzhao wrote: > On Mon, 2010-08-23 at 23:25 +0800, Adam Jackson wrote: >> On Mon, 2010-08-23 at 10:23 +0200, Mark Kettenis wrote: From: yakui.z...@intel.com Date: Mon, 23 Aug 2010 15:20:05 +0800 From: Zhao Yakui --- os/utils.c | 14 +- 1 files changed, 13 insertions(+), 1 deletions(-) diff --git a/os/utils.c b/os/utils.c index 51455cc..a08d591 100644 --- a/os/utils.c +++ b/os/utils.c @@ -242,6 +242,10 @@ OsSignal(int sig, OsSigHandlerPtr handler) #endif #endif +#ifndef CLOCK_MONOTONIC_COARSE +#define CLOCK_MONOTONIC_COARSE 6 +#endif >>> What if an OS doesn't have CLOCK_MONOTONIC_COARSE, but uses the clock >>> ID 6 for some other purpose? >> Then this patch would be wrong. >> >> NAK on that basis. > > Yes. Agree. > > How about using the constant value(6) directly? That's the part that's wrong, assuming 6 will always mean CLOCK_MONOTONIC_COARSE. Perhaps if it's now part of the Linux ABI, you could do it inside an #ifdef linux, but you can't assume that value means anything useful on a non-linux kernel. -- -Alan Coopersmith-alan.coopersm...@oracle.com Oracle Solaris Platform Engineering: X Window System ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
Daniel Stone wrote:
> Hi,
>
> On Tue, Aug 24, 2010 at 08:55:22AM +0800, ykzhao wrote:
>> What Mark mentioned is that the CLOCK_MONOTONIC_COARSE posix timer is
>> not supported while the corresponding ID is used for other posix timer.
>> Right?
>
> 6 has no meaning to clock_gettime(). CLOCK_MONOTONIC_COARSE is the only
> thing that has any meaning: if it's not defined, you can't just invent a
> definition and hope that it works. That's why the spec says
> CLOCK_MONOTONIC_COARSE and not 6.
>
>> If so, is there an approach that helps us to detect whether the
>> CLOCK_MONOTONIC_COARSE posix timer is supported on one OS?
>
> Try the following completely untested patch (hey, it compiles). It's
> not perfect though: if CLOCK_MONOTONIC or CLOCK_MONOTONIC_COARSE were
> ever 0, we'd make two or three syscalls for GetTimeInMillis() instead of
> one, and if either of them were ~0L, we'd never use them.
>
> Cheers,
> Daniel
>
> diff --git a/os/utils.c b/os/utils.c
> index 51455cc..a1659ec 100644
> --- a/os/utils.c
> +++ b/os/utils.c
> @@ -427,7 +427,20 @@ GetTimeInMillis(void)
>
> #ifdef MONOTONIC_CLOCK
> struct timespec tp;
> -if (clock_gettime(CLOCK_MONOTONIC, &tp) == 0)
> +static clockid_t clockid;
> +if (!clockid) {
> +#ifdef CLOCK_MONOTONIC_COARSE
> +if (clock_getres(CLOCK_MONOTONIC_COARSE, &tp) == 0 &&
> +(tp.tv_nsec / 1000) <= 1000)
> +clockid = CLOCK_MONOTONIC_COARSE;
> +else
> +#endif
> +if (clock_gettime(CLOCK_MONOTONIC, &tp) == 0)
> +clockid = CLOCK_MONOTONIC;
> +else
> +clockid = ~0L;
> +}
> +if (clockid != ~0L && clock_gettime(clockid, &tp) == 0)
> return (tp.tv_sec * 1000) + (tp.tv_nsec / 100L);
> #endif
>From Review, not Testing, that looks like a pretty good solution to me.
--
-Alan Coopersmith-alan.coopersm...@oracle.com
Oracle Solaris Platform Engineering: X Window System
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
On Tue, Aug 24, 2010 at 16:15:44 +0800, ykzhao wrote: > I also agree that it is very pretty to get the time by using mentioned > order. But the configure script already helps us to test whether the > MONOTONIC_CLOCK is supported(It uses the CLOCK_MONOTONIC as the argument > of clock_gettime). >If so, maybe we can avoid some redundant check. > The configure check tells you whether CLOCK_MONOTONIC is defined on the *build* system. That's not the same thing as telling you whether it *works* on the *target*. Cheers, Julien ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
Twas brillig at 16:45:34 24.08.2010 UTC+08 when yakui.z...@intel.com did gyre and gimble: y> What side effect will it bring if we define it explicitly and use it y> when it is not defined in system header? Bad side-effect is X.org becoming OS again :) It isn't hard to require newer linux-libc-dev headers for CLOCK_MONOTONIC_COARSE definition. -- http://fossarchy.blogspot.com/ pgpSrFzo1Hd7K.pgp Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
On Tue, 2010-08-24 at 16:13 +0800, Daniel Stone wrote: > On Tue, Aug 24, 2010 at 03:55:41PM +0800, ykzhao wrote: > > On Tue, 2010-08-24 at 09:59 +0800, Daniel Stone wrote: > > > That doesn't change anything - if a system is using ID 6 for something > > > else, then using 6 is wholly incorrect, no matter whether you use the > > > constant directly, define some other symbol for it, or whatever. > > > > Maybe the system is using the ID 6 for something else. But in theory it > > should not affect the system-call of posix timer. > > What do you mean, it shouldn't affect anything? What if timer ID 6 is > being used for 'number of seconds since last email to xorg-devel'? If we add a specific posix timer and the corresponding ID is 6, it can't work. In such case we have no way to make it work. But this doesn't happen on the upstream linux kernel. > > > If the CLOCK_MONOTONIC_COARSE posix timer is already supported in the > > linux kernel(clockid is 6), the system-call of "clock_getres/gettime(6, > > &tp)" will return the valid value. > > It will return a valid value _for that clock_. That clock may be > exactly what you don't want, if clock ID 6 is actually something else. > > > If the CLOCK_MONOTONIC_COARSE is not supported in the linux kernel, the > > system-call of "clock_getres/gettime(6, &tp)" will return the invalid > > value, which indicates that we will fallback to the next posix > > timer(CLOCK_MONOTONIC). > > Hopefully, yes. > > > Of course there exists another scenario. The CLOCK_MONOTONIC_COARSE > > posix timer is not supported. In course of compiling the Xorg we add the > > macro definition of "CLOCK_MONOTONIC_COARSE=3". > > 3?!? It is only an example. What I want to say is that we will use the incorrect posix timer if we explicitly add one definition of "CLOCK_MONOTONIC_COARSE" for the following code: >clock_getres(CLOCK_MONOTONIC_COARSE, &tp) But in theory that won't happen on Linux. I agree with what Samuel Thibault said. > > > In such case we will use > > the incorrect posix timer after calling the function of > > clock_getres(CLOCK_MONOTONIC_COARSE, &tp). But if we compile it again > > under the latest glibc, we will get the warning message of "macro > > redefinition". > > > > Based on the above consideration, not sure whether it is meaningful to > > use the constant value(6) for the system-call of clock_getres? Otherwise > > we will have to consider the following two cases: > > a. CLOCK_MONOTONIC_COARSE is not defined > > in /usr/include/linux/time.h > > b. CLOCK_MONOTONIC_COARSE is already defined. But the corresponding > > id is not 6. > > > > If wrong, please correct me. > > 6 is not meaningful in any way. Only CLOCK_MONOTONIC_COARSE, _as > defined by the system headers_, means a coarse/non-HPET monotonic clock. > Nothing else - not 6, not 3. So, if it's not in the system headers, I > don't think we should be using it. I'm not really sure if I can be more > clear ... The CLOCK_MONOTONIC_COARSE is not defined in the header file for the previous distribution. But it is really supported on the latest linux kernel. If we define it, it is assured that it can work on both the previous and latest kernel. Especially we compile it on the previous distribution and execute it on the latest kernel. What side effect will it bring if we define it explicitly and use it when it is not defined in system header? ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
ykzhao, le Tue 24 Aug 2010 15:55:41 +0800, a écrit : > b. CLOCK_MONOTONIC_COARSE is already defined. But the corresponding > id is not 6. That won't happen on Linux, the value is now cast into stone. That's why it's safe to use #ifdef __linux__ # ifndef CLOCK_MONOTONIC_COARSE # define CLOCK_MONOTONIC_COARSE 6 # endif #endif On other OSes that may happen (that's the original Nack against your patch), thus the #ifdef __linux__ above is necessary. Samuel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
On Tue, 2010-08-24 at 12:46 +0800, Daniel Stone wrote: > Hi, > > On Tue, Aug 24, 2010 at 11:05:23AM +0800, ykzhao wrote: > > Maybe there is no definition of "CLOCK_MONOTONIC_COARSE" > > in /usr/include/bits/time.h when compiling the xorg. But the xorg is > > executed on the linux kernel that supports the CLOCK_MONOTONIC_COARSE > > posix timer.(In fact for most previous Linux distribution there is no > > definition of CLOCK_MONOTONIC_COARSE in /usr/include/bits/time.h). > > If it is executed on the kernel that doesn't support the > > CLOCK_MONOTONIC_COARSE timer, it will fallback to the CLOCK_MONOTOIC > > posix timer(the function of clock_getres will return the invalid value). > > > > Do we need to consider the above scenario? If the above scenario doesn't > > need to be cared, I will update the patch to assure that > > the CLOCK_MONOTONIC_COARSE posix timer will be tried only when there > > exists the corresponding definition. > > Why not just fix glibc to include the definition? > > > > > If so, is there an approach that helps us to detect whether the > > > > CLOCK_MONOTONIC_COARSE posix timer is supported on one OS? > > > > > > Try the following completely untested patch (hey, it compiles). It's > > > not perfect though: if CLOCK_MONOTONIC or CLOCK_MONOTONIC_COARSE were > > > ever 0, we'd make two or three syscalls for GetTimeInMillis() instead of > > > one, and if either of them were ~0L, we'd never use them. > > > > the corresponding code is put under the condition definition of > > MONOTONIC_CLOCK. This is already checked by using configure script. > > > > In theory the CLOCK_MONOTONIC exists if the MONOTONIC_CLOCK is > > defined. Not sure whether it is still necessary to check the > > CLOCK_MONOTONIC again? > > It was just an overly paranoid check to make sure that CLOCK_MONOTONIC > actually works on the target system, as well as exists on the build > system. I'm not sure if there's a Linux kernel we really support that > doesn't have a working CLOCK_MONOTONIC, but I wasn't sure, so. > > I'd say CLOCK_MONOTONIC_COARSE -> CLOCK_MONOTONIC -> gettimeofday is the > best order, given that it's in descending order of usefulness, but > ascending order of likelihood of working. I also agree that it is very pretty to get the time by using mentioned order. But the configure script already helps us to test whether the MONOTONIC_CLOCK is supported(It uses the CLOCK_MONOTONIC as the argument of clock_gettime). If so, maybe we can avoid some redundant check. Thanks. > > Cheers, > Daniel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
On Tue, Aug 24, 2010 at 03:55:41PM +0800, ykzhao wrote: > On Tue, 2010-08-24 at 09:59 +0800, Daniel Stone wrote: > > That doesn't change anything - if a system is using ID 6 for something > > else, then using 6 is wholly incorrect, no matter whether you use the > > constant directly, define some other symbol for it, or whatever. > > Maybe the system is using the ID 6 for something else. But in theory it > should not affect the system-call of posix timer. What do you mean, it shouldn't affect anything? What if timer ID 6 is being used for 'number of seconds since last email to xorg-devel'? > If the CLOCK_MONOTONIC_COARSE posix timer is already supported in the > linux kernel(clockid is 6), the system-call of "clock_getres/gettime(6, > &tp)" will return the valid value. It will return a valid value _for that clock_. That clock may be exactly what you don't want, if clock ID 6 is actually something else. > If the CLOCK_MONOTONIC_COARSE is not supported in the linux kernel, the > system-call of "clock_getres/gettime(6, &tp)" will return the invalid > value, which indicates that we will fallback to the next posix > timer(CLOCK_MONOTONIC). Hopefully, yes. > Of course there exists another scenario. The CLOCK_MONOTONIC_COARSE > posix timer is not supported. In course of compiling the Xorg we add the > macro definition of "CLOCK_MONOTONIC_COARSE=3". 3?!? > In such case we will use > the incorrect posix timer after calling the function of > clock_getres(CLOCK_MONOTONIC_COARSE, &tp). But if we compile it again > under the latest glibc, we will get the warning message of "macro > redefinition". > > Based on the above consideration, not sure whether it is meaningful to > use the constant value(6) for the system-call of clock_getres? Otherwise > we will have to consider the following two cases: > a. CLOCK_MONOTONIC_COARSE is not defined > in /usr/include/linux/time.h > b. CLOCK_MONOTONIC_COARSE is already defined. But the corresponding > id is not 6. > > If wrong, please correct me. 6 is not meaningful in any way. Only CLOCK_MONOTONIC_COARSE, _as defined by the system headers_, means a coarse/non-HPET monotonic clock. Nothing else - not 6, not 3. So, if it's not in the system headers, I don't think we should be using it. I'm not really sure if I can be more clear ... signature.asc Description: Digital signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
On Tue, 2010-08-24 at 09:59 +0800, Daniel Stone wrote: > On Tue, Aug 24, 2010 at 08:32:48AM +0800, ykzhao wrote: > > On Mon, 2010-08-23 at 23:25 +0800, Adam Jackson wrote: > > > On Mon, 2010-08-23 at 10:23 +0200, Mark Kettenis wrote: > > > > > diff --git a/os/utils.c b/os/utils.c > > > > > index 51455cc..a08d591 100644 > > > > > --- a/os/utils.c > > > > > +++ b/os/utils.c > > > > > @@ -242,6 +242,10 @@ OsSignal(int sig, OsSigHandlerPtr handler) > > > > > #endif > > > > > #endif > > > > > > > > > > +#ifndef CLOCK_MONOTONIC_COARSE > > > > > +#define CLOCK_MONOTONIC_COARSE 6 > > > > > +#endif > > > > > > > > What if an OS doesn't have CLOCK_MONOTONIC_COARSE, but uses the clock > > > > ID 6 for some other purpose? > > > > > > Then this patch would be wrong. > > > > > > NAK on that basis. > > > > Yes. Agree. > > > > How about using the constant value(6) directly? > > That doesn't change anything - if a system is using ID 6 for something > else, then using 6 is wholly incorrect, no matter whether you use the > constant directly, define some other symbol for it, or whatever. Maybe the system is using the ID 6 for something else. But in theory it should not affect the system-call of posix timer. If the CLOCK_MONOTONIC_COARSE posix timer is already supported in the linux kernel(clockid is 6), the system-call of "clock_getres/gettime(6, &tp)" will return the valid value. If the CLOCK_MONOTONIC_COARSE is not supported in the linux kernel, the system-call of "clock_getres/gettime(6, &tp)" will return the invalid value, which indicates that we will fallback to the next posix timer(CLOCK_MONOTONIC). Of course there exists another scenario. The CLOCK_MONOTONIC_COARSE posix timer is not supported. In course of compiling the Xorg we add the macro definition of "CLOCK_MONOTONIC_COARSE=3". In such case we will use the incorrect posix timer after calling the function of clock_getres(CLOCK_MONOTONIC_COARSE, &tp). But if we compile it again under the latest glibc, we will get the warning message of "macro redefinition". Based on the above consideration, not sure whether it is meaningful to use the constant value(6) for the system-call of clock_getres? Otherwise we will have to consider the following two cases: a. CLOCK_MONOTONIC_COARSE is not defined in /usr/include/linux/time.h b. CLOCK_MONOTONIC_COARSE is already defined. But the corresponding id is not 6. If wrong, please correct me. Best regards. Yakui > > #ifdef CLOCK_MONOTONIC_COARSE > /* include support for CLOCK_MONOTONIC_COARSE */ > #else > /* it's not there so don't */ > #endif ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
Daniel Stone, le Tue 24 Aug 2010 14:46:24 +1000, a écrit : > > Do we need to consider the above scenario? If the above scenario doesn't > > need to be cared, I will update the patch to assure that > > the CLOCK_MONOTONIC_COARSE posix timer will be tried only when there > > exists the corresponding definition. > > Why not just fix glibc to include the definition? It's there already of course. But you won't see it appearing in distribution before some time. Samuel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
Hi, On Tue, Aug 24, 2010 at 11:05:23AM +0800, ykzhao wrote: > Maybe there is no definition of "CLOCK_MONOTONIC_COARSE" > in /usr/include/bits/time.h when compiling the xorg. But the xorg is > executed on the linux kernel that supports the CLOCK_MONOTONIC_COARSE > posix timer.(In fact for most previous Linux distribution there is no > definition of CLOCK_MONOTONIC_COARSE in /usr/include/bits/time.h). > If it is executed on the kernel that doesn't support the > CLOCK_MONOTONIC_COARSE timer, it will fallback to the CLOCK_MONOTOIC > posix timer(the function of clock_getres will return the invalid value). > > Do we need to consider the above scenario? If the above scenario doesn't > need to be cared, I will update the patch to assure that > the CLOCK_MONOTONIC_COARSE posix timer will be tried only when there > exists the corresponding definition. Why not just fix glibc to include the definition? > > > If so, is there an approach that helps us to detect whether the > > > CLOCK_MONOTONIC_COARSE posix timer is supported on one OS? > > > > Try the following completely untested patch (hey, it compiles). It's > > not perfect though: if CLOCK_MONOTONIC or CLOCK_MONOTONIC_COARSE were > > ever 0, we'd make two or three syscalls for GetTimeInMillis() instead of > > one, and if either of them were ~0L, we'd never use them. > > the corresponding code is put under the condition definition of > MONOTONIC_CLOCK. This is already checked by using configure script. > > In theory the CLOCK_MONOTONIC exists if the MONOTONIC_CLOCK is > defined. Not sure whether it is still necessary to check the > CLOCK_MONOTONIC again? It was just an overly paranoid check to make sure that CLOCK_MONOTONIC actually works on the target system, as well as exists on the build system. I'm not sure if there's a Linux kernel we really support that doesn't have a working CLOCK_MONOTONIC, but I wasn't sure, so. I'd say CLOCK_MONOTONIC_COARSE -> CLOCK_MONOTONIC -> gettimeofday is the best order, given that it's in descending order of usefulness, but ascending order of likelihood of working. Cheers, Daniel signature.asc Description: Digital signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
On Tue, 2010-08-24 at 10:12 +0800, Daniel Stone wrote:
> Hi,
>
> On Tue, Aug 24, 2010 at 08:55:22AM +0800, ykzhao wrote:
> > What Mark mentioned is that the CLOCK_MONOTONIC_COARSE posix timer is
> > not supported while the corresponding ID is used for other posix timer.
> > Right?
>
> 6 has no meaning to clock_gettime(). CLOCK_MONOTONIC_COARSE is the only
> thing that has any meaning: if it's not defined, you can't just invent a
> definition and hope that it works. That's why the spec says
> CLOCK_MONOTONIC_COARSE and not 6.
In theory the CLOCK_MONOTONIC_COARSE posix timer id will be defined in
the /usr/include/bits/time.h.
Maybe there is no definition of "CLOCK_MONOTONIC_COARSE"
in /usr/include/bits/time.h when compiling the xorg. But the xorg is
executed on the linux kernel that supports the CLOCK_MONOTONIC_COARSE
posix timer.(In fact for most previous Linux distribution there is no
definition of CLOCK_MONOTONIC_COARSE in /usr/include/bits/time.h).
If it is executed on the kernel that doesn't support the
CLOCK_MONOTONIC_COARSE timer, it will fallback to the CLOCK_MONOTOIC
posix timer(the function of clock_getres will return the invalid value).
Do we need to consider the above scenario? If the above scenario doesn't
need to be cared, I will update the patch to assure that
the CLOCK_MONOTONIC_COARSE posix timer will be tried only when there
exists the corresponding definition.
> > If so, is there an approach that helps us to detect whether the
> > CLOCK_MONOTONIC_COARSE posix timer is supported on one OS?
>
> Try the following completely untested patch (hey, it compiles). It's
> not perfect though: if CLOCK_MONOTONIC or CLOCK_MONOTONIC_COARSE were
> ever 0, we'd make two or three syscalls for GetTimeInMillis() instead of
> one, and if either of them were ~0L, we'd never use them.
the corresponding code is put under the condition definition of
MONOTONIC_CLOCK. This is already checked by using configure script.
In theory the CLOCK_MONOTONIC exists if the MONOTONIC_CLOCK is
defined. Not sure whether it is still necessary to check the
CLOCK_MONOTONIC again?
Best regards.
>
> Cheers,
> Daniel
>
> diff --git a/os/utils.c b/os/utils.c
> index 51455cc..a1659ec 100644
> --- a/os/utils.c
> +++ b/os/utils.c
> @@ -427,7 +427,20 @@ GetTimeInMillis(void)
>
> #ifdef MONOTONIC_CLOCK
> struct timespec tp;
> -if (clock_gettime(CLOCK_MONOTONIC, &tp) == 0)
> +static clockid_t clockid;
> +if (!clockid) {
> +#ifdef CLOCK_MONOTONIC_COARSE
> +if (clock_getres(CLOCK_MONOTONIC_COARSE, &tp) == 0 &&
> +(tp.tv_nsec / 1000) <= 1000)
> +clockid = CLOCK_MONOTONIC_COARSE;
> +else
> +#endif
> +if (clock_gettime(CLOCK_MONOTONIC, &tp) == 0)
> +clockid = CLOCK_MONOTONIC;
> +else
> +clockid = ~0L;
> +}
> +if (clockid != ~0L && clock_gettime(clockid, &tp) == 0)
> return (tp.tv_sec * 1000) + (tp.tv_nsec / 100L);
> #endif
>
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
Hi,
On Tue, Aug 24, 2010 at 08:55:22AM +0800, ykzhao wrote:
> What Mark mentioned is that the CLOCK_MONOTONIC_COARSE posix timer is
> not supported while the corresponding ID is used for other posix timer.
> Right?
6 has no meaning to clock_gettime(). CLOCK_MONOTONIC_COARSE is the only
thing that has any meaning: if it's not defined, you can't just invent a
definition and hope that it works. That's why the spec says
CLOCK_MONOTONIC_COARSE and not 6.
> If so, is there an approach that helps us to detect whether the
> CLOCK_MONOTONIC_COARSE posix timer is supported on one OS?
Try the following completely untested patch (hey, it compiles). It's
not perfect though: if CLOCK_MONOTONIC or CLOCK_MONOTONIC_COARSE were
ever 0, we'd make two or three syscalls for GetTimeInMillis() instead of
one, and if either of them were ~0L, we'd never use them.
Cheers,
Daniel
diff --git a/os/utils.c b/os/utils.c
index 51455cc..a1659ec 100644
--- a/os/utils.c
+++ b/os/utils.c
@@ -427,7 +427,20 @@ GetTimeInMillis(void)
#ifdef MONOTONIC_CLOCK
struct timespec tp;
-if (clock_gettime(CLOCK_MONOTONIC, &tp) == 0)
+static clockid_t clockid;
+if (!clockid) {
+#ifdef CLOCK_MONOTONIC_COARSE
+if (clock_getres(CLOCK_MONOTONIC_COARSE, &tp) == 0 &&
+(tp.tv_nsec / 1000) <= 1000)
+clockid = CLOCK_MONOTONIC_COARSE;
+else
+#endif
+if (clock_gettime(CLOCK_MONOTONIC, &tp) == 0)
+clockid = CLOCK_MONOTONIC;
+else
+clockid = ~0L;
+}
+if (clockid != ~0L && clock_gettime(clockid, &tp) == 0)
return (tp.tv_sec * 1000) + (tp.tv_nsec / 100L);
#endif
signature.asc
Description: Digital signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
On Tue, Aug 24, 2010 at 08:32:48AM +0800, ykzhao wrote: > On Mon, 2010-08-23 at 23:25 +0800, Adam Jackson wrote: > > On Mon, 2010-08-23 at 10:23 +0200, Mark Kettenis wrote: > > > > diff --git a/os/utils.c b/os/utils.c > > > > index 51455cc..a08d591 100644 > > > > --- a/os/utils.c > > > > +++ b/os/utils.c > > > > @@ -242,6 +242,10 @@ OsSignal(int sig, OsSigHandlerPtr handler) > > > > #endif > > > > #endif > > > > > > > > +#ifndef CLOCK_MONOTONIC_COARSE > > > > +#define CLOCK_MONOTONIC_COARSE 6 > > > > +#endif > > > > > > What if an OS doesn't have CLOCK_MONOTONIC_COARSE, but uses the clock > > > ID 6 for some other purpose? > > > > Then this patch would be wrong. > > > > NAK on that basis. > > Yes. Agree. > > How about using the constant value(6) directly? That doesn't change anything - if a system is using ID 6 for something else, then using 6 is wholly incorrect, no matter whether you use the constant directly, define some other symbol for it, or whatever. #ifdef CLOCK_MONOTONIC_COARSE /* include support for CLOCK_MONOTONIC_COARSE */ #else /* it's not there so don't */ #endif signature.asc Description: Digital signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
On Tue, 2010-08-24 at 09:16 +0800, Samuel Thibault wrote:
> ykzhao, le Tue 24 Aug 2010 09:07:49 +0800, a écrit :
> > +static clockid_t clockid;
> > +if (!clockid) {
> > +#ifdef __linux__
> > +#ifndef CLOCK_MONONOTIC_COARSE
> > +#define CLOCK_MONOTONIC_COARSE
>
> If you don't provide the value 6 here, it's completely useless.
Yes. You are right. It should be "#define CLOCK_MONOTONIC_COARSE 6".
Sorry for my fault.
Thanks.
Yakui
>
> > +#endif
> > + if ((clock_getres(CLOCK_MONOTONIC_COARSE, &tp) == 0) &&
>
> Samuel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
ykzhao, le Tue 24 Aug 2010 09:07:49 +0800, a écrit :
> +static clockid_t clockid;
> +if (!clockid) {
> +#ifdef __linux__
> +#ifndef CLOCK_MONONOTIC_COARSE
> +#define CLOCK_MONOTONIC_COARSE
If you don't provide the value 6 here, it's completely useless.
> +#endif
> + if ((clock_getres(CLOCK_MONOTONIC_COARSE, &tp) == 0) &&
Samuel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
On Tue, 2010-08-24 at 08:42 +0800, Samuel Thibault wrote:
> ykzhao, le Tue 24 Aug 2010 08:32:48 +0800, a écrit :
> > On Mon, 2010-08-23 at 23:25 +0800, Adam Jackson wrote:
> > > On Mon, 2010-08-23 at 10:23 +0200, Mark Kettenis wrote:
> > > > > From: yakui.z...@intel.com
> > > > > Date: Mon, 23 Aug 2010 15:20:05 +0800
> > > > > From: Zhao Yakui
> > > > >
> > > > > ---
> > > > > os/utils.c | 14 +-
> > > > > 1 files changed, 13 insertions(+), 1 deletions(-)
> > > > >
> > > > > diff --git a/os/utils.c b/os/utils.c
> > > > > index 51455cc..a08d591 100644
> > > > > --- a/os/utils.c
> > > > > +++ b/os/utils.c
> > > > > @@ -242,6 +242,10 @@ OsSignal(int sig, OsSigHandlerPtr handler)
> > > > > #endif
> > > > > #endif
> > > > >
> > > > > +#ifndef CLOCK_MONOTONIC_COARSE
> > > > > +#define CLOCK_MONOTONIC_COARSE 6
> > > > > +#endif
> > > >
> > > > What if an OS doesn't have CLOCK_MONOTONIC_COARSE, but uses the clock
> > > > ID 6 for some other purpose?
> > >
> > > Then this patch would be wrong.
> > >
> > > NAK on that basis.
> >
> > Yes. Agree.
> >
> > How about using the constant value(6) directly?
>
> Err, you must be kidding...
>
> #ifdef __linux__
> # ifndef CLOCK_MONOTONIC_COARSE
> # define CLOCK_MONOTONIC_COARSE 6
> # endif
> #endif
>
> should however be fine.
How about the following code? It is only applied to Linux platform.
#ifdef MONOTONIC_CLOCK
struct timespec tp;
-if (clock_gettime(CLOCK_MONOTONIC, &tp) == 0)
+static clockid_t clockid;
+if (!clockid) {
+#ifdef __linux__
+#ifndef CLOCK_MONONOTIC_COARSE
+#define CLOCK_MONOTONIC_COARSE
+#endif
+ if ((clock_getres(CLOCK_MONOTONIC_COARSE, &tp) == 0) &&
+ (tp.tv_nsec / 1000 <= 1000))
+ clockid = CLOCK_MONOTONIC_COARSE;
+ else
+ clockid = CLOCK_MONOTONIC;
+#else
+ clockid = CLOCK_MONOTONIC;
+#endif
+ }
+if (clock_gettime(clockid, &tp) == 0)
return (tp.tv_sec * 1000) + (tp.tv_nsec / 100L);
#endif
>
> Samuel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
On Tue, 2010-08-24 at 08:32 +0800, ykzhao wrote: > On Mon, 2010-08-23 at 23:25 +0800, Adam Jackson wrote: > > On Mon, 2010-08-23 at 10:23 +0200, Mark Kettenis wrote: > > > > From: yakui.z...@intel.com > > > > Date: Mon, 23 Aug 2010 15:20:05 +0800 > > > > From: Zhao Yakui > > > > > > > > --- > > > > os/utils.c | 14 +- > > > > 1 files changed, 13 insertions(+), 1 deletions(-) > > > > > > > > diff --git a/os/utils.c b/os/utils.c > > > > index 51455cc..a08d591 100644 > > > > --- a/os/utils.c > > > > +++ b/os/utils.c > > > > @@ -242,6 +242,10 @@ OsSignal(int sig, OsSigHandlerPtr handler) > > > > #endif > > > > #endif > > > > > > > > +#ifndef CLOCK_MONOTONIC_COARSE > > > > +#define CLOCK_MONOTONIC_COARSE 6 > > > > +#endif > > > > > > What if an OS doesn't have CLOCK_MONOTONIC_COARSE, but uses the clock > > > ID 6 for some other purpose? > > > > Then this patch would be wrong. > > > > NAK on that basis. > > Yes. Agree. > > How about using the constant value(6) directly? Sorry that I misunderstand it.(I misunderstand it as the incorrect macro definition.) What Mark mentioned is that the CLOCK_MONOTONIC_COARSE posix timer is not supported while the corresponding ID is used for other posix timer. Right? If so, is there an approach that helps us to detect whether the CLOCK_MONOTONIC_COARSE posix timer is supported on one OS? Thanks. > > > > > - ajax > > ___ > xorg-devel@lists.x.org: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
ykzhao, le Tue 24 Aug 2010 08:32:48 +0800, a écrit : > On Mon, 2010-08-23 at 23:25 +0800, Adam Jackson wrote: > > On Mon, 2010-08-23 at 10:23 +0200, Mark Kettenis wrote: > > > > From: yakui.z...@intel.com > > > > Date: Mon, 23 Aug 2010 15:20:05 +0800 > > > > From: Zhao Yakui > > > > > > > > --- > > > > os/utils.c | 14 +- > > > > 1 files changed, 13 insertions(+), 1 deletions(-) > > > > > > > > diff --git a/os/utils.c b/os/utils.c > > > > index 51455cc..a08d591 100644 > > > > --- a/os/utils.c > > > > +++ b/os/utils.c > > > > @@ -242,6 +242,10 @@ OsSignal(int sig, OsSigHandlerPtr handler) > > > > #endif > > > > #endif > > > > > > > > +#ifndef CLOCK_MONOTONIC_COARSE > > > > +#define CLOCK_MONOTONIC_COARSE 6 > > > > +#endif > > > > > > What if an OS doesn't have CLOCK_MONOTONIC_COARSE, but uses the clock > > > ID 6 for some other purpose? > > > > Then this patch would be wrong. > > > > NAK on that basis. > > Yes. Agree. > > How about using the constant value(6) directly? Err, you must be kidding... #ifdef __linux__ # ifndef CLOCK_MONOTONIC_COARSE # define CLOCK_MONOTONIC_COARSE 6 # endif #endif should however be fine. Samuel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
On Mon, 2010-08-23 at 23:25 +0800, Adam Jackson wrote: > On Mon, 2010-08-23 at 10:23 +0200, Mark Kettenis wrote: > > > From: yakui.z...@intel.com > > > Date: Mon, 23 Aug 2010 15:20:05 +0800 > > > From: Zhao Yakui > > > > > > --- > > > os/utils.c | 14 +- > > > 1 files changed, 13 insertions(+), 1 deletions(-) > > > > > > diff --git a/os/utils.c b/os/utils.c > > > index 51455cc..a08d591 100644 > > > --- a/os/utils.c > > > +++ b/os/utils.c > > > @@ -242,6 +242,10 @@ OsSignal(int sig, OsSigHandlerPtr handler) > > > #endif > > > #endif > > > > > > +#ifndef CLOCK_MONOTONIC_COARSE > > > +#define CLOCK_MONOTONIC_COARSE 6 > > > +#endif > > > > What if an OS doesn't have CLOCK_MONOTONIC_COARSE, but uses the clock > > ID 6 for some other purpose? > > Then this patch would be wrong. > > NAK on that basis. Yes. Agree. How about using the constant value(6) directly? > > - ajax ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
On Mon, 2010-08-23 at 10:23 +0200, Mark Kettenis wrote: > > From: yakui.z...@intel.com > > Date: Mon, 23 Aug 2010 15:20:05 +0800 > > From: Zhao Yakui > > > > --- > > os/utils.c | 14 +- > > 1 files changed, 13 insertions(+), 1 deletions(-) > > > > diff --git a/os/utils.c b/os/utils.c > > index 51455cc..a08d591 100644 > > --- a/os/utils.c > > +++ b/os/utils.c > > @@ -242,6 +242,10 @@ OsSignal(int sig, OsSigHandlerPtr handler) > > #endif > > #endif > > > > +#ifndef CLOCK_MONOTONIC_COARSE > > +#define CLOCK_MONOTONIC_COARSE 6 > > +#endif > > What if an OS doesn't have CLOCK_MONOTONIC_COARSE, but uses the clock > ID 6 for some other purpose? Then this patch would be wrong. NAK on that basis. - ajax signature.asc Description: This is a digitally signed message part ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] use CLOCK_MONOTONIC_COARSE posix timer instead of CLOCK_MONOTONIC in Xorg
> From: yakui.z...@intel.com > Date: Mon, 23 Aug 2010 15:20:05 +0800 > From: Zhao Yakui > > --- > os/utils.c | 14 +- > 1 files changed, 13 insertions(+), 1 deletions(-) > > diff --git a/os/utils.c b/os/utils.c > index 51455cc..a08d591 100644 > --- a/os/utils.c > +++ b/os/utils.c > @@ -242,6 +242,10 @@ OsSignal(int sig, OsSigHandlerPtr handler) > #endif > #endif > > +#ifndef CLOCK_MONOTONIC_COARSE > +#define CLOCK_MONOTONIC_COARSE 6 > +#endif What if an OS doesn't have CLOCK_MONOTONIC_COARSE, but uses the clock ID 6 for some other purpose? ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
