From: Peter Zijlstra <peterz@infradead.org>
To: arun@linux.vnet.ibm.com
Cc: Gautham R Shenoy <ego@in.ibm.com>,
"Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>,
linux-kernel@vger.kernel.org, Paul Mackerras <paulus@samba.org>,
Ingo Molnar <mingo@elte.hu>,
linuxppc-dev@lists.ozlabs.org,
Balbir Singh <balbir@linux.vnet.ibm.com>
Subject: Re: [PATCH 2/4]: CPUIDLE: Introduce architecture independent cpuidle_pm_idle in drivers/cpuidle/cpuidle.c
Date: Fri, 28 Aug 2009 09:01:12 +0200 [thread overview]
Message-ID: <1251442872.18584.125.camel@twins> (raw)
In-Reply-To: <1251442085.18584.120.camel@twins>
On Fri, 2009-08-28 at 08:48 +0200, Peter Zijlstra wrote:
>
> > void cpuidle_install_idle_handler(void)
> > {
> > .........
> > .........
> > cpuidle_pm_idle = cpuidle_idle_call;
> > }
>
> All I'm seeing here is a frigging mess.
>
> How on earths can something called: cpuidle_install_idle_handler() have
> a void argument, _WHAT_ handler is it going to install?
Argh, now I see, it installs itself as the platform idle handler.
so cpuidle_install_idle_handler() pokes at the unmanaged pm_idle pointer
to make cpuidle take control.
On module load it does:
pm_idle_old = pm_idle;
then in the actual idle loop it does:
if (!dev || !dev->enabled) {
if (pm_idle_old)
pm_idle_old();
who is to say that the pointer stored at module init time is still
around at that time?
So cpuidle recognised the pm_idle stuff was a flaky, but instead of
fixing it, they build a whole new layer on top of it. Brilliant.
/me goes mark this whole thread read, I've got enough things to do.
WARNING: multiple messages have this Message-ID (diff)
From: Peter Zijlstra <peterz@infradead.org>
To: arun@linux.vnet.ibm.com
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Joel Schopp <jschopp@austin.ibm.com>,
Paul Mackerras <paulus@samba.org>, Ingo Molnar <mingo@elte.hu>,
Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>,
Dipankar Sarma <dipankar@in.ibm.com>,
Balbir Singh <balbir@linux.vnet.ibm.com>,
Gautham R Shenoy <ego@in.ibm.com>,
"Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>,
linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH 2/4]: CPUIDLE: Introduce architecture independent cpuidle_pm_idle in drivers/cpuidle/cpuidle.c
Date: Fri, 28 Aug 2009 09:01:12 +0200 [thread overview]
Message-ID: <1251442872.18584.125.camel@twins> (raw)
In-Reply-To: <1251442085.18584.120.camel@twins>
On Fri, 2009-08-28 at 08:48 +0200, Peter Zijlstra wrote:
>
> > void cpuidle_install_idle_handler(void)
> > {
> > .........
> > .........
> > cpuidle_pm_idle = cpuidle_idle_call;
> > }
>
> All I'm seeing here is a frigging mess.
>
> How on earths can something called: cpuidle_install_idle_handler() have
> a void argument, _WHAT_ handler is it going to install?
Argh, now I see, it installs itself as the platform idle handler.
so cpuidle_install_idle_handler() pokes at the unmanaged pm_idle pointer
to make cpuidle take control.
On module load it does:
pm_idle_old = pm_idle;
then in the actual idle loop it does:
if (!dev || !dev->enabled) {
if (pm_idle_old)
pm_idle_old();
who is to say that the pointer stored at module init time is still
around at that time?
So cpuidle recognised the pm_idle stuff was a flaky, but instead of
fixing it, they build a whole new layer on top of it. Brilliant.
/me goes mark this whole thread read, I've got enough things to do.
next prev parent reply other threads:[~2009-08-28 7:03 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-08-27 11:49 [v3 PATCH 0/4]: CPUIDLE/POWER: Introducing cpuidle infrastructure to POWER Arun R Bharadwaj
2009-08-27 11:49 ` Arun R Bharadwaj
2009-08-27 11:51 ` [PATCH 1/4]: CPUIDLE/POWER: Enable cpuidle for pSeries Arun R Bharadwaj
2009-08-27 11:51 ` Arun R Bharadwaj
2009-08-27 11:53 ` [PATCH 2/4]: CPUIDLE: Introduce architecture independent cpuidle_pm_idle in drivers/cpuidle/cpuidle.c Arun R Bharadwaj
2009-08-27 11:53 ` Arun R Bharadwaj
2009-08-27 12:53 ` Peter Zijlstra
2009-08-27 12:53 ` Peter Zijlstra
2009-08-27 21:28 ` Benjamin Herrenschmidt
2009-08-27 21:28 ` Benjamin Herrenschmidt
2009-08-28 4:49 ` Arun R Bharadwaj
2009-08-28 4:49 ` Arun R Bharadwaj
2009-08-28 6:40 ` Peter Zijlstra
2009-08-28 6:40 ` Peter Zijlstra
2009-08-28 6:14 ` Arun R Bharadwaj
2009-08-28 6:14 ` Arun R Bharadwaj
2009-08-28 6:48 ` Peter Zijlstra
2009-08-28 6:48 ` Peter Zijlstra
2009-08-28 6:59 ` Balbir Singh
2009-08-28 6:59 ` Balbir Singh
2009-08-28 7:01 ` Peter Zijlstra [this message]
2009-08-28 7:01 ` Peter Zijlstra
2009-08-28 8:19 ` Vaidyanathan Srinivasan
2009-08-28 8:19 ` Vaidyanathan Srinivasan
2009-08-28 6:43 ` Arun R Bharadwaj
2009-08-28 6:43 ` Arun R Bharadwaj
2009-08-27 11:55 ` [PATCH 3/4]: ACPI/ARM: Register for cpuidle_pm_idle in drivers/acpi/processor_idle.c and arch/arm/mach-kirkwood/cpuidle.c Arun R Bharadwaj
2009-08-27 11:55 ` Arun R Bharadwaj
2009-08-27 11:57 ` [PATCH 4/4]: CPUIDLE/POWER: Implement Pseries Processor Idle module Arun R Bharadwaj
2009-08-27 11:57 ` Arun R Bharadwaj
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=1251442872.18584.125.camel@twins \
--to=peterz@infradead.org \
--cc=arun@linux.vnet.ibm.com \
--cc=balbir@linux.vnet.ibm.com \
--cc=ego@in.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mingo@elte.hu \
--cc=paulus@samba.org \
--cc=venkatesh.pallipadi@intel.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.