On Mon, Jul 11, 2016 at 11:19:30AM -0700, Leonid Yegoshin wrote: > On 07/11/2016 11:07 AM, James Hogan wrote: > > Hi Leonid, > > > > On Mon, Jul 11, 2016 at 11:02:00AM -0700, Leonid Yegoshin wrote: > >> On 07/10/2016 06:04 AM, yhb@xxxxxxxxxxxxx wrote: > >>> Subject: [PATCH] MIPS: We need to clear MMU contexts of all other processes > >>> when asid_cache(cpu) wraps to 0. > >>> > >>> Suppose that asid_cache(cpu) wraps to 0 every n days. > >>> case 1: > >>> (1)Process 1 got ASID 0x101. > >>> (2)Process 1 slept for n days. > >>> (3)asid_cache(cpu) wrapped to 0x101, and process 2 got ASID 0x101. > >>> (4)Process 1 is woken,and ASID of process 1 is same as ASID of process 2. > >>> > >>> case 2: > >>> (1)Process 1 got ASID 0x101 on CPU 1. > >>> (2)Process 1 migrated to CPU 2. > >>> (3)Process 1 migrated to CPU 1 after n days. > >>> (4)asid_cache on CPU 1 wrapped to 0x101, and process 2 got ASID 0x101. > >>> (5)Process 1 is scheduled, and ASID of process 1 is same as ASID of process 2. > >>> > >>> So we need to clear MMU contexts of all other processes when asid_cache(cpu) wraps to 0. > >>> > >>> Signed-off-by: yhb <yhb@xxxxxxxxxxxxx> > >>> > >> I think a more clear description should be given here - there is no > >> indication that wrap happens over 32bit integer. > >> > >> And taking into account "n days" frequency - can we just kill all local > >> ASIDs in all processes (additionally to local_flush_tlb_all) and enforce > >> reassignment if wrap happens? It should be a very rare event, you are > >> first to hit this. > >> > >> It seems to be some localized stuff in get_new_mmu_context() instead of > >> widespread patching. > > That is what this patch does, but to do so it appears you need to lock > > the other tasks one by one, and that must be doable from a context > > switch, i.e. hardirq context, and that requires the task lock to be of > > the _irqsave variant, hence the widespread changes and the relatively > > tiny MIPS change hidden in the middle. > > > Not exactly. The change must be done only for local CPU which executes > at the moment get_new_mmu_context(). Just prevent preemption here and > change of cpu_context(THIS_CPU,...) can be done safely - other CPUs > don't do anything with this variable besides killing it (writing 0 to it). Right, but I was thinking more along the lines of whether you can ensure the other tasks / mm continues to exist. I think this is partly achieved by the read_lock'ing of tasklist_lock, but also possibly by the find_lock_task_mm() call, which has a comment saying: /* * The process p may have detached its own ->mm while exiting or through * use_mm(), but one or more of its subthreads may still have a valid * pointer. Return p, or any of its subthreads with a valid ->mm, with * task_lock() held. */ (but of course I could be mistaken and something else guarantees it won't go away). Note also that I have a patch I'm about to submit which changes some of those assignments of 0 to assign 1 instead (so as not to confuse the cache management code into thinking the CPU has never run the code when it has, while still triggering ASID regeneration). That applies here too, so it should perhaps be doing something like this instead: if (t->mm != mm && cpu_context(cpu, t->mm)) cpu_context(cpu, t->mm) = 1; Cheers James > > You can look into flush_tlb_mm() for example how it is cleared for > single memory map. > > We have a macro to safely walk all processes, right? (don't remember > it's name). > >
Attachment:
signature.asc
Description: Digital signature
