Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <B976CC13-D014-433A-83DE-F8DF9AB4F421@surriel.com>
References: <20180716190337.26133-1-riel@surriel.com> <20180716190337.26133-5-riel@surriel.com>
 <CALCETrUMntRe_LYX5wj9YD4xZ+84QExK+ZNb3yxEBDEKa7nePQ@mail.gmail.com>
 <B6566AA4-2E8A-43BD-A224-0F6D4747B8FB@surriel.com> <F93A3335-4830-4E36-9179-D0FA5D640C8A@amacapital.net>
 <B976CC13-D014-433A-83DE-F8DF9AB4F421@surriel.com>
From:   Andy Lutomirski <luto@amacapital.net>
Date:   Thu, 19 Jul 2018 09:45:47 -0700
Message-ID: <CALCETrWDiMhgiR3f8n0jdWcW31EDJ+Waq0wh5sMDutfigANGnA@mail.gmail.com>
Subject: Re: [PATCH 4/7] x86,tlb: make lazy TLB mode lazier
To:     Rik van Riel <riel@surriel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Vitaly Kuznetsov <vkuznets@redhat.com>
Cc:     Andy Lutomirski <luto@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>, X86 ML <x86@kernel.org>,
        Mike Galbraith <efault@gmx.de>,
        kernel-team <kernel-team@fb.com>, Ingo Molnar <mingo@kernel.org>,
        Dave Hansen <dave.hansen@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

[I added PeterZ and Vitaly -- can you see any way in which this would
break something obscure?  I don't.]

On Thu, Jul 19, 2018 at 7:14 AM, Rik van Riel <riel@surriel.com> wrote:
> I guess we can skip both switch_ldt and load_mm_cr4 if real_prev equals
> next?

Yes, AFAICS.

>
> On to the lazy TLB mm_struct refcounting stuff :)
>
>>
>> Which refcount?  mm_users shouldn=E2=80=99t be hot, so I assume you=E2=
=80=99re talking about
>> mm_count. My suggestion is to get rid of mm_count instead of trying to
>> optimize it.
>
>
> Do you have any suggestions on how? :)
>
> The TLB shootdown sent at __exit_mm time does not get rid of the
> kernelthread->active_mm
> pointer pointing at the mm that is exiting.
>

Ah, but that's conceptually very easy to fix.  Add a #define like
ARCH_NO_TASK_ACTIVE_MM.  Then just get rid of active_mm if that
#define is set.  After some grepping, there are very few users.  The
only nontrivial ones are the ones in kernel/ and mm/mmu_context.c that
are involved in the rather complicated dance of refcounting active_mm.
If that field goes away, it doesn't need to be refcounted.  Instead, I
think the refcounting can get replaced with something like:

/*
 * Release any arch-internal references to mm.  Only called when
mm_users is zero
 * and all tasks using mm have either been switch_mm()'d away or have had
 * enter_lazy_tlb() called.
 */
extern void arch_shoot_down_dead_mm(struct mm_struct *mm);

which the kernel calls in __mmput() after tearing down all the page
tables.  The body can be something like:

if (WARN_ON(cpumask_any_but(mm_cpumask(...), ...)) {
  /* send an IPI.  Maybe just call tlb_flush_remove_tables() */
}

(You'll also have to fix up the highly questionable users in
arch/x86/platform/efi/efi_64.c, but that's easy.)

Does all that make sense?  Basically, as I understand it, the
expensive atomic ops you're seeing are all pointless because they're
enabling an optimization that hasn't actually worked for a long time,
if ever.