* Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote: > On Mon, Jan 11, 2016 at 10:25 AM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote: > > On Mon, Jan 11, 2016 at 03:42:40AM -0800, tip-bot for Andy Lutomirski wrote: > >> --- a/arch/x86/include/asm/mmu_context.h > >> +++ b/arch/x86/include/asm/mmu_context.h > >> @@ -116,8 +116,34 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next, > >> #endif > >> cpumask_set_cpu(cpu, mm_cpumask(next)); > >> > >> - /* Re-load page tables */ > >> + /* > >> + * Re-load page tables. > >> + * > >> + * This logic has an ordering constraint: > >> + * > >> + * CPU 0: Write to a PTE for 'next' > >> + * CPU 0: load bit 1 in mm_cpumask. if nonzero, send IPI. > >> + * CPU 1: set bit 1 in next's mm_cpumask > >> + * CPU 1: load from the PTE that CPU 0 writes (implicit) > >> + * > >> + * We need to prevent an outcome in which CPU 1 observes > >> + * the new PTE value and CPU 0 observes bit 1 clear in > >> + * mm_cpumask. (If that occurs, then the IPI will never > >> + * be sent, and CPU 0's TLB will contain a stale entry.) > >> + * > >> + * The bad outcome can occur if either CPU's load is > >> + * reordered before that CPU's store, so both CPUs much > > > > s/much/must/ ? > > Indeed. Is this worth a follow-up patch? Absolutely! Any typos in code noticed by humans are worth fixing, especially when it's comments around tricky code. Could be done together with improving this part of the comments: > > + > > /* > > * We were in lazy tlb mode and leave_mm disabled > > * tlb flush IPI delivery. We must reload CR3 > > * to make sure to use no freed page tables. > > + * > > + * As above, this is a barrier that forces > > + * TLB repopulation to be ordered after the > > + * store to mm_cpumask. > > somewhat confused by this comment, cpumask_set_cpu() is a LOCK BTS, that is > already fully ordered. ... as pretty much any barriers related comment that confuses Peter needs to be improved, by definition. ;-) Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-tip-commits" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
|