Messages in this thread | | | Subject | Re: [PATCH 2/7] x86,tlb: leave lazy TLB mode at page table free time | From | Rik van Riel <> | Date | Thu, 16 Aug 2018 01:31:55 -0400 |
| |
On Wed, 2018-08-15 at 18:54 -0700, Andy Lutomirski wrote: > On Mon, Jul 16, 2018 at 12:03 PM, Rik van Riel <riel@surriel.com> > wrote:
> Hi Rik- > > I was looking through this, and I see: > > > -static void tlb_remove_table_one(void *table) > > +static void tlb_remove_table_one(void *table, struct mmu_gather > > *tlb) > > { > > /* > > * This isn't an RCU grace period and hence the page-tables > > cannot be > > @@ -344,7 +348,7 @@ static void tlb_remove_table_one(void *table) > > * It is however sufficient for software page-table walkers > > that rely on > > * IRQ disabling. See the comment near struct > > mmu_table_batch. > > */ > > - smp_call_function(tlb_remove_table_smp_sync, NULL, 1); > > + smp_call_function(tlb_remove_table_smp_sync, tlb->mm, 1); > > __tlb_remove_table(table); > > } > > But tlb_remove_table() doesn't always call > tlb_remove_table_one(). Do > the other paths through tlb_remove_table() do the right thing?
Urghhh, reading through the code it seems that I got one case wrong.
The obvious cases, where tlb->mm->mm_users >= 2, are both correct. The functions tlb_remove_table_one() and tlb_table_flush() both get rid of lazy TLB users of the MM.
However, there is one case where we don't but should: /* * When there's less then two users of this mm there cannot be a * concurrent page-table walk. */ if (atomic_read(&tlb->mm->mm_users) < 2) { __tlb_remove_table(table); return; }
I guess the obvious fix is to call tlb_flush_remove_tables() in the branch above. The first time it might shoot down a CPU in lazy TLB mode, while the second time it is called that CPU will have already switched to init_mm and no IPI is sent.
Alternatively, we could always do the batching, even when mm_users is 1, and examine the bitmap with the other CPUs less frequently in the exit path.
I can send in a patch for this tomorrow, or early next week, depending on what other stuff comes up...
-- All Rights Reversed.[unhandled content-type:application/pgp-signature] | |