[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20090922085644.GA5511@linux.vnet.ibm.com>
Date: Tue, 22 Sep 2009 14:26:44 +0530
From: Arun R Bharadwaj <arun@...ux.vnet.ibm.com>
To: Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc: Joel Schopp <jschopp@...tin.ibm.com>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Paul Mackerras <paulus@...ba.org>, Ingo Molnar <mingo@...e.hu>,
Vaidyanathan Srinivasan <svaidy@...ux.vnet.ibm.com>,
Dipankar Sarma <dipankar@...ibm.com>,
Balbir Singh <balbir@...ibm.com>,
Gautham R Shenoy <ego@...ibm.com>,
linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
Arun Bharadwaj <arun@...ux.vnet.ibm.com>
Subject: Re: [v5 RFC PATCH 0/7]: cpuidle/x86/POWER (REDESIGN): Cleanup idle
power management code in x86, cleanup drivers/cpuidle/cpuidle.c and
introduce cpuidle to POWER.
* Peter Zijlstra <a.p.zijlstra@...llo.nl> [2009-09-22 09:25:59]:
>
>
> Much better :-)
>
>
> But I'm puzzled by all the per-cpu-ish-ness of the stuff. Why would you
> need to register things on a per-cpu basis?
>
> Also:
>
>
> + list_for_each(pos, &per_cpu(cpuidle_devices_list, cpu)) {
> + temp_dev = container_of(pos, struct cpuidle_device,
> + percpu_list[cpu]);
> + if (dev == temp_dev) {
> + list_del(&temp_dev->percpu_list[cpu]);
> + cpuidle_remove_state_sysfs(temp_dev);
> + }
> + }
>
> Looks buggy, either you want to break out of the loop on dev ==
> temp_dev, or you want to use list_for_each_safe().
>
>
>
Hi Peter,
There were a couple of buggy issues, which i have cleaned up for the
next iteration.
* As you pointed out above, the loop is buggy.
* Also, the percpu_list[NR_CPUS] which i am defining inside
struct cpuidle_device is wrong. It does not need to be an
array.
Thanks for the quick turnaround
arun
--
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/
Powered by blists - more mailing lists