lkml.org 
[lkml]   [2022]   [Oct]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [BUG?] X86 arch_tlbbatch_flush() seems to be lacking mm_tlb_flush_nested() integration
    On Sat, Oct 15, 2022 at 12:23 AM Kirill A. Shutemov
    <kirill@shutemov.name> wrote:
    > On Fri, Oct 14, 2022 at 08:19:42PM +0200, Jann Horn wrote:
    > > Hi!
    > >
    > > I haven't actually managed to reproduce this behavior, so maybe I'm
    > > just misunderstanding how this works; but I think the
    > > arch_tlbbatch_flush() path for batched TLB flushing in vmscan ought to
    > > have some kind of integration with mm_tlb_flush_nested().
    > >
    > > I think that currently, the following race could happen:
    > >
    > > [initial situation: page P is mapped into a page table of task B, but
    > > the page is not referenced, the PTE's A/D bits are clear]
    > > A: vmscan begins
    > > A: vmscan looks at P and P's PTEs, and concludes that P is not currently in use
    > > B: reads from P through the PTE, setting the Accessed bit and creating
    > > a TLB entry
    > > A: vmscan enters try_to_unmap_one()
    > > A: try_to_unmap_one() calls should_defer_flush(), which returns true
    > > A: try_to_unmap_one() removes the PTE and queues a TLB flush
    > > (arch_tlbbatch_add_mm())
    > > A: try_to_unmap_one() returns, try_to_unmap() returns to shrink_folio_list()
    > > B: calls munmap() on the VMA that mapped P
    > > B: no PTEs are removed, so no TLB flush happens
    > > B: munmap() returns
    >
    > I think here we will serialize against anon_vma/i_mmap lock in
    > __do_munmap() -> unmap_region() -> free_pgtables() that A also holds.
    >
    > So I believe munmap() is safe, but MADV_DONTNEED (and its flavours) is not.

    shrink_folio_list() is not in a context that is operating on a
    specific MM; it is operating on a list of pages that might be mapped
    into different processes all over the system.

    So A has temporarily held those locks somewhere inside
    try_to_unmap_one(), but it will drop them before it reaches the point
    where it issues the batched TLB flush.
    And this batched TLB flush potentially covers multiple MMs at once; it
    is not targeted towards a specific MM, but towards all of the CPUs on
    which any of the touched MMs might be active.

    \
     
     \ /
      Last update: 2022-10-15 00:31    [W:3.039 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site