On Thu, Jan 28, 2016 at 02:40:07AM +0100, Andrew Lunn wrote:
On Thu, Jan 28, 2016 at 09:28:07AM +0800, Antonio Quartulli wrote:
On Wed, Jan 20, 2016 at 06:48:30PM +0100, Andrew Lunn wrote:
[...]
+static DEFINE_MUTEX(batadv_debugfs_ns_mutex);
[...]
+static void batadv_debugfs_ns_put(struct net *net) +{
- struct batadv_debugfs_ns_entry *ns_entry;
- mutex_lock(&batadv_debugfs_ns_mutex);
- list_for_each_entry(ns_entry, &batadv_debugfs_ns, link) {
if (ns_entry->net == net) {kref_put(&ns_entry->refcount, batadv_ns_entry_release);break;}- }
- mutex_unlock(&batadv_debugfs_ns_mutex);
+}
Andrew, is there any particular reason why in this case we use a mutex instead of a spinlock + RCU ? I imagine it is something related to debugfs..or just to make the code simpler as we don't really require a full-blown RCU strategy ?
I just wanted it to be KISS. We are on a very slow path, only called when interfaces are added and removed. Keeping it KISS makes it easier to review, make sure i have an unlock for every lock, etc. It is also what is the kref documentation suggests.
ok, thanks for explaining, Andrew! I agree with that, but I wanted to be sure I was understanding the ratio behind :)
how about:
debugfs_ns_dir = batadv_debugfs;
- if (net != &init_net) {
debugfs_ns_dir = batadv_debugfs_ns_get(net);if (!debugfs_ns_dir)goto out;}
hard_iface->debug_dir = debugfs_create_dir(name, debugfs_ns_dir);
isn't it nicer? :)
O.K, i will rearrange.
The last chunk reported in my reply could be rearranged the same way.
Thanks a lot! Other than these points the patch seems sound to me.
Cheers,