linux-kernel - Re: [Kgdb-bugreport] [PATCH 4/5] kgdb: Use atomic operators whichuse barriers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4BB649E3.5060606@windriver.com>
Date:	Fri, 02 Apr 2010 14:47:47 -0500
From:	Jason Wessel <jason.wessel@...driver.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
CC:	kgdb-bugreport@...ts.sourceforge.net,
	Will Deacon <will.deacon@....com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Russell King - ARM Linux <linux@....linux.org.uk>,
	linux-arm@...r.kernel.org
Subject: Re: [Kgdb-bugreport] [PATCH 4/5] kgdb: Use atomic operators whichuse
 barriers


Here is the revised patch, which is going into the kgdb-fixes branch.

Thanks,
Jason.

--
From: Jason Wessel <jason.wessel@...driver.com>
Subject: [PATCH] kgdb: Use atomic operators which use barriers

The cpu_relax() does not mandate that there is an smp memory barrier.
As a result on the arm smp architecture the kernel debugger can hang
on entry from time to time, as shown by the kgdb regression tests.

The solution is simply to use the atomic operators which include a
proper smp memory barrier, instead of using atomic_set().

Tested-by: Will Deacon <will.deacon@....com>
Signed-off-by: Jason Wessel <jason.wessel@...driver.com>
---
 kernel/kgdb.c |   15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

--- a/kernel/kgdb.c
+++ b/kernel/kgdb.c
@@ -1379,8 +1379,7 @@ acquirelock:
 	 * Make sure the above info reaches the primary CPU before
 	 * our cpu_in_kgdb[] flag setting does:
 	 */
-	smp_wmb();
-	atomic_set(&cpu_in_kgdb[cpu], 1);
+	atomic_inc(&cpu_in_kgdb[cpu]);
 
 	/*
 	 * CPU will loop if it is a slave or request to become a kgdb
@@ -1400,7 +1399,7 @@ return_normal:
 			 */
 			if (arch_kgdb_ops.correct_hw_break)
 				arch_kgdb_ops.correct_hw_break();
-			atomic_set(&cpu_in_kgdb[cpu], 0);
+			atomic_dec(&cpu_in_kgdb[cpu]);
 			touch_softlockup_watchdog_sync();
 			clocksource_touch_watchdog();
 			local_irq_restore(flags);
@@ -1449,7 +1448,7 @@ return_normal:
 	 */
 	if (!kgdb_single_step) {
 		for (i = 0; i < NR_CPUS; i++)
-			atomic_set(&passive_cpu_wait[i], 1);
+			atomic_inc(&passive_cpu_wait[i]);
 	}
 
 #ifdef CONFIG_SMP
@@ -1483,11 +1482,11 @@ return_normal:
 	if (kgdb_io_ops->post_exception)
 		kgdb_io_ops->post_exception();
 
-	atomic_set(&cpu_in_kgdb[ks->cpu], 0);
+	atomic_dec(&cpu_in_kgdb[ks->cpu]);
 
 	if (!kgdb_single_step) {
 		for (i = NR_CPUS-1; i >= 0; i--)
-			atomic_set(&passive_cpu_wait[i], 0);
+			atomic_dec(&passive_cpu_wait[i]);
 		/*
 		 * Wait till all the CPUs have quit
 		 * from the debugger.
@@ -1736,11 +1735,11 @@ EXPORT_SYMBOL_GPL(kgdb_unregister_io_mod
  */
 void kgdb_breakpoint(void)
 {
-	atomic_set(&kgdb_setting_breakpoint, 1);
+	atomic_inc(&kgdb_setting_breakpoint);
 	wmb(); /* Sync point before breakpoint */
 	arch_kgdb_breakpoint();
 	wmb(); /* Sync point after breakpoint */
-	atomic_set(&kgdb_setting_breakpoint, 0);
+	atomic_dec(&kgdb_setting_breakpoint);
 }
 EXPORT_SYMBOL_GPL(kgdb_breakpoint);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/