Re: [PATCH v4] power: introduce library for device-specific OPPs

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

 



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


[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