Skip to content

Commit

Permalink
i386: Fix a couple busy loops in mach_wakecpu.h:wait_for_init_deassert()
Browse files Browse the repository at this point in the history
Use cpu_relax() in the busy loops, as atomic_read() doesn't automatically
imply volatility for i386 and x86_64. x86_64 doesn't have this issue because
it open-codes the while loop in smpboot.c:smp_callin() itself that already
uses cpu_relax().

For i386, however, smpboot.c:smp_callin() calls wait_for_init_deassert()
which is buggy for mach-default and mach-es7000 cases.

[ I test-built a kernel -- smp_callin() itself got inlined in its only
  callsite, smpboot.c:start_secondary() -- and the relevant piece of
  code disassembles to the following:

0xc1019704 <start_secondary+12>:        mov    0xc144c4c8,%eax
0xc1019709 <start_secondary+17>:        test   %eax,%eax
0xc101970b <start_secondary+19>:        je     0xc1019709 <start_secondary+17>

  init_deasserted (at 0xc144c4c8) gets fetched into %eax only once and
  then we loop over the test of the stale value in the register only,
  so these look like real bugs to me. With the fix below, this becomes:

0xc1019706 <start_secondary+14>:        pause
0xc1019708 <start_secondary+16>:        cmpl   $0x0,0xc144c4c8
0xc101970f <start_secondary+23>:        je     0xc1019706 <start_secondary+14>

  which looks nice and healthy. ]

Thanks to Heiko Carstens for noticing this.

Signed-off-by: Satyam Sharma <satyam@infradead.org>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
  • Loading branch information
Satyam Sharma authored and Linus Torvalds committed Aug 18, 2007
1 parent 06bfb7e commit 62be900
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 2 deletions.
3 changes: 2 additions & 1 deletion include/asm-i386/mach-default/mach_wakecpu.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@

static inline void wait_for_init_deassert(atomic_t *deassert)
{
while (!atomic_read(deassert));
while (!atomic_read(deassert))
cpu_relax();
return;
}

Expand Down
3 changes: 2 additions & 1 deletion include/asm-i386/mach-es7000/mach_wakecpu.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ wakeup_secondary_cpu(int phys_apicid, unsigned long start_eip)
static inline void wait_for_init_deassert(atomic_t *deassert)
{
#ifdef WAKE_SECONDARY_VIA_INIT
while (!atomic_read(deassert));
while (!atomic_read(deassert))
cpu_relax();
#endif
return;
}
Expand Down

0 comments on commit 62be900

Please sign in to comment.