Messages in this thread | | | Date | Wed, 13 Jan 2010 09:46:59 -0500 | From | Mathieu Desnoyers <> | Subject | Re: [RFC PATCH] introduce sys_membarrier(): process-wide memory barrier (v5) |
| |
* Heiko Carstens (heiko.carstens@de.ibm.com) wrote: > On Tue, Jan 12, 2010 at 08:37:57PM -0500, Mathieu Desnoyers wrote: > > +static void membarrier_retry(void) > > +{ > > + struct mm_struct *mm; > > + int cpu; > > + > > + for_each_cpu(cpu, mm_cpumask(current->mm)) { > > + spin_lock_irq(&cpu_rq(cpu)->lock); > > + mm = cpu_curr(cpu)->mm; > > + spin_unlock_irq(&cpu_rq(cpu)->lock); > > + if (current->mm == mm) > > + smp_call_function_single(cpu, membarrier_ipi, NULL, 1); > > + } > > You would need to disable cpu unplug operations while doing this > or you might end up sending IPIs to offline cpus.
smp_call_function_single() checks for cpu_online(cpu), and returns -ENXIO if the cpu is not online. So I think disabling cpu hotplug would be redundant with this test.
smp_call_function_many uses cpumask_next_and(cpu, mask, cpu_online_mask) to alter the mask, so no cpu hotplug disabling needed there neither.
These checks are protected by preemption disabling.
> > > + cpumask_copy(tmpmask, mm_cpumask(current->mm)); > > + preempt_disable(); > > + cpumask_clear_cpu(smp_processor_id(), tmpmask); > > + for_each_cpu(cpu, tmpmask) { > > + spin_lock_irq(&cpu_rq(cpu)->lock); > > + mm = cpu_curr(cpu)->mm; > > This might access the rq of an offline cpu. > But maybe it's intended since offline cpus "run" idle?
In a rare race with hotunplug vs lazy TLB shootdown, yes. Although even then, as you point out, ->mm will be NULL, so we won't even consider the CPU for IPI. In any case, I think adding a cpumask online "and" would be an added performance overhead for the common case compared to the performance gain in the rare cpu hotunplug race window.
Thanks,
Mathieu
-- Mathieu Desnoyers OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
| |