On Fri, Jan 03, 2025 at 11:11:51PM +0100, Jann Horn wrote: > On Fri, Jan 3, 2025 at 10:55 PM Rik van Riel <riel@xxxxxxxxxxx> wrote: > > On Fri, 2025-01-03 at 19:39 +0100, Jann Horn wrote: > > > 02fc2aa06e9e0ecdba3fe948cafe5892b72e86c0..3da645139748538daac70166618d > > > 8ad95116eb74 100644 > > > --- a/arch/x86/include/asm/tlbflush.h > > > +++ b/arch/x86/include/asm/tlbflush.h > > > @@ -242,7 +242,7 @@ void flush_tlb_multi(const struct cpumask > > > *cpumask, > > > flush_tlb_mm_range((vma)->vm_mm, start, > > > end, \ > > > ((vma)->vm_flags & > > > VM_HUGETLB) \ > > > ? > > > huge_page_shift(hstate_vma(vma)) \ > > > - : PAGE_SHIFT, false) > > > + : PAGE_SHIFT, true) > > > > > > > > > > The code looks good, but should this macro get > > a comment indicating that code that only frees > > pages, but not page tables, should be calling > > flush_tlb() instead? > > Documentation/core-api/cachetlb.rst seems to be the common place > that's supposed to document the rules - the macro I'm touching is just > the x86 implementation. (The arm64 implementation also has some fairly > extensive comments that say flush_tlb_range() "also invalidates any > walk-cache entries associated with translations for the specified > address range" while flush_tlb_page() "only invalidates a single, > last-level page-table entry and therefore does not affect any > walk-caches".) I wouldn't want to add yet more documentation for this > API inside the X86 code. I guess it would make sense to add pointers > from the x86 code to the documentation (and copy the details about > last-level TLBs from the arm64 code into the docs). Right, that documentation update might be nice. Anyway, I'm picking up this patch for tip/x86/mm once -rc1 happens. Thanks!
