Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.1\))
Subject: Re: [BUG?] X86 arch_tlbbatch_flush() seems to be lacking
 mm_tlb_flush_nested() integration
From:   Nadav Amit <nadav.amit@gmail.com>
In-Reply-To: <CAHk-=wh+dMSbeuEDatZbtycm9_RFyLe60nigQA8o5w0W_HkQSg@mail.gmail.com>
Date:   Sun, 16 Oct 2022 08:31:42 +0300
Cc:     Jann Horn <jannh@google.com>, Andy Lutomirski <luto@kernel.org>,
        Linux-MM <linux-mm@kvack.org>, Mel Gorman <mgorman@suse.de>,
        Rik van Riel <riel@redhat.com>,
        kernel list <linux-kernel@vger.kernel.org>,
        Kees Cook <keescook@chromium.org>,
        Ingo Molnar <mingo@kernel.org>,
        Sasha Levin <sasha.levin@oracle.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Will Deacon <will@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <2C85D898-7D10-4E5C-9B2C-017B202C7026@gmail.com>
References: <CAG48ez0B18eh3Q1853Cug8WSip7dPb2G9fhgqsPWzr0D_TBjRQ@mail.gmail.com>
 <0484E294-D6D6-45CE-87F7-5AFDA5309BA1@gmail.com>
 <CAHk-=wh+dMSbeuEDatZbtycm9_RFyLe60nigQA8o5w0W_HkQSg@mail.gmail.com>
To:     Linus Torvalds <torvalds@linuxfoundation.org>
Precedence: bulk

On Oct 16, 2022, at 2:47 AM, Linus Torvalds =
<torvalds@linuxfoundation.org> wrote:

> On Fri, Oct 14, 2022 at 8:51 PM Nadav Amit <nadav.amit@gmail.com> =
wrote:
>> Unless I am missing something, flush_tlb_batched_pending() is would =
be
>> called and do the flushing at this point, no?
>=20
> Ahh, yes.
>=20
> That seems to be doing the right thing, although looking a bit more at
> it, I think it might be improved.
>=20
> At least in the zap_pte_range() case, instead of doing a synchronous
> TLB flush if there are pending batched flushes, it migth be better if
> flush_tlb_batched_pending() would set the "need_flush_all" bit in the
> mmu_gather structure.
>=20
> That would possibly avoid that extra TLB flush entirely - since
> *normally* fzap_page_range() will cause a TLB flush anyway.
>=20
> Maybe it doesn't matter.

It seems possible and simple.

But in general, there are still various unnecessary TLB flushes due to =
the
TLB batching. Specifically, ptep_clear_flush() might flush unnecessarily
when pte_accessible() finds tlb_flush_pending holding a non-zero value.
Worse, the complexity of the code is high.

To simplify the TLB flushing mechanism and eliminate the unnecessary TLB
flushes, it is possible to track the =E2=80=9Ccompleted=E2=80=9D TLB =
generation (i.e., one
that was flushed). Tracking pending TLB flushes can be done in VMA- or
page-table granularity instead of mm-grnaulrity to avoid unnecessary =
flushes
on ptep_clear_flush(). Andy also suggested having a queue of the pending =
TLB
flushes.

The main problem is that each of the aforementioned enhancements can add
some cache references, and therefore might induce additional overheads. =
I
sent some patches before [1], which I can revive. The main question is
whether we can prioritize simplicity and unification of the various
TLB-flush batching mechanisms over (probably very small) performance =
gains.


[1] =
https://lore.kernel.org/linux-mm/20210131001132.3368247-1-namit@vmware.com=
/=