Re: [PATCH] rust: i2c: avoid locking when calling I2cAdapter::inc_ref

Igor Korotin <[email protected]>
Newsgroups org.kernel.vger.rust-for-linux,dev.linux.lists.linux-kernel-mentees,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
Hello Nicolás

Sorry for the delay.

Yes, worth fixing regardless of real users -- inc_ref taking a lock it
doesn't need is a real bug, not speculative work.

Instead of adding a separate Rust-only wrapper, please consider splitting
i2c_get_adapter itself: pull the increment into a helper, export it, and
call it directly from inc_ref -- no lock, no lookup:

+bool __i2c_adapter_get(struct i2c_adapter *adapter)
+{
+	if (try_module_get(adapter->owner)) {
+		get_device(&adapter->dev);
+		return true;
+	}
+	return false;
+}
+EXPORT_SYMBOL(__i2c_adapter_get);
+
 struct i2c_adapter *i2c_get_adapter(int nr)
 {
 	struct i2c_adapter *adapter;

 	mutex_lock(&core_lock);
 	adapter = idr_find(&i2c_adapter_idr, nr);
-	if (!adapter)
-		goto exit;
-
-	if (try_module_get(adapter->owner))
-		get_device(&adapter->dev);
-	else
+	if (adapter && !__i2c_adapter_get(adapter))
 		adapter = NULL;
-
- exit:
 	mutex_unlock(&core_lock);
 	return adapter;
 }
 EXPORT_SYMBOL(i2c_get_adapter);

Heads up for v2: Trevor Chan has a patch changing AlwaysRefCounted::inc_ref
to an associated function (fn inc_ref(obj: &Self)). Not merged yet --
rebase onto it if it lands first.

Cheers
Igor
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.