Re: [PATCH] MIPS: We need to clear MMU contexts of all other processes when asid_cache(cpu) wraps to 0.

[Leonid Yegoshin <Leonid.Yegoshin@imgtec.com>]

On 07/11/2016 12:21 PM, James Hogan wrote:
> On Mon, Jul 11, 2016 at 11:19:30AM -0700, Leonid Yegoshin wrote:
>> On 07/11/2016 11:07 AM, James Hogan wrote:
>>>
>> 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).
I don't look into details of that but a safe way to do is - walk through 
all memory maps and lock it before change.

And to walk through memory maps we could use something like 
'find_lock_task_mm' but if there is a concern like you stated above then 
we could walk through all subthreads of task or just through all threads 
in system - anywhere, this even (ASID wrap) is pretty rare.

The advantage is in keeping all that stuff local and avoid patching 
other arch and common code.

>
> 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;
Not sure, but did you have chance to look into having another variable 
for cache flush control? It can be that some more states may be needed 
in future, so - just disjoin both, TLB and cache coontrol.

- Leonid.

>
> 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).
>>
>>