On Fri, 2010-08-27 at 18:51 +0800, Daniel Stone wrote: > On Fri, Aug 27, 2010 at 05:32:55PM +0800, ykzhao wrote: > > On Fri, 2010-08-27 at 10:15 +0800, Daniel Stone wrote: > > > On Fri, Aug 27, 2010 at 09:20:59AM +0800, yakui.z...@intel.com wrote: > > > > Limit the CLOCK_MONOTONIC_COARSE posix timer to linux platform. This > > > > is > > > > to avoid the issue that OS doesn't support CLOCK_MONOTINC_COARSE posix > > > > timer > > > > while the corresponding clock id is for other purpose. At the same time > > > > it > > > > won't be created if the CLOCK_MONOTONIC_COARSE is defined in system > > > > header > > > > file. > > > > > > ... what?? CLOCK_MONOTONIC_COARSE _always_ means the coarse monotonic > > > clock if defined, on any platform. It's always safe to use. > > > > Do you mean that it is safe to use the coarse monotonic clock if it is > > defined? Right? > > Yes. > > > But in the discussion of V1 version, it seems that it is not reasonable > > for other OS. Maybe the CLOCK_MONOTONIC_COARSE posix timer is not > > supported. But the corresponding clock id is used for other purpose. In > > such case it will be not safe. > > I don't think the point's getting across, and for a patch that reduces > the CPU load _less than the measurement error_, it doesn't really seem > worth discussing more. Can you please test the attached patch and give > your Tested-by/Reviewed-by if OK? I'd like to end these threads after, > what, 60 mails? More?
I already tested this patch on one machine using HPET as the stable clocksource. The test result shows that the overhead of read_hpet can reduce 12% in course of playing video workload. The patch still looks very good although I have two smaller comments. 1. The more check of CLOCK_MONOTONIC is mixed with the usage of CLOCK_MONOTONIC_COARSE. 2. The check for CLOCK_MONOTONIC_COARSE by using clock_gettime is redundant as the clock_getres already helps to check whether the CLOCK_MONOTONIC_COARSE is supported. Tested-by: Zhao Yakui <yakui.z...@intel.com> Tested-by: Samuel Xu <samuel...@intel.com> > > Cheers, > Daniel > > From 2ba33ed4671f9034172358ddc8d432b0f1730c85 Mon Sep 17 00:00:00 2001 > From: Daniel Stone <dan...@fooishbar.org> > Date: Fri, 27 Aug 2010 20:36:37 +1000 > Subject: [PATCH] GetTimeInMillis: Use CLOCK_MONOTONIC_COARSE where available > > On some systems, using CLOCK_MONOTONIC forces a readback of HPET or some > similarly expensive timer. CLOCK_MONOTONIC_COARSE can alleviate this, > at the cost of negligibly-reduced resolution, so prefer that where we > can. > > Signed-off-by: Daniel Stone <dan...@fooishbar.org> > --- > os/utils.c | 16 +++++++++++++++- > 1 files changed, 15 insertions(+), 1 deletions(-) > > diff --git a/os/utils.c b/os/utils.c > index 7aa392a..efca6b4 100644 > --- a/os/utils.c > +++ b/os/utils.c > @@ -429,7 +429,21 @@ 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 && > + clock_gettime(CLOCK_MONOTONIC_COARSE, &tp) == 0) > + 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 / 1000000L); > #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