Re: [RFC PATCH 0/5] Add support for S3 non-stop TSC support.

[Jason Gunthorpe <jgunthorpe@obsidianresearch.com>]

On Tue, Jan 22, 2013 at 07:07:04PM -0800, John Stultz wrote:

> But personally, I'm less fond of adding additional state to the
> clocksources, as I'm (admittedly, very) slowly trying to go the
> other way, and make the clocksources mostly state free. This is in
> part to allow for faster timekeeping updates (see:
> https://lkml.org/lkml/2012/3/2/66) - but again, I've not made much
> progress there recently, so this probably isn't a strong enough
> argument against it.

I think there should be ways to avoid storing the suspend time in the
clocksource struct, but since the suspend time is orthogonal to
timekeeping updates maybe it doesn't matter?

> Another downside is that accessing a clocksource can be costly, so
> doing so for every clocksource could unnecessarily slow
> suspend/resume down. Reading all the clocksources avoids the
> complexity of creating the secondary selection and management of a
> suspend-time measuring clocksource, but it also feels a little
> hackish to me. And iterating over the clocksource list requires
> exposing currently private clocksource data to the timekeeping core.

I was imagining these functions would be in the clocksource code and
called from suspend (clocksource_suspend_prepare,
clocksource_suspend_delta or some such). Not sure on iteration
expense, but you only need to look at clock sources that have a
active_during_suspend function pointer, so there would be various ways
to minimize the cost of finding that list, including precomputing it
during clocksource registration.

Generally there would be 0 or 1 active_during_suspend sources, I
expect. So in practice this probably boils down to locking only one
clocksource.

> The reason I like the idea of a new persistent_clock api, is that it
> formalizes existing usage, and doesn't require changes to the
> timekeeping logic, or to architectures that don't have running

Having seen ARM go through so many iterations of removing these sorts
of non-driver APIs and moving to dynamic bindings just makes it seem
wrong to add more, especially when the API is expected to work with
hardware already handled by a dynamically bound driver.

> But don't let my naysaying stop you from submitting a patch. It
> would be interesting to see your idea fully fleshed out.

Maybe Feng will try a v2 of his patch with some of these ideas? He has
hardware to test it :) I agree it would be clearer to see with code!!
 
> I appreciate your persistence here, and apologies for my thick-headed-ness.

NP

Regards,
Jason