CLOCK_BOOTIME (commit cf87f8587)

[Ralph Böhme]

Hi!

Simo wrote:
> On Mon, Aug 01, 2016 at 03:00:16PM +0200, Volker Lendecke wrote:
> > Just talked to Björn Jacke: He told me that commit cf87f8587 was
> > triggered by a suggestion you made. CLOCK_BOOTTIME is the better
> > alternative than CLOCK_MONOTONIC. I don't fully understand it, but
> > what I get is that CLOCK_BOOTTIME correctly takes care of a machine in
> > suspend mode.
> > 
> > This of course makes a lot of sense for laptops, but it comes at
> > a cost: If I run the attached program on a PPC machine I get that
> > calling CLOCK_BOOTTIME is roughly 8 times more expensive than calling
> > CLOCK_MONOTONIC. Christof Schmitt has figured out that this is due to
> > CLOCK_BOOTTIME being a real syscall whereas CLOCK_MONOTONIC is a pure
> > read from a kernel page like getpid() is.
> > 
> > [root at p8-10-rhel-71be-01 vlendeke]# time ./timetest 100000000
> > real    0m26.443s
> > user    0m7.298s
> > sys     0m19.145s
> > [root at p8-10-rhel-71be-01 vlendeke]# time ./timetest 100000000 mono
> > real    0m3.253s
> > user    0m3.244s
> > sys     0m0.010s
> > 
> > Simo, do you really see the need for CLOCK_BOOTTIME for your RHEL
> > deployments? Or could you live with CLOCK_MONOTONIC only?
> > 
> > If your customers need CLOCK_BOOTTIME, has RedHat the capability to
> > make CLOCK_BOOTTIME a kernel page read too, so that it is just as
> > cheap as CLOCK_MONOTONIC?
> 
> Hi Volker,
> I think it is fine to use clock monotonic in the file server, the main reason
> to use clock boottime was for winbind when used on laptops indeed.
> 
> If it is possible to use one for the file server and the other for winbindd
> that would be nice, otherwise feel free to switch to monotonic.

would a build time option help? Something like the attached?

We want to add any overhead at runtime to clock_gettime_mono(), so I
don't see a way of making this a runtime option.

Cheerio!
-slow
-------------- next part --------------
From 56319227b96b2891a48305c13a6b0df663f69e87 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sat, 13 Aug 2016 08:37:24 +0200
Subject: [PATCH] lib/util: add --with-clock-boottime

Since cf87f8587415df2119995e82ccf51bb64e44115b we prefer CLOCK_BOOTTIME
over CLOCK_MONOTONIC, but the former is a costly syscall while the
latter is a simple read from vdso.

This commit effectively reverts cf87f8587415df2119995e82ccf51bb64e44115b,
choosing CLOCK_MONOTONIC by default and instead adds a configure option
to prefer CLOCK_BOOTTIME.

Using CLOCK_BOOTTIME can be useful in winbindd on laptops.

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 lib/util/time.c            | 2 +-
 lib/util/wscript           | 4 ++++
 lib/util/wscript_configure | 4 ++++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/util/time.c b/lib/util/time.c
index 8c01627..f3a71a2 100644
--- a/lib/util/time.c
+++ b/lib/util/time.c
@@ -54,7 +54,7 @@ a wrapper to preferably get the monotonic time
 _PUBLIC_ void clock_gettime_mono(struct timespec *tp)
 {
 /* prefer a suspend aware monotonic CLOCK_BOOTTIME: */
-#ifdef CLOCK_BOOTTIME
+#if (CLOCK_TYPE == CLOCK_BOOTTIME) && defined(CLOCK_BOOTTIME)
 	if (clock_gettime(CLOCK_BOOTTIME,tp) == 0) {
 		return;
 	}
diff --git a/lib/util/wscript b/lib/util/wscript
index 953becf..303e0ad 100644
--- a/lib/util/wscript
+++ b/lib/util/wscript
@@ -21,3 +21,7 @@ def set_options(opt):
     opt.add_option('--with-gpfs',
                    help=("Directory under which gpfs headers are installed"),
                    action="store", dest='gpfs_headers_dir', default="/usr/lpp/mmfs/include/")
+
+    opt.add_option('--with-clock-boottime',
+                  help=("Use CLOCK_BOOTTIME instead of CLOCK_MONOTONIC (default=CLOCK_MONOTONIC)"),
+                  action="store_true", dest='use_clock_boottime')
diff --git a/lib/util/wscript_configure b/lib/util/wscript_configure
index a1e5801..9a7c11b 100644
--- a/lib/util/wscript_configure
+++ b/lib/util/wscript_configure
@@ -138,3 +138,7 @@ else:
 conf.env['CPPPATH_GPFS'] = Options.options.gpfs_headers_dir
 if conf.CHECK_HEADERS('gpfs.h', False, False, "gpfs"):
     conf.DEFINE('HAVE_GPFS', '1')
+
+conf.DEFINE('CLOCK_TYPE', 'CLOCK_MONOTONIC')
+if Options.options.use_clock_boottime:
+    conf.DEFINE('CLOCK_TYPE', 'CLOCK_BOOTTIME')
-- 
2.7.4