"Rafael J. Wysocki" <rjw@xxxxxxx> writes:Fair enough. we can relook if the lock becomes a contended lock in the future. I do agree that simplifying the locking will benefit readability. Will post a v6 with a singular lock and updated documentation for the same.
On Tuesday, October 05, 2010, Nishanth Menon wrote:Rafael J. Wysocki had written, on 10/04/2010 05:36 PM, the following:On Friday, October 01, 2010, Nishanth Menon wrote:
[...]
Because of the simplicity, mostly. If there's only a relatively short periodI'm not really sure why so many mutexes are needed here. I don't think youI did consider using dev_opp_list_lock to lock everything *but* here is the contention:
need a separate mutex in every struct device_opp object. I'd just use
dev_opp_list_lock for everything.
dev_opp_list_lock locks modification for addition of domains device. This operation happens usually during init stage.
each domain device has multiple opps, new opps can be added, but the more often usage will probably be opp_enable and disable. domain are usually modifiable independent of each other - device_opp->lock provides device level lock allowing for each domain device opp list to be modified independent of each other. e.g. on thermal overage we may choose to lower mpu domain while a coprocessor driver in parallel might choose to disable co-processor domain in parallel.
Wondering why you'd like a single lock for all domains and restrict parallelization?
when the lock will be contended for, that still is not too bad and it's much
easier to get the synchronization right with just one lock for starters.
FWIW, I agree with Rafael
These are not going be highly contended locks, and the lock durations
are very short, so simplifying the locking is a big win for readability.
Kevin