Skip to contentSkip to conversation

net: iptables: rework the fix of xt_info locking

PATCH In review Luis Claudio R. Goncalves 2 participants 2 messages Mar 2, 2011

Subsystems (2)
Networking netfilter
lore
Share & tools
lore

Luis Claudio R. Goncalves

author
#0 lore raw

PATCH 1/1 net: iptables: rework the fix of xt_info locking

Thomas,

Maybe PREEMPT_NONE is not really what we want to check against, maybe it is RT_MUTEXES... anyway, this patch works fine as is in kernels with PREEMPT_NONE, PREEMPT_DESKTOP, PREEMPT_VOLUNTARY and PREEMPT_RT set.

---

net: iptables: rework the fix of xt_info locking

Commit 5bbbedc from tip/rt/2.6.33 fixed an annoying locking issue on PREEMPT_RT enabled kernels. I have observed a similar issue when testing a kernel with PREEMPT_DESKTOP enabled. My test kernels would boot fine and as soon as I tried to ssh into the test box I would see messages like this one:

	BUG: %Ps exited with wrong preemption count!
	=> enter: 00000101, exit: 00000100.
	BUG: %Ps exited with wrong preemption count!
	=> enter: 00000101, exit: 00000100.
	BUG: %Ps exited with wrong preemption count!
	=> enter: 00000102, exit: 00000101.

And soon after a flood of backtraces such as:

	WARNING: at kernel/sched.c:5571 sub_preempt_count+0x68/0x87()

This patch extends the changes made to the PREEMPT_RT case to all of the cases where PREEMPT_NONE is not enabled.

Signed-off-by Luis Claudio R. Gonçalves <[email protected]>
include/linux/netfilter/x_tables.h +5 -5
index f85b72b..0b61a0f 100644
@@ -439,7 +439,7 @@ extern void xt_free_table_info(struct xt_table_info *info);
  *  necessary for reading the counters.
  */
 struct xt_info_lock {
-#ifndef CONFIG_PREEMPT_RT
+#ifdef CONFIG_PREEMPT_NONE
 	spinlock_t lock;
 #else
 	rwlock_t lock;

@@ -471,7 +471,7 @@ static inline int xt_info_rdlock_bh(void)
 	cpu = smp_processor_id();
 	lock = &per_cpu(xt_info_locks, cpu);
 
-#ifndef CONFIG_PREEMPT_RT
+#ifdef CONFIG_PREEMPT_NONE
 	if (likely(!--lock->readers))
 		spin_unlock(&lock->lock);
 #else

@@ -485,7 +485,7 @@ static inline void xt_info_rdunlock_bh(int cpu)
 {
 	struct xt_info_lock *lock = &per_cpu(xt_info_locks, cpu);
 
-#ifndef CONFIG_PREEMPT_RT
+#ifdef CONFIG_PREEMPT_NONE
 	if (likely(!--lock->readers)) {
 		preempt_enable_rt();
 		spin_unlock(&lock->lock);

@@ -504,7 +504,7 @@ static inline void xt_info_rdunlock_bh(int cpu)
  */
 static inline void xt_info_wrlock(unsigned int cpu)
 {
-#ifndef CONFIG_PREEMPT_RT
+#ifdef CONFIG_PREEMPT_NONE
 	spin_lock(&per_cpu(xt_info_locks, cpu).lock);
 #else
 	write_lock(&per_cpu(xt_info_locks, cpu).lock);

@@ -513,7 +513,7 @@ static inline void xt_info_wrlock(unsigned int cpu)
 
 static inline void xt_info_wrunlock(unsigned int cpu)
 {
-#ifndef CONFIG_PREEMPT_RT
+#ifdef CONFIG_PREEMPT_NONE
 	spin_unlock(&per_cpu(xt_info_locks, cpu).lock);
 #else
 	write_unlock(&per_cpu(xt_info_locks, cpu).lock);
net/netfilter/x_tables.c +1 -1
index 3eae9d9..546e6a1 100644
@@ -1184,7 +1184,7 @@ static int __init xt_init(void)
 	for_each_possible_cpu(i) {
 		struct xt_info_lock *lock = &per_cpu(xt_info_locks, i);
 
-#ifndef CONFIG_PREEMPT_RT
+#ifdef CONFIG_PREEMPT_NONE
 		spin_lock_init(&lock->lock);
 #else
 		rwlock_init(&lock->lock);
Signature · 7 lines
[ Luis Claudio R. Goncalves                    Bass - Gospel - RT ]
[ Fingerprint: 4FDD B8C4 3C59 34BD 8BE9  2696 7203 D980 A448 C8F8 ]

--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#1 lore raw
Quoted 15 lines · Luis Claudio R. Goncalves

Thomas,

Maybe PREEMPT_NONE is not really what we want to check against, maybe it is RT_MUTEXES... anyway, this patch works fine as is in kernels with PREEMPT_NONE, PREEMPT_DESKTOP, PREEMPT_VOLUNTARY and PREEMPT_RT set.

---

net: iptables: rework the fix of xt_info locking

Commit 5bbbedc from tip/rt/2.6.33 fixed an annoying locking issue on PREEMPT_RT enabled kernels. I have observed a similar issue when testing a kernel with PREEMPT_DESKTOP enabled. My test kernels would boot fine and as soon as I tried to ssh into the test box I would see messages like this one:

Quoted 6 lines
	BUG: %Ps exited with wrong preemption count!
	=> enter: 00000101, exit: 00000100.
	BUG: %Ps exited with wrong preemption count!
	=> enter: 00000101, exit: 00000100.
	BUG: %Ps exited with wrong preemption count!
	=> enter: 00000102, exit: 00000101.

This error message would be a bit more usful after this patch:

kernel/softirq.c +2 -2
index c35f72b..b3b20b0 100644
@@ -335,8 +335,8 @@ debug_check_preempt_count_stop(__u32 *preempt_count, struct softirq_action *h)
 	if (*preempt_count == preempt_count())
 		return;
 
-	print_symbol("BUG: %Ps exited with wrong preemption count!\n",
-		     (unsigned long)h->action);
+	printk("BUG: %ps exited with wrong preemption count!\n",
+			h->action);
 	printk("=> enter: %08x, exit: %08x.\n", *preempt_count, preempt_count());
 	preempt_count() = *preempt_count;
 #endif

Note this is completely untested and applies on top of v2.6.33.7.2-rt30.

Thomas: should I send a proper patch or isn't it worth the effort?

Best regards
Uwe

Signature · 6 lines
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html