All of lore.kernel.org
 help / color / mirror / Atom feed
From: john stultz <johnstul@us.ibm.com>
To: Vinay Sridhar <vinay@linux.vnet.ibm.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [Patch] Fix sys_time to handle intra-tick correction
Date: Tue, 09 Dec 2008 18:25:28 -0800	[thread overview]
Message-ID: <1228875929.6022.6.camel@localhost> (raw)
In-Reply-To: <1228868667.6758.176.camel@localhost>

On Tue, 2008-12-09 at 16:24 -0800, john stultz wrote:
> On Tue, 2008-12-09 at 17:28 +0530, Vinay Sridhar wrote:
> > Hi All,
> > 
> > This fix changes sys_time to use do_gettimeofday instead of get_seconds.
> > Running the stime01 test from LTP triggers this error. Calling sys_stime
> > and then calling sys_time causes this. "do_settimeofday" subtracts the
> > nsec offset from the nsec value(0 in this case) passed to it.
> > Subsequently, "set_normalized_timespec" modifies sec and nsec
> > accordingly. This compensation is handled in the do_gettimeofday path.
> > However, sys_time does not handle this case and reports an incorrect
> > seconds value.
> > 
> > 
> > signed-off by : Vinay Sridhar <vinay@linux.vnet.ibm.com>
> 
> This does fix the symmetry by using the same interfaces on both sides,
> so it seems reasonable to me. However, since do_gettimeofday reads the
> clocksource hardware, this may have a slight performance impact to
> time() users.
> 
> I know privately I said this patch looked fine earlier, so sorry for the
> mixed messages, but I really think we should be able to use
> get_seconds() without this problem.
> 
> Looking more carefully at the code in do_settimeofday(), we should
> accumulate the left over cycles, then we set the current time to the
> specified time and update the xtime_cache.
> 
> So I'm not sure if we're really fixing the right issue here. Further,
> running on the x86_64 system, I don't see this issue crop up at all (for
> either 64 or 32 bit binaries). I think you mentioned privately that this
> was seen on i386, so maybe we need to dig just a bit deeper before going
> with this fix.
> 
> And sorry again for running hot then cold on this patch. I just didn't
> look deeply enough at the issue the first time around. :(

Hey Vinay,
	So I spent some time checking various kernels and it seems this issue
is already resolved upstream by the clocksource_forward_now() change:
9a055117d3d9cb562f83f8d4cd88772761f4cab0

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=9a055117d3d9cb562f83f8d4cd88772761f4cab0

thanks
-john




      reply	other threads:[~2008-12-10  2:25 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-09 11:58 [Patch] Fix sys_time to handle intra-tick correction Vinay Sridhar
2008-12-10  0:24 ` john stultz
2008-12-10  2:25   ` john stultz [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1228875929.6022.6.camel@localhost \
    --to=johnstul@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=vinay@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.