[B.A.T.M.A.N.] [PATCH] batman-adv: Correct sparse warning about different lock contexts for basic block

[B.A.T.M.A.N.] [PATCH] batman-adv: Correct sparse warning about different lock contexts for basic block

[Sven Eckelmann]

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)
-- 
1.7.2.3

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Correct sparse warning about different lock contexts for basic block

[Sven Eckelmann]

[-- Attachment #1: Type: Text/Plain, Size: 2305 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]