All of lore.kernel.org
 help / color / mirror / Atom feed
* monotonic time for mount.cifs timeouts
@ 2010-08-24 16:16 Björn JACKE
       [not found] ` <E1OnwAo-008Rzd-Bd-dqLtpHMqGvUyWpdLl23E4A@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Björn JACKE @ 2010-08-24 16:16 UTC (permalink / raw)
  To: linux-cifs-u79uwXL29TY76Z2rM5mHXA


[-- Attachment #1.1: Type: text/plain, Size: 220 bytes --]

Hi,

attached are patches to spot the clock_gettime function and to use that and the
monotonic clock for timeout handling if possible.

If there are no objections, can you please apply them, Jeff?

Thanks
Björn

[-- Attachment #1.2: 0001-autoconf-add-checks-for-clock_gettime.patch --]
[-- Type: text/x-patch, Size: 1199 bytes --]

From 250d5b5df23d93ebed66a4c39bccf3a3614c54ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Jacke?= <bj-3ekOc4rQMZmzQB+pC5nmwQ@public.gmane.org>
Date: Tue, 24 Aug 2010 09:39:32 +0200
Subject: [PATCH 1/2] autoconf: add checks for clock_gettime

---
 configure.ac |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/configure.ac b/configure.ac
index c7d420d..7f82cd6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -110,6 +110,16 @@ AC_FUNC_STRNLEN
 # check for required functions
 AC_CHECK_FUNCS([alarm atexit endpwent getmntent getpass gettimeofday inet_ntop memset realpath setenv strchr strcmp strdup strerror strncasecmp strndup strpbrk strrchr strstr strtol strtoul tolower uname], , [AC_MSG_ERROR([necessary functions(s) not found])])
 
+AC_CHECK_FUNCS(clock_gettime, [], [
+  AC_CHECK_LIB(rt, clock_gettime, [
+      AC_DEFINE(HAVE_CLOCK_GETTIME, 1)
+	AC_DEFINE(HAVE_CLOCK_GETTIME,1,
+		[Whether the clock_gettime func is there])
+      LIBS="$LIBS -lrt"
+        ])
+  ])
+
+
 # ugly, but I'm not sure how to check for functions in a library that's not in $LIBS
 cu_saved_libs=$LIBS
 LIBS="$LIBS $KRB5_LDADD"
-- 
1.7.1


[-- Attachment #1.3: 0002-mount.cifs-use-monotonic-time-for-timeouts.patch --]
[-- Type: text/x-patch, Size: 2138 bytes --]

From 4e8d243b9b0a5eb7bf4619c4cddbd9d3a8f4eb2d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Jacke?= <bj-3ekOc4rQMZmzQB+pC5nmwQ@public.gmane.org>
Date: Tue, 24 Aug 2010 09:41:01 +0200
Subject: [PATCH 2/2] mount.cifs: use monotonic time for timeouts

this is especially important during the boot process, where the clock is often
being set initially and clock jumps are more common.
---
 mtab.c |   23 ++++++++++++++++++++---
 1 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/mtab.c b/mtab.c
index de1aabd..64e7250 100644
--- a/mtab.c
+++ b/mtab.c
@@ -39,6 +39,7 @@
 #include <stdlib.h>
 #include <signal.h>
 #include "mount.h"
+#include "config.h"
 
 
 /* Updating mtab ----------------------------------------------*/
@@ -60,6 +61,22 @@ setlkw_timeout (int sig __attribute__((unused))) {
      /* nothing, fcntl will fail anyway */
 }
 
+/* use monotonic time for timeouts */
+struct timeval
+mono_time(void) {
+	struct timeval ret;
+#if defined(HAVE_CLOCK_GETTIME) && defined(CLOCK_MONOTONIC)
+	struct timespec ts;
+	if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
+		ret.tv_sec = ts.tv_sec;
+		ret.tv_usec = ts.tv_nsec/1000;
+		return ret;
+	}
+#endif
+	gettimeofday(&ret,NULL);
+	return ret;
+}
+
 /* Remove lock file.  */
 void
 unlock_mtab (void) {
@@ -150,7 +167,7 @@ lock_mtab (void) {
 	}
 	close(i);
 
-	gettimeofday(&maxtime, NULL);
+	maxtime = mono_time();
 	maxtime.tv_sec += MOUNTLOCK_MAXTIME;
 
 	waittime.tv_sec = 0;
@@ -177,7 +194,7 @@ lock_mtab (void) {
 
 		if (lockfile_fd < 0) {
 			/* Strange... Maybe the file was just deleted? */
-			gettimeofday(&now, NULL);
+			now = mono_time();
 			if (errno == ENOENT && now.tv_sec < maxtime.tv_sec) {
 				we_created_lockfile = 0;
 				continue;
@@ -199,7 +216,7 @@ lock_mtab (void) {
 			(void) unlink(linktargetfile);
 		} else {
 			/* Someone else made the link. Wait. */
-			gettimeofday(&now, NULL);
+			now = mono_time();
 			if (now.tv_sec < maxtime.tv_sec) {
 				alarm(maxtime.tv_sec - now.tv_sec);
 				if (fcntl (lockfile_fd, F_SETLKW, &flock) == -1) {
-- 
1.7.1


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: monotonic time for mount.cifs timeouts
       [not found] ` <E1OnwAo-008Rzd-Bd-dqLtpHMqGvUyWpdLl23E4A@public.gmane.org>
@ 2010-08-24 17:34   ` Jeff Layton
       [not found]     ` <20100824133418.6319ead7-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff Layton @ 2010-08-24 17:34 UTC (permalink / raw)
  To: Björn JACKE; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tue, 24 Aug 2010 18:16:19 +0200
Björn JACKE <bj-PS7XAnAlDA+VvDNblw4Uiw@public.gmane.org> wrote:

> Hi,
> 
> attached are patches to spot the clock_gettime function and to use that and the
> monotonic clock for timeout handling if possible.
> 
> If there are no objections, can you please apply them, Jeff?
> 
> Thanks
> Björn

Sure, they look reasonable to me. If no one chimes in the next couple of
days, I'll plan to apply them.

Thanks for the patches,
-- 
Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: monotonic time for mount.cifs timeouts
       [not found]     ` <20100824133418.6319ead7-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
@ 2010-08-24 18:07       ` Steve French
       [not found]         ` <AANLkTi==5ZbC7Fb8wnCea50OndXZ=QwV5xPwv19K1Fyz-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Steve French @ 2010-08-24 18:07 UTC (permalink / raw)
  To: Jeff Layton; +Cc: Björn JACKE, linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tue, Aug 24, 2010 at 12:34 PM, Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> On Tue, 24 Aug 2010 18:16:19 +0200
> Björn JACKE <bj-PS7XAnAlDA+VvDNblw4Uiw@public.gmane.org> wrote:
>
>> Hi,
>>
>> attached are patches to spot the clock_gettime function and to use that and the
>> monotonic clock for timeout handling if possible.
>>
>> If there are no objections, can you please apply them, Jeff?

Any pointers to background on gettimeofday vs
clock_gettime(CLOCK_MONOTONIC) and why the latter is better?

-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: monotonic time for mount.cifs timeouts
       [not found]         ` <AANLkTi==5ZbC7Fb8wnCea50OndXZ=QwV5xPwv19K1Fyz-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-08-24 18:22           ` Jeff Layton
       [not found]             ` <20100824142248.164caebe-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff Layton @ 2010-08-24 18:22 UTC (permalink / raw)
  To: Steve French; +Cc: Björn JACKE, linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tue, 24 Aug 2010 13:07:55 -0500
Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Any pointers to background on gettimeofday vs
> clock_gettime(CLOCK_MONOTONIC) and why the latter is better?

IIUC, the reason for this is that gettimeofday is affected by wallclock
changes (think NTP):

       CLOCK_MONOTONIC
              Clock  that  cannot  be  set and represents monotonic time since
              some unspecified starting point.

...so by preferring CLOCK_MONOTONIC, the mtab locking code isn't
affected by clock jumps or skew. Bjorn, is this correct?

-- 
Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: monotonic time for mount.cifs timeouts
       [not found]             ` <20100824142248.164caebe-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
@ 2010-08-24 18:34               ` Jeff Layton
       [not found]                 ` <20100824143433.5049ebe1-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff Layton @ 2010-08-24 18:34 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Steve French, Björn JACKE, linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tue, 24 Aug 2010 14:22:48 -0400
Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:

> On Tue, 24 Aug 2010 13:07:55 -0500
> Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
> > Any pointers to background on gettimeofday vs
> > clock_gettime(CLOCK_MONOTONIC) and why the latter is better?
> 
> IIUC, the reason for this is that gettimeofday is affected by wallclock
> changes (think NTP):
> 
>        CLOCK_MONOTONIC
>               Clock  that  cannot  be  set and represents monotonic time since
>               some unspecified starting point.
> 
> ...so by preferring CLOCK_MONOTONIC, the mtab locking code isn't
> affected by clock jumps or skew. Bjorn, is this correct?
> 

Correction...CLOCK_MONOTONIC is affected by adjtimex(). Over a 30s
period though, that shouldn't mean much of a delta. The big danger is
large clock jumps and this should take care of that.

Out of curiousity though...did you or someone you know hit this problem
in practice, Bjorn? Or did you just notice it via inspection?

-- 
Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: monotonic time for mount.cifs timeouts
       [not found]                 ` <20100824143433.5049ebe1-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
@ 2010-08-24 20:08                   ` Björn JACKE
       [not found]                     ` <E1OnznA-008e2X-Fm-dqLtpHMqGvUyWpdLl23E4A@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Björn JACKE @ 2010-08-24 20:08 UTC (permalink / raw)
  To: Jeff Layton; +Cc: Steve French, linux-cifs-u79uwXL29TY76Z2rM5mHXA

On 2010-08-24 at 14:34 -0400 Jeff Layton sent off:
> > > Any pointers to background on gettimeofday vs
> > > clock_gettime(CLOCK_MONOTONIC) and why the latter is better?
> > 
> > IIUC, the reason for this is that gettimeofday is affected by wallclock
> > changes (think NTP):
> > 
> >        CLOCK_MONOTONIC
> >               Clock  that  cannot  be  set and represents monotonic time since
> >               some unspecified starting point.
> > 
> > ...so by preferring CLOCK_MONOTONIC, the mtab locking code isn't
> > affected by clock jumps or skew. Bjorn, is this correct?

yes, correct. In the worst case you start a timeout period while the clock is
set to 2020, during the timeout period the clock is corrected by ntpdate. In
that case the timeout using the RTC clock is a bit longer than expected :-)


> Correction...CLOCK_MONOTONIC is affected by adjtimex(). Over a 30s
> period though, that shouldn't mean much of a delta. The big danger is
> large clock jumps and this should take care of that.
> 
> Out of curiousity though...did you or someone you know hit this problem
> in practice, Bjorn? Or did you just notice it via inspection?

you see all kinds of strange timing effects if you have a VM that has
an unstable clock which you need to regulary adjust had via ntpdate for
example.  Programs hang or for a hile or forever in the worst case. I didn't
see a problem in this particular case with mount.cifs but as this is something
which is called at boot time where ntpdate often runs at the sme time to
initally hard reset the clock this is something I thought should to be
adjusted to use the monotonic clock.

Cheers
Björn

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: monotonic time for mount.cifs timeouts
       [not found]                     ` <E1OnznA-008e2X-Fm-dqLtpHMqGvUyWpdLl23E4A@public.gmane.org>
@ 2010-08-24 23:30                       ` Jeff Layton
       [not found]                         ` <20100824193025.24b0f8c4-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Jeff Layton @ 2010-08-24 23:30 UTC (permalink / raw)
  To: Björn JACKE; +Cc: Steve French, linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tue, 24 Aug 2010 22:08:09 +0200
Björn JACKE <bj-PS7XAnAlDA+VvDNblw4Uiw@public.gmane.org> wrote:

> On 2010-08-24 at 14:34 -0400 Jeff Layton sent off:
> > > > Any pointers to background on gettimeofday vs
> > > > clock_gettime(CLOCK_MONOTONIC) and why the latter is better?
> > > 
> > > IIUC, the reason for this is that gettimeofday is affected by wallclock
> > > changes (think NTP):
> > > 
> > >        CLOCK_MONOTONIC
> > >               Clock  that  cannot  be  set and represents monotonic time since
> > >               some unspecified starting point.
> > > 
> > > ...so by preferring CLOCK_MONOTONIC, the mtab locking code isn't
> > > affected by clock jumps or skew. Bjorn, is this correct?
> 
> yes, correct. In the worst case you start a timeout period while the clock is
> set to 2020, during the timeout period the clock is corrected by ntpdate. In
> that case the timeout using the RTC clock is a bit longer than expected :-)
> 
> 
> > Correction...CLOCK_MONOTONIC is affected by adjtimex(). Over a 30s
> > period though, that shouldn't mean much of a delta. The big danger is
> > large clock jumps and this should take care of that.
> > 
> > Out of curiousity though...did you or someone you know hit this problem
> > in practice, Bjorn? Or did you just notice it via inspection?
> 
> you see all kinds of strange timing effects if you have a VM that has
> an unstable clock which you need to regulary adjust had via ntpdate for
> example.  Programs hang or for a hile or forever in the worst case. I didn't
> see a problem in this particular case with mount.cifs but as this is something
> which is called at boot time where ntpdate often runs at the sme time to
> initally hard reset the clock this is something I thought should to be
> adjusted to use the monotonic clock.
> 

Ok, thanks for the clarification. Committed...

This should make the 4.7 release.

Thanks!
-- 
Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: monotonic time for mount.cifs timeouts
       [not found]                         ` <20100824193025.24b0f8c4-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
@ 2010-08-25  0:47                           ` Steve French
  0 siblings, 0 replies; 8+ messages in thread
From: Steve French @ 2010-08-25  0:47 UTC (permalink / raw)
  To: Jeff Layton; +Cc: Björn JACKE, linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tue, Aug 24, 2010 at 6:30 PM, Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> On Tue, 24 Aug 2010 22:08:09 +0200
> Björn JACKE <bj-PS7XAnAlDA+VvDNblw4Uiw@public.gmane.org> wrote:
>
> Ok, thanks for the clarification. Committed...
>
> This should make the 4.7 release.
>
> Thanks!

Yes - makes sense - thanks



-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2010-08-25  0:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-24 16:16 monotonic time for mount.cifs timeouts Björn JACKE
     [not found] ` <E1OnwAo-008Rzd-Bd-dqLtpHMqGvUyWpdLl23E4A@public.gmane.org>
2010-08-24 17:34   ` Jeff Layton
     [not found]     ` <20100824133418.6319ead7-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2010-08-24 18:07       ` Steve French
     [not found]         ` <AANLkTi==5ZbC7Fb8wnCea50OndXZ=QwV5xPwv19K1Fyz-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-08-24 18:22           ` Jeff Layton
     [not found]             ` <20100824142248.164caebe-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2010-08-24 18:34               ` Jeff Layton
     [not found]                 ` <20100824143433.5049ebe1-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2010-08-24 20:08                   ` Björn JACKE
     [not found]                     ` <E1OnznA-008e2X-Fm-dqLtpHMqGvUyWpdLl23E4A@public.gmane.org>
2010-08-24 23:30                       ` Jeff Layton
     [not found]                         ` <20100824193025.24b0f8c4-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
2010-08-25  0:47                           ` Steve French

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.