[RFC PATCH 2/2] ARMv7: Invalidate the TLB before freeing page tables

On 21 February 2011 10:30, Russell King - ARM Linux
[off-list ref] wrote:

On Mon, Feb 21, 2011 at 09:39:32AM +0000, Catalin Marinas wrote:

quoted

On 20 February 2011 12:12, Russell King - ARM Linux
[off-list ref] wrote:

quoted

On Tue, Feb 15, 2011 at 02:42:06PM +0000, Catalin Marinas wrote:

quoted

On Tue, 2011-02-15 at 12:14 +0000, Russell King - ARM Linux wrote:

quoted

On Tue, Feb 15, 2011 at 11:32:42AM +0000, Russell King - ARM Linux wrote:

quoted

The point of TLB shootdown is that we unmap the entries from the page
tables, then issue the TLB flushes, and then free the pages and page
tables after that. ?All that Peter's patch tries to do is to get ARM to
use the generic stuff.

As Peter's patch preserves the current behaviour, that's not sufficient.
So, let's do this our own way and delay pages and page table frees on
ARMv6 and v7. ?Untested.

ARMv7 should be enough, I'm not aware of any pre-v7 with this behaviour.

ARM11MPCore. ?Any SMP system can access a page which was free'd by the
tlb code but hasn't been flushed from the hardware TLBs. ?So maybe we
want it to be "defined(CONFIG_SMP) || defined(CONFIG_CPU_32v7)" ?

In practice, since the hardware TLB does not store higher level
entries on existing v6 cores, there is no cached value pointing to the
freed pte page.

It's not about cached values of PTE pointers.

My point is about cached values in the 1st level of page tables
pointing to the second level and the freeing of the 2nd level of page
tables.

I think we talk about two different cases (but could be fixable with
the same patch). Your scenario is valid as well, just different.

quoted

In theory, we first clear the pmd entry but another
CPU could be doing a PTW at the same time and had already read the pmd
before being cleared. But the timing constraints are difficult to
reproduce in practice.

I don't think you properly understand the problem.

CPU#0 is unmapping page tables, eg due to munmap(), mremap(), etc.
CPU#1 is running a thread, and has TLB entries for the region being unmapped.

? ? ? ?CPU#0 ? ? ? ? ? ? ? ? ? ? ? ? ? CPU#1
? ? ? ?clear page table entry
? ? ? ?frees page
? ? ? ?loop continues
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?accesses page
? ? ? ?...
? ? ? ?sometime in the future
? ? ? ?invalidates TLB

For this scenario we only need to implement tlb_remove_page() to
invalidate the TLB before releasing the page (called via
zap_pte_range). We currently delay the TLB operation until
tlb_end_vma() which happens after the page was released.

For my scenario, we also need pte_free_tlb() because of the pmd
caching in the TLB.

So your patch fixes both. My ack is still valid.

-- 
Catalin