linux-kernel - Re: [PATCH v2 2/2] intel_pstate: Set core to min P state during core offline

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]

Message-ID: <5329B873.7070100@intel.com>
Date:	Wed, 19 Mar 2014 08:32:03 -0700
From:	Dirk Brandewie <dirk.brandewie@...il.com>
To:	Viresh Kumar <viresh.kumar@...aro.org>,
	Dirk Brandewie <dirk.brandewie@...il.com>
CC:	dirk.j.brandewie@...el.com,
	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>,
	Linux PM list <linux-pm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Patrick Marlier <patrick.marlier@...il.com>
Subject: Re: [PATCH v2 2/2] intel_pstate: Set core to min P state during core
 offline

On 03/18/2014 10:20 PM, Viresh Kumar wrote:
> On 19 March 2014 01:14, Dirk Brandewie <dirk.brandewie@...il.com> wrote:
>> There was no problem per se.  In stop() all I really needed to do is stop
>> the
>> timer and set the P state to MIN.
>>
>> At init time I need to allocate memory and start timer.  If stopping the
>> timer
>> and deallocating memory are separated then I need code in init() to detect
>> this case.
>
> Sorry, I didn't understood what exactly is special here :(
>
> If we return failure from CPU_POST_DEAD for some reason without
> calling exit() then you will have memory leak in your init() as we are
> allocating memory without checking if we already have that (nothing wrong
> in it though as other parts of kernel should handle things properly here).

No.  If you got the CPU_POST_DEAD callback CPU_DOWN_PREPARE has already
succeeded.  init() is called on the CPU_ONLINE and CPU_DOWN_FAILED path.

The issue is there is a two part teardown that can fail and the teardown
fail will be followed by a call to init().

If the timer is not running (stopped in stop()) then there is no reason to
have the memory around. If CPU_DOWN_PREPARE happens followed by CPU_DOWN_FAILED
then intel_pstate is ready for init() to be called with no special case
code.  This maintains the semantics the core expects.

>
> Probably the situation would be exactly same if we divide the exit path into
> stop and exit routines, which I still feel is the right way forward. Because
> ideally cpufreq shouldn't call init() if it hasn't called exit() (If
> it is doing that
> right now then its wrong and can be fixed). And so you must do the cleanup
> in exit()..
>

The core *is* doing this on the CPU_DOWN_FAILED path ATM.

On the CPU_DOWN_FAILED path the core should be undoing the work it did in the
CPU_DOWN_PREPARE path this would require another callback to drivers to let
them "restart" after a call to stop() as well.

I don't think it is worth that level of effort IMHO.

--Dirk
--
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/