On Fri, Aug 19, 2011 at 7:06 AM, Turquette, Mike <mturquette@xxxxxx> wrote:
> On Thu, Aug 18, 2011 at 3:27 AM, MyungJoo Ham <myungjoo.ham@xxxxxxxxxxx> wrote:
>>
>> @@ -504,6 +513,11 @@ static int opp_set_availability(struct device *dev, unsigned long freq,
>> mutex_unlock(&dev_opp_list_lock);
>> synchronize_rcu();
>
> I'm not an RCU expert. Is the above synchronize_rcu call still needed
> with the addition of the srcu_notifier_call_chain below? The newly
> added src_notifier_call_chain will result in the following call graph:
>
> srcu_notifier_call_chain
> -> __srcu_notifier_call_chain
> -> srcu_read_lock
> -> __srcu_read_lock
> -> srcu_barrier
Though didn't read thorough kernel/rcutree_plugin.h, synchronize_rcu()
appears to do a lot more than sleepable RCU's srcu_barrier.
srcu_barrier() provides "barrier" for compiler at compile-time and
synchronize_rcu() provides "barrier" for processes at run-time. For
distributed systems (including multi-core systems) in general, I think
using srcu_barrier() instead of synchronize_rcu() is very dangerous.
>
[]
>
> Nitpick: I'm not a fan of conditional statements as func parameters.
>
[]
>
> Nitpick: unnecessary whitespace addition.
>
Ok, I'll revise them.
[]
>>
>> +enum opp_event {
>> + OPP_EVENT_ADD, OPP_EVENT_ENABLE, OPP_EVENT_DISABLE, OPP_EVENT_MAX
>
> I don't see OPP_EVENT_MAX used anywhere else in the code. What is it's purpose?
>
It was placed to mark "the end of the list" for opp notifier users.
However, it seems useless. I'll remove it.
I'll resubmit this part of the patchset.
Thanks.
MyungJoo
--
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
