Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:
> On Tue, May 26, 2015 at 6:18 PM, Luck, Tony <tony.luck@xxxxxxxxx> wrote:
>>
>> cpumask_test_cpu() doesn't take volatile, unlike the obsoleted
>> cpu_isset. The only place ia64 really cares is the spin waiting for a
>> bit; udelay() is probably a barrier but insert rmb() to be sure.
>
> Hmm. udelay() definitely should be a barrier, so this patch shouldn't
> matter. But even if it weren't, rmb() would be the wrong barrier to
> use here.
>
> rmb() is a "CPU or IO read barrier" to order reads wrt other cores or
> DMA, and needs to be paired with a wmb() on other cores (and for DMA
> it is assumed that there is sufficiently write ordering for the DMA
> itself).
>
> If it's about just other CPU's, it would be "smp_rmb()", which is
> often much cheaper (due to no IO serialization, particularly on
> POWER).
>
> But in this case it doesn't seem to be about other agents at all
> (CPU's or DMA), you literally are just looking for a compiler barrier
> to make sure the read doesn't get moved out of the loop. And the
> function for that is just "barrier()".
>
> So I'm skipping this patch, because I dont' think it changes anything
> real, and from a "let's be careful" standpoint it's actually doing the
> wrong thing.
Good point: the fact that other cpus are the ones setting the bit is a
red herring here.
Thanks,
Rusty.
From: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
Subject: ia64: make cpu_callin_map non-volatile.
cpumask_test_cpu() doesn't take volatile, unlike the obsoleted
cpu_isset. The only place ia64 really cares is the spin waiting for a
bit; udelay() is probably a barrier but insert barrier() to be sure.
Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c
index 15051e9c2c6f..629975b56608 100644
--- a/arch/ia64/kernel/smpboot.c
+++ b/arch/ia64/kernel/smpboot.c
@@ -127,7 +127,7 @@ int smp_num_siblings = 1;
volatile int ia64_cpu_to_sapicid[NR_CPUS];
EXPORT_SYMBOL(ia64_cpu_to_sapicid);
-static volatile cpumask_t cpu_callin_map;
+static cpumask_t cpu_callin_map;
struct smp_boot_data smp_boot_data __initdata;
@@ -477,6 +477,7 @@ do_boot_cpu (int sapicid, int cpu, struct task_struct *idle)
for (timeout = 0; timeout < 100000; timeout++) {
if (cpumask_test_cpu(cpu, &cpu_callin_map))
break; /* It has booted */
+ barrier(); /* Make sure we re-read cpu_callin_map */
udelay(100);
}
Dprintk("\n");
--
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
|