[lkml]   [2002]   [May]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    SubjectQ: backport of the free_pgtables tlb fixes to 2.4
    Hello Linus,

    I'm dealing with the backport of the free_pgtables() tlb flushes fixes
    to 2.4 (got interrupted with it in the middle of the inode highmem
    imbalance). I seen a patch and an explanation floating around but it's
    wrong as far I can tell in the sense it seems completly a noop.

    As far I can see by reading the code the only kernel bug was in the
    free_pgtables path and nothing else, so in theory by commenting out the
    call of free_pgtables (that is not strictly required except for security
    against malicious proggy) the segfault should go away too (could be even
    a valid workaround for 2.4). Also it's not related to the P4 cpu, it's
    just a generic bug for all archs it seems.

    In both 2.4 and 2.5 we were just doing the release of the pages
    correctly in zap_page_range, by clearing the pte entry, then
    invalidating the tlb and finally freeing the page, I can't see problems
    in the normal do_munmap path unless free_pgtables triggers too.

    Only in free_pgtables there's an exception: if during an munmap a whole
    pgd slot is unmapped we go to a deeper level and we throw away both pmd
    and pte too (under the now empty pgd slot) to make space and avoid
    expoits that fills the whole pagetable unswappable metadata but without
    real pages associated to it.

    The problem is that while freeing the pte and pmd we don't do the
    "clear; invalidate tlb; free ram" ordered-thread-SMP-safe shootdown
    sequence but we instead do the racy "clear; free; invalidate tlb"
    SMP-thread-unsafe ordering and that's _THE_ bug. So the fix is simply to
    extend the tlb_gather_mm/tlb_finish_mmu/tlb_remove_page to the
    free_pgtables path too (while dropping pte/pmd), so in turn to
    clear_page_tables, even if only the "free_pgtables" caller really needs
    it. exit_mmap obviously cannot need the enforced "clear; invalidate tlb;
    free ram" ordering because by the time exit_mmap is recalled there
    cannot be any racy reader of the address space in parallel [mm_users ==
    0] (and higher performance for exit_mmap are taken care by the fast
    mode, or better it was supposed to be taken care before the fast mode
    broke in latest 2.5, see below).

    I don't see why you changed the "fast mode" to 1 cpu (even without an
    #ifdef SMP), if the mm_users is == 1 we are just guaranteed that no
    parallel reading of the address space that we're working on can happen
    (we're also guaranteed that mm_users cannot increase or decrease under
    us), no matter on the number of the cpus. I think the "fast mode" should
    return to the more efficient one of 2.4, not to be "slow mode for every
    true SMP".

    About the patch floating around I also seen random changes in leave_mm
    that I cannot explain (that's in 2.5 too, I think that part of 2.5
    is superflous too infact)

    * We cannot call mmdrop() because we are in interrupt context,
    * instead update mm->cpu_vm_mask.
    + *
    + * We need to reload %cr3 since the page tables may be going
    + * away from under us..
    static void inline leave_mm (unsigned long cpu)
    if (cpu_tlbstate[cpu].state == TLBSTATE_OK)
    clear_bit(cpu, &cpu_tlbstate[cpu].active_mm->cpu_vm_mask);
    + load_cr3(swapper_pg_dir);


    explanation of why the above is superflous: every time a task enters the
    lazy state we bump the mm_users so the active_mm is pinned somehow and
    nothing can throw away the kernel side of the pgd, so we cannot care
    less about loading swapper_pg_dir in %cr3 for tasks in lazy state, it's
    simply not required, so I think the patch above is superflous. The
    pinned mm (if it's the last refernece) is released by the scheduler
    after our cr3 changed in switch_mm. The comment is wrong, our page
    tables cannot go away under us, because being a lazy task it only
    allowed to use the kernel side of the address space and nothing can
    throw it away.

    A change like this (not from 2.5) as well is obviously superflous:

    #define tlb_remove_page(ctxp, pte, addr) do {\
    /* Handle the common case fast, first. */\
    if ((ctxp)->nr == ~0UL) {\
    - __free_pte(*(pte));\
    - pte_clear((pte));\
    + pte_t __pte = *(pte);\
    + pte_clear(pte);\
    + __free_pte(__pte);\

    nr == ~0UL is the single threaded case, so again there cannot be any
    parallel reader of the address space, and so it's perfectly fine to free
    the page, clear the pte and finally flush the tlb, we don't care about
    the ordering if we're single threaded, any ordering is ok, nobody can
    access the address space while we're working on it.

    This below change as well is superflous after we backout the above
    change to leave_mm.

    @@ -51,9 +51,9 @@
    if(!test_and_set_bit(cpu, &next->cpu_vm_mask)) {
    /* We were in lazy tlb mode and leave_mm * disabled
    - * tlb flush IPI delivery. We must flush our
    + * tlb flush IPI delivery. We must reload %cr3.
    - local_flush_tlb();
    + load_cr3(next->pgd);

    This actually gets more near to the real problem...
    static inline void flush_tlb_pgtables(struct mm_struct *mm,
    unsigned long start, unsigned long end)
    - /* i386 does not keep any page table caches in TLB */
    + flush_tlb_mm(mm);
    void clear_page_tables(struct mm_struct *mm, unsigned long first, int
    pgd_t * page_dir = mm->pgd;
    + unsigned long last = first + nr;
    page_dir += first;
    @@ -153,6 +154,8 @@
    } while (--nr);
    + flush_tlb_pgtables(mm, first * PGDIR_SIZE, last * PGDIR_SIZE);
    /* keep the page table cache within bounds */
    ... but it's again a noop, cannot fix anything, still any tlb flush done
    there is way too late, the pte just gone away in the freelist at that
    time. It may reduce the window for the race at best.

    The only effective fix that nobody backported yet to 2.4 AFIK is to
    avoid the race between free_pgtables and a parallel reader thread in the
    address space, and it consists in backporting the pte_free_tlb(tlb, pte)
    in the clear_page_tables path to 2.4, nothing else and nothing more. the
    zap_page_range path and the fast mode of 2.4 just looks perfectly
    correct, only free_pgtables must start using the "slow mode that enforce
    ordering" when the mm_count is > 1 (either that or commenting out
    free_pgtables will be a valid workaround for the SMP race).
    Could you confirm/comment or explain what the problem really is? Many

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2005-03-22 13:26    [W:0.030 / U:39.532 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site