Paul E. McKenney had written, on 09/25/2010 07:56 PM, the following:
On Sat, Sep 25, 2010 at 10:55:20PM +0200, Rafael J. Wysocki wrote:
On Friday, September 24, 2010, Paul E. McKenney wrote:
On Fri, Sep 24, 2010 at 07:50:40AM -0500, Nishanth Menon wrote:
...
Looks like a good start!!! Some questions and suggestions about RCU
usage interspersed below.
...
+ * Locking: RCU reader.
+ */
+int opp_get_opp_count(struct device *dev)
+{
+ struct device_opp *dev_opp;
+ struct opp *temp_opp;
+ int count = 0;
+
+ dev_opp = find_device_opp(dev);
+ if (IS_ERR(dev_opp))
+ return PTR_ERR(dev_opp);
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(temp_opp, &dev_opp->opp_list, node) {
+ if (temp_opp->available)
+ count++;
+ }
+ rcu_read_unlock();
This one is OK as well. You are returning a count, so if all of the
counted structures are freed at this point, no problem. The count was
valid when it was accumulated, and the fact that it might now be obsolete
is (usually) not a problem.
However, it looks like it should run rcu_read_lock() before calling
find_device_opp(dev), shouldn't it?
Indeed it does appear that you are right -- good catch!!!
Thanx, Paul
dev_opp as discussed before is safe as it is never freed
(find_device_opp uses it's own rcu_read_lock, the rcu_read_lock in this
context is for the opp list. what am I missing?
ack on Paul's comments w.r.t risk on opp structures itself.. will look
to fix that in v5.
--
Regards,
Nishanth Menon
--
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
