Linus Lüssing wrote: > Looks quite good and seems to make sense to avoid the > double-locking. Tested it here and together with patch 1/3 fixes > the initial issue, too. > > Just one minor thing: > ---- > [58518.225154] batman_adv: bat0: The MTU of interface eth1 is too > small (1500) to handle the transport of batman-adv packets. > Packets going over this interface will be fragmented on layer2 > which could impact the performance. Setting the MTU to 1527 would > solve the problem. > [58518.278111] RTNL: assertion failed at > /home/mesh-node/incoming/batman-adv-git-builddir/hard-interface.c > (136) > [58518.293167] Pid: 6366, comm: bash Not tainted 2.6.38+ #4 > [58518.299142] Call Trace: > [58518.300953] [<f8cb91f3>] primary_if_select+0x2f/0x9f > [batman_adv] > [58518.447844] [<f8cb8e9c>] ? rcu_read_unlock+0x1c/0x1e > [batman_adv] > [58518.452572] [<f8cb9146>] ? primary_if_get_selected+0x6a/0x71 > [batman_adv] > [58518.455759] [<f8cb92c6>] hardif_activate_interface+0x63/0x91 > [batman_adv] > [58518.459168] [<f8cb98e4>] hardif_enable_interface+0x1cc/0x21e > [batman_adv] > [58518.462481] [<f8cb7bca>] store_mesh_iface+0xed/0x113 > [batman_adv] > [58518.465564] [<f8cb7add>] ? store_aggregated_ogms+0x28/0x28 > [batman_adv] > [58518.468714] [<c11e1121>] kobj_attr_store+0x1a/0x22 > [58518.472277] [<c11481dc>] sysfs_write_file+0xbc/0xe7 > [58518.613128] [<c1148120>] ? sysfs_open_file+0x205/0x205 > [58518.616400] [<c10ff7a7>] vfs_write+0x84/0xde > [58518.660437] [<c10ff89a>] sys_write+0x3b/0x60 > [58518.667248] [<c151fd9d>] syscall_call+0x7/0xb > [58518.672117] batman_adv: bat0: Interface activated: eth1 > ---- > > So maybe it makes sense to add the assertion in > primary_if_select() in patch 3/3 instead?
No, the problem is that I changed the patch after I noticed that it wasn't
made for the current master - and only tested after all patches were applied.
I would like to add rtnl_lock/rtnl_unlock around primary_if_get_selected()
instead.
> Hmm, and I'm wondering if it might make sense to also document it
> with a little comment above the struct list_head hardif_list;,
> like:
> /* List additions and deletions have to be rtnl_lock()'ed,
> * list traversals just rcu-locked */
> (or is that overdoing it?)
Ok, will steal this comment from your mail - ok, change it slightly to present
it as my own work.
Kind regards,
Sven
signature.asc
Description: This is a digitally signed message part.
