linux-kernel - Re: [PATCH 1/3] timekeeping: Add persistent_clock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <50CA8837.5010800@linaro.org>
Date:	Thu, 13 Dec 2012 18:00:23 -0800
From:	John Stultz <john.stultz@...aro.org>
To:	Feng Tang <feng.tang@...el.com>
CC:	Thomas Gleixner <tglx@...utronix.de>,
	Alessandro Zummo <a.zummo@...ertech.it>,
	linux-kernel@...r.kernel.org, alek.du@...el.com,
	jgunthorpe@...idianresearch.com
Subject: Re: [PATCH 1/3] timekeeping: Add persistent_clock_exist flag

On 12/13/2012 05:37 PM, Feng Tang wrote:
> On Thu, Dec 13, 2012 at 05:20:36PM -0800, John Stultz wrote:
>> On 12/12/2012 06:05 PM, Feng Tang wrote:
>>> In current kernel, there are several places which need to check
>>> whether there is a persistent clock for the platform. Current check
>>> is done by calling the read_persistent_clock() and validating the
>>> return value.
>>>
>>> Add such a flag to make code more readable and call read_persistent_clock()
>>> only once for all the checks.
>> Sorry.. What  the actual benefit of this patch set?   (Usually with
>> changelogs its better to explain why you're doing something, rather
>> then just what you're doing.)
> The main benefits is not bother to do the rtc_resume and rtc_suspend work
> if persistent clock exists. Current RTC suspend/resume code will do many
> time calculation and compensation work at first, and then call
> timekeeping_inject_sleeptime() which will just return for platform with
> persistent clock, what I did in this patchset is to put the check at
> the start, also I save the persistent_clock_exist flag for all possible
> check after timekeeping_init().

CC'ing Jason as his recent patch is conceptually connected here.

Ok, Feng, so your patch set is a suspend/resume optimization for the 
case where the architecture has a read_persistent_clock() 
implementation, but the kernel config has also the rtc HCTOSYS_DEVICE 
set, right?

So we basically short-cut the rtc's HCTOSYS_DEVICE suspend/resume logic, 
likely to speed up suspend/resume.

So per Jason's related patch, he's made the point that the 
persistent_clock and RTC class functionality are basically exclusive 
(well, in his case, he said this with respect to updating the RTC, not 
reading it - I don't mean to put words in his mouth - Please do correct 
me here Jason. :).  In other words, we probably should avoid 
configurations where both the rtc hctosys and persistent_clock 
interfaces are both active.

So my thought here is that this same behavioral change could be made via 
Kconfig constraints rather then extra run-time conditionals. Basically 
we add a HAS_PERSISTENT_CLOCK, that architectures select if they want to 
use the read/update_persistent_clock calls. Then we make the HCTOSYS 
option be dependent on !HAS_PERSISTENT_CLOCK. This way we avoid having 
configs where there are conflicting paths that we chose from.

thanks
-john
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/