[B.A.T.M.A.N.] [PATCH] batman-adv: Correct sparse warning about different lock contexts for basic block
sparse noticed that if_list_lock is taken in some situations with bottom halfes disabled in some SMP kernels and that we must always lock it with spin_lock_bh.
Signed-off-by: Sven Eckelmann sven.eckelmann@gmx.de --- batman-adv/hard-interface.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/batman-adv/hard-interface.c b/batman-adv/hard-interface.c index 37f0f8b..b5514e3 100644 --- a/batman-adv/hard-interface.c +++ b/batman-adv/hard-interface.c @@ -441,9 +441,9 @@ static struct batman_if *hardif_add_interface(struct net_device *net_dev)
check_known_mac_addr(batman_if->net_dev->dev_addr);
- spin_lock(&if_list_lock); + spin_lock_bh(&if_list_lock); list_add_tail_rcu(&batman_if->list, &if_list); - spin_unlock(&if_list_lock); + spin_unlock_bh(&if_list_lock);
/* extra reference for return */ kref_get(&batman_if->refcount); @@ -478,11 +478,11 @@ void hardif_remove_interfaces(void) struct batman_if *batman_if, *batman_if_tmp;
rtnl_lock(); - spin_lock(&if_list_lock); + spin_lock_bh(&if_list_lock); list_for_each_entry_safe(batman_if, batman_if_tmp, &if_list, list) { hardif_remove_interface(batman_if); } - spin_unlock(&if_list_lock); + spin_unlock_bh(&if_list_lock); rtnl_unlock(); }
@@ -508,9 +508,9 @@ static int hard_if_event(struct notifier_block *this, hardif_deactivate_interface(batman_if); break; case NETDEV_UNREGISTER: - spin_lock(&if_list_lock); + spin_lock_bh(&if_list_lock); hardif_remove_interface(batman_if); - spin_unlock(&if_list_lock); + spin_unlock_bh(&if_list_lock); break; case NETDEV_CHANGEMTU: if (batman_if->soft_iface)
On Wednesday 27 October 2010 10:02:25 Sven Eckelmann wrote:
sparse noticed that if_list_lock is taken in some situations with bottom halfes disabled in some SMP kernels and that we must always lock it with spin_lock_bh.
I would like to reject that patch by myself. Not that it does something bad, but more that it does not make any sense what sparse says. I've looked through the 2.6.32 smp code and couldn't locate the real problem. So if anyone has a opinion or can show were the actual problem is... please tell me.
My current test was done like that (on debian squeeze i386): ----------------------------------------------------------- $ cd $MY_v2.6.32 $ make mrproper $ make allnoconfig $ grep -v 'CONFIG_MODULES is not set' .config > .config.tmp; mv .config.tmp \ .config $ grep -v 'CONFIG_NET is not set' .config > .config.tmp; mv .config.tmp \ .config $ grep -v 'CONFIG_SMP is not set' .config > .config.tmp; mv .config.tmp \ .config $ grep -v 'CONFIG_MODULE_UNLOAD is not set' .config > .config.tmp; mv \ .config.tmp .config $ echo 'CONFIG_MODULES=y' >> .config $ echo 'CONFIG_NET=y' >> .config $ echo 'CONFIG_SMP=y' >> .config $ echo 'CONFIG_MODULE_UNLOAD=y' >> .config $ echo 'xy'|make menuconfig $ make prepare $ make modules
$ mkdir test $ cd test $ cat << EOF > Makefile obj-m += test.o test-y += main.o EOF $ cat << EOF > main.c #include <linux/module.h> #include <linux/spinlock.h>
static DEFINE_SPINLOCK(testlock);
static int __init test_init(void) { spin_lock(&testlock); spin_unlock(&testlock); return 0; }
static void __exit test_exit(void) { }
module_init(test_init); module_exit(test_exit); EOF
$ make CC=cgcc -C $MY_v2.6.32/linux-next M=$(pwd) modules make: Entering directory `$MY_v2.6.32' CC [M] $MY_v2.6.32/testmodule/main.o $MY_v2.6.32/test/main.c:6:19: warning: context imbalance in 'test_init' - wrong count at exit LD [M] $MY_v2.6.32/test/test.o Building modules, stage 2. MODPOST 1 modules LD [M] $MY_v2.6.32/test/test.ko make: Leaving directory `$MY_v2.6.32' -----------------------------------------------------------
The sparse version was v0.4.3.
I will post a question regarding that problem on the sparse mailinglist
Best regards, Sven
b.a.t.m.a.n@lists.open-mesh.org
-
Sven Eckelmann