Messages in this thread |  | | Date | Fri, 25 Mar 2011 04:12:03 -0400 (EDT) | From | Len Brown <> | Subject | Re: [RFC PATCH V1 1/2] cpuidle: Data structure changes for global cpuidle device |
| |
I agree it is silly to allocate a cpuidle_device for every cpu in the system as we do today.
Yes, splitting the counters out of cpuidle_device is a necessary part of fixing that.
However, cpuidle_device.cpuidle_state[] is currently not per-driver, it is per-cpu, and it is writable.
In particular, the cpuidle_device->prepare() mechanism causes updates to the cpuidle_state[].flags, setting and clearing CPUIDLE_FLAG_IGNORE to tell the governor not to chose a state on a per-cpu basis at run-time.
I don't like that mechanism. I'd like to see it replaced, and when replaced, cpuidle_state[] can be per system-wide driver.
I think the real problem that prepare() was trying to solve is that the driver today does not have the ability to over-rule the choice made by the governor. The driver may discover in the course of trying to satisfy the request of the governor that it needs to demote to a shallower state; or it may do its best to satisfy the governor's request, and the hardware may demote its request to a shallower state.
Unfortunately, when this happens, the driver dutifully returns the time spent in the state to cpuidle_idle_call(), who then updates the wrong last_residency, time, and usage counters.
Sure is ironic for the driver to allocate the data structures and then hand the timer to the uppper layer, just to have the upper layer update the wrong data structures...
Surely the driver enter routine should update the counters that the driver was obligated to allocate, and it should return the state actually entered (for tracing), rather than the time spent there.
The generic cpuidle code should simply handle where the counters live in the sysfs namespace, not updating the counters.
This needs to be addressed before cpuidle_device.cpuidle_state[] can be made one/system.
cheers, Len Brown, Intel Open Source Technology Center
|  |