Re: [PATCH 06/17][V2] ARM: OMAP4: cpuidle - use the omap4_idle_data variable directly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/10/2012 12:56 AM, Kevin Hilman wrote:
Daniel Lezcano<daniel.lezcano@xxxxxxxxxx>  writes:

We are storing the 'omap4_idle_data' in the private data field
of the cpuidle device. As we are using this variable only in this file,
that does not really make sense. Let's use the global variable directly
instead dereferencing pointers in an idle critical loop.

Did you notice a performance impact before this change?

No, I didn't and I don't think I will be able to measure it. But by experience, when dereferencing a pointer that could leads to a cache miss and impact the performances. That may not be the case here, so I won't argue with that as I won't able to prove it :)

Also, that simplfies the code.

possibly, but at the expense of clean abstractions which IMO helps readability.

Unless there is a real performance hit here (which I doubt), I'd prefer
to leave this as is.

There are two reasons of this change. We are storing 'state_count' times a pointer in a private structure for state_usage but the information is already available in the file and easily accessible. Also, this is set in the fill_cstate function I am removing to let all the initialization to be done at compile time.

Furthermore, I don't get why we have a 'driver data' area in a structure which is dedicated for the states statistics, IMHO that does not help understanding. My mid-term cleanup is to kill the 'driver_data' field in the cpuidle core because I don't think it is at the right place.

An idea I had for consolidate all the cpuidle driver was to use the containerof macro to define the architecture specific structure and declare inside this structure the cpuidle driver and the devices. It is similar on how are implemented the 'routes' for the network stack or the cgroup subsystems, there is a core engine handling generic structure which a contained by the specific structure related to the engine's user. That helps a lot for readability.

Well this is an idea but before I have to unify the cpuidle drivers code to make it clear what is doable or not.



--
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux