On 13-01-16, 11:04, Viresh Kumar wrote:

On 12-01-16, 11:45, Stephen Boyd wrote:

quoted

quoted

+ * Locking: This function takes rcu_read_lock().

False.

Yeah, I realized it and fixed it right after sending it:

+ * Locking: This function internally uses mutex locks to keep the integrity of
+ * the internal data structures. Callers should ensure that this function is
+ * *NOT* called under RCU protection or in contexts where mutex cannot be
+ * locked.

quoted

quoted

+	/*
+	 * Hold our list modification lock here as regulator_set_voltage_time()
+	 * can possibly take another mutex, which isn't allowed within rcu
+	 * locks.
+	 */
+	mutex_lock(&dev_opp_list_lock);

So now we take the list modification mutex. Why can't we
rcu_read_lock(), find the min and max voltage, and then release
the read lock and ask regulator framework for the voltage time?

That was possible before this series came in..

quoted

From what I can tell we're just trying to make sure that the list
is stable when iterating through it to find the min/max voltage.

Hmm, we have pointer to regulator within the dev-opp struct. If we
drop the lock, the dev-opp struct can get freed and regulator will be
put just before that. So, we may be using an already freed resource.

How do you suggest we fix it ?

Ping..

-- 
viresh