Messages in this thread | | | Date | Thu, 19 Jul 2012 11:59:59 +0530 | From | "Srivatsa S. Bhat" <> | Subject | Re: [PATCH v2] x86, mm: only wait for flushes from online cpus |
| |
On 07/19/2012 03:43 AM, Mandeep Singh Baines wrote: > Srivatsa S. Bhat (srivatsa.bhat@linux.vnet.ibm.com) wrote: >> On 06/23/2012 03:36 AM, Mandeep Singh Baines wrote: >>> A cpu in the mm_cpumask could go offline before we send the invalidate >>> IPI causing us to wait forever. Avoid this by only waiting for online >>> cpus. >>> [...] >> This function is always called with preempt_disabled() right? >> In that case, _while_ this function is running, a CPU cannot go offline >> because of stop_machine(). (I understand that it might go offline in between >> calculating that cpumask and calling preempt_disable() - which is the race >> you are trying to handle). >> > > Ah. Good point. A cpu cannot be remove from the cpu_online_mask while > preemption is disabled because stop_machine() can't run until > preemption is enabled. > > ./kernel/cpu.c: err = __stop_machine(take_cpu_down, &tcd_param, cpumask_of(cpu)); > >> So, why not take the offline cpus out of the way even before sending that IPI? >> That way, we need not modify the while loop below. >> > > Acked-off-by: Mandeep Singh Baines <msb@chromium.org> > > Do you mind re-sending you're patch with a proper sign-off. >
Sure, will do. I'll post it in a separate thread.
Thanks!
Regards, Srivatsa S. Bhat
> >>> - while (!cpumask_empty(to_cpumask(f->flush_cpumask))) >>> + /* Only wait for online cpus */ >>> + do { >>> + cpumask_and(to_cpumask(tmp_cpumask), >>> + to_cpumask(f->flush_cpumask), >>> + cpu_online_mask); >>> cpu_relax(); >>> + } while (!cpumask_empty(to_cpumask(tmp_cpumask))); >>> } >>> >>> f->flush_mm = NULL; >>> >> >> That is, how about something like this: >> >> diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c >> index 5e57e11..9d387a9 100644 >> --- a/arch/x86/mm/tlb.c >> +++ b/arch/x86/mm/tlb.c >> @@ -186,7 +186,11 @@ static void flush_tlb_others_ipi(const struct cpumask *cpumask, >> >> f->flush_mm = mm; >> f->flush_va = va; >> - if (cpumask_andnot(to_cpumask(f->flush_cpumask), cpumask, cpumask_of(smp_processor_id()))) { >> + >> + cpumask_and(to_cpumask(f->flush_cpumask), cpumask, cpu_online_mask); >> + cpumask_clear_cpu(smp_processor_id(), to_cpumask(f->flush_cpumask)); >> + >> + if (!cpumask_empty(to_cpumask(f->flush_cpumask))) { >> /* >> * We have to send the IPI only to >> * CPUs affected. >> >> >> Regards, >> Srivatsa S. Bhat >> IBM Linux Technology Center >> >
| |