linux-kernel - Re: [PATCH 2/7] x86,tlb: leave lazy TLB mode at page table free time

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1534397515.28585.138.camel@surriel.com>
Date:   Thu, 16 Aug 2018 01:31:55 -0400
From:   Rik van Riel <riel@...riel.com>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     LKML <linux-kernel@...r.kernel.org>, X86 ML <x86@...nel.org>,
        Mike Galbraith <efault@....de>,
        kernel-team <kernel-team@...com>, Ingo Molnar <mingo@...nel.org>,
        Dave Hansen <dave.hansen@...el.com>
Subject: Re: [PATCH 2/7] x86,tlb: leave lazy TLB mode at page table free time

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@...riel.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.
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)