Messages in this thread | | | Subject | Re: [patch] CFS scheduler, -v19 | From | Ian Kent <> | Date | Thu, 19 Jul 2007 01:31:31 +0800 |
| |
On Wed, 2007-07-18 at 09:03 -0700, Linus Torvalds wrote: > > On Tue, 17 Jul 2007, Ingo Molnar wrote: > > > > * Ian Kent <raven@themaw.net> wrote: > > > > > > In several places I have code similar to: > > > > > > wait.tv_sec = time(NULL) + 1; > > > wait.tv_nsec = 0; > > Ok, that definitely should work. > > Does the patch below help? > > > ah! It passes in a low-res time source into a high-res time interface > > (pthread_cond_timedwait()). Could you change the time(NULL) + 1 to > > time(NULL) + 2, or change it to: > > > > gettimeofday(&wait, NULL); > > wait.tv_sec++; > > This is wrong. It's wrong for two reasons: > > - it really shouldn't be needed. I don't think "time()" has to be > *exactly* in sync, but I don't think it can be off by a third of a > second or whatever (as the "30% CPU load" would seem to imply) > > - gettimeofday works on a timeval, pthread_cond_timedwait() works on a > timespec. > > So if it actually makes a difference, it makes a difference for the > *wrong* reason: the time is still totally nonsensical in the tv_nsec field > (because it actually got filled in with msecs!), but now the tv_sec field > is in sync, so it hides the bug.
Oh ya .. I thought it wouldn't hurt to add the fraction of the current second for correctness and actually put things like:
gettimeofday(&now, NULL); wait.tv_sec = now.tv_sec + 1; wait.tv_nsec = now.tv_usec * 1000;
in autofs.
Ian
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |