linux-kernel - Re: [PATCH][RFC] ACPI throttling: Save/restore tstate for each CPUs across suspend/resume

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] [day] [month] [year] [list]

Date:   Tue, 29 Nov 2016 02:50:08 +0100
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     "Chen, Yu C" <yu.c.chen@...el.com>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
        Rui Wang <ruwang@...hat.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Len Brown <lenb@...nel.org>, Pavel Machek <pavel@....cz>,
        Matthew Garrett <mjg59@...f.ucam.org>,
        "Zhang, Rui" <rui.zhang@...el.com>,
        Linux PM <linux-pm@...r.kernel.org>
Subject: Re: [PATCH][RFC] ACPI throttling: Save/restore tstate for each CPUs
 across suspend/resume

On Tue, Nov 29, 2016 at 2:27 AM, Chen, Yu C <yu.c.chen@...el.com> wrote:
> Hi,
>> -----Original Message-----
>> From: rjwysocki@...il.com [mailto:rjwysocki@...il.com] On Behalf Of
>> Rafael J. Wysocki
>> Sent: Wednesday, November 23, 2016 7:03 AM

[cut]

>>
>> > +               work_on_cpu(cpu, tstate_pm_fn, &save);
>>
>> Does work_on_cpu() wait for the work to complete?
>>
> Yes, it might increase the suspend/resume time, a 'queue_work_on' might be better?

Not really, because you'd need to wait before doing the
put_online_cpus() anyway.

>> > +       put_online_cpus();
>> > +}
>> > +
>> > +static int tstate_suspend(struct notifier_block *nb,
>> > +                               unsigned long mode, void *_unused) {
>> > +       tstate_check(mode, true);
>> > +       return 0;
>> > +}
>> > +
>> > +static int tstate_resume(struct notifier_block *nb,
>> > +                               unsigned long mode, void *_unused) {
>> > +       tstate_check(mode, false);
>> > +       return 0;
>> > +}
>> > +
>> > +static int __init tstate_pm_init(void) {
>> > +       /*
>> > +        * tstate_suspend should save tstate after
>> > +        * thermal zone's update in thermal_pm_notify,
>> > +        * vice versa tstate_resume restore tstate before
>> > +        * thermal_pm_notify, thus the thermal framework
>> > +        * has a chance to re-adjust tstate according to the
>> > +        * temperature trend.
>> > +        */
>> > +       pm_notifier(tstate_suspend, -1);
>> > +       pm_notifier(tstate_resume, 1);
>>
>> I don't think this is going to do what you really want.
>>
>> Each of these notifiers is going to be invoked during both suspend and resume,
> Yes,
>
>> so I guess you only need one notifier?
>
> Here's my original thought:  tstate_suspend needs to be invoked after
>  thermal_pm_notify, which has a priority of '0',
> so the notifier of tstate_suspend should be lower than '0',
> thus '-1'. And the same for tstate_resume,
> it should be invoked before thermal_pm_notify,
> thus priority is '1' ?

If there's a dependency like that, it needs to be explicit.  That is,
thermal_pm_notify() needs to invoke your new code in the right order
with respect to what it does already.

Thanks,
Rafael