Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   Andy Lutomirski <luto@amacapital.net>
Mime-Version: 1.0 (1.0)
Subject: Re: [RFC 15/20] mm: detect deferred TLB flushes in vma granularity
Date:   Mon, 1 Feb 2021 16:14:58 -0800
Message-Id: <8F37526F-8189-483A-A16E-E0EB8662AD98@amacapital.net>
References: <A6E4897D-8D5A-4084-8288-8E43F3039921@gmail.com>
Cc:     Linux-MM <linux-mm@kvack.org>, LKML <linux-kernel@vger.kernel.org>,
        Andy Lutomirski <luto@kernel.org>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Will Deacon <will@kernel.org>, Yu Zhao <yuzhao@google.com>,
        X86 ML <x86@kernel.org>
In-Reply-To: <A6E4897D-8D5A-4084-8288-8E43F3039921@gmail.com>
To:     Nadav Amit <nadav.amit@gmail.com>
Precedence: bulk


> On Feb 1, 2021, at 2:04 PM, Nadav Amit <nadav.amit@gmail.com> wrote:
>=20
> =EF=BB=BF
>>=20
>> On Jan 30, 2021, at 4:11 PM, Nadav Amit <nadav.amit@gmail.com> wrote:
>>=20
>> From: Nadav Amit <namit@vmware.com>
>>=20
>> Currently, deferred TLB flushes are detected in the mm granularity: if
>> there is any deferred TLB flush in the entire address space due to NUMA
>> migration, pte_accessible() in x86 would return true, and
>> ptep_clear_flush() would require a TLB flush. This would happen even if
>> the PTE resides in a completely different vma.
>=20
> [ snip ]
>=20
>> +static inline void read_defer_tlb_flush_gen(struct mmu_gather *tlb)
>> +{
>> +    struct mm_struct *mm =3D tlb->mm;
>> +    u64 mm_gen;
>> +
>> +    /*
>> +     * Any change of PTE before calling __track_deferred_tlb_flush() mus=
t be
>> +     * performed using RMW atomic operation that provides a memory barri=
ers,
>> +     * such as ptep_modify_prot_start().  The barrier ensure the PTEs ar=
e
>> +     * written before the current generation is read, synchronizing
>> +     * (implicitly) with flush_tlb_mm_range().
>> +     */
>> +    smp_mb__after_atomic();
>> +
>> +    mm_gen =3D atomic64_read(&mm->tlb_gen);
>> +
>> +    /*
>> +     * This condition checks for both first deferred TLB flush and for o=
ther
>> +     * TLB pending or executed TLB flushes after the last table that we
>> +     * updated. In the latter case, we are going to skip a generation, w=
hich
>> +     * would lead to a full TLB flush. This should therefore not cause
>> +     * correctness issues, and should not induce overheads, since anyhow=
 in
>> +     * TLB storms it is better to perform full TLB flush.
>> +     */
>> +    if (mm_gen !=3D tlb->defer_gen) {
>> +        VM_BUG_ON(mm_gen < tlb->defer_gen);
>> +
>> +        tlb->defer_gen =3D inc_mm_tlb_gen(mm);
>> +    }
>> +}
>=20
> Andy=E2=80=99s comments managed to make me realize this code is wrong. We m=
ust
> call inc_mm_tlb_gen(mm) every time.
>=20
> Otherwise, a CPU that saw the old tlb_gen and updated it in its local
> cpu_tlbstate on a context-switch. If the process was not running when the
> TLB flush was issued, no IPI will be sent to the CPU. Therefore, later
> switch_mm_irqs_off() back to the process will not flush the local TLB.
>=20
> I need to think if there is a better solution. Multiple calls to
> inc_mm_tlb_gen() during deferred flushes would trigger a full TLB flush
> instead of one that is specific to the ranges, once the flush actually tak=
es
> place. On x86 it=E2=80=99s practically a non-issue, since anyhow any updat=
e of more
> than 33-entries or so would cause a full TLB flush, but this is still ugly=
.
>=20

What if we had a per-mm ring buffer of flushes?  When starting a flush, we w=
ould stick the range in the ring buffer and, when flushing, we would read th=
e ring buffer to catch up.  This would mostly replace the flush_tlb_info str=
uct, and it would let us process multiple partial flushes together.=