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;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\))
Subject: Re: [PATCH 4/7] x86,tlb: make lazy TLB mode lazier
From:   Rik van Riel <riel@surriel.com>
In-Reply-To: <CALCETrUMntRe_LYX5wj9YD4xZ+84QExK+ZNb3yxEBDEKa7nePQ@mail.gmail.com>
Date:   Wed, 18 Jul 2018 16:58:28 -0400
Cc:     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-Transfer-Encoding: 7bit
Message-Id: <B6566AA4-2E8A-43BD-A224-0F6D4747B8FB@surriel.com>
References: <20180716190337.26133-1-riel@surriel.com>
 <20180716190337.26133-5-riel@surriel.com>
 <CALCETrUMntRe_LYX5wj9YD4xZ+84QExK+ZNb3yxEBDEKa7nePQ@mail.gmail.com>
To:     Andy Lutomirski <luto@kernel.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> On Jul 17, 2018, at 4:04 PM, Andy Lutomirski <luto@kernel.org> wrote:
> 
> 
> I think you've introduced a minor-ish performance regression due to
> changing the old (admittedly terribly documented) control flow a bit.
> Before, if real_prev == next, we would skip:
> 
> load_mm_cr4(next);
> switch_ldt(real_prev, next);
> 
> Now we don't any more.  I think you should reinstate that
> optimization.  It's probably as simple as wrapping them in an if
> (real_priv != next) with a comment like /* Remote changes that would
> require a cr4 or ldt reload will unconditionally send an IPI even to
> lazy CPUs.  So, if we aren't changing our mm, we don't need to refresh
> cr4 or the ldt */

Looks like switch_ldt already skips reloading the LDT when prev equals
next, or when they simply have the same LDT values:

        if (unlikely((unsigned long)prev->context.ldt |
                     (unsigned long)next->context.ldt))
                load_mm_ldt(next);

It appears that the cr4 bits have a similar optimization:

static inline void cr4_set_bits(unsigned long mask)
{
        unsigned long cr4, flags;

        local_irq_save(flags);
        cr4 = this_cpu_read(cpu_tlbstate.cr4);
        if ((cr4 | mask) != cr4)
                __cr4_set(cr4 | mask);
        local_irq_restore(flags);
}

> 
> Hmm.  load_mm_cr4() should bypass itself when mm == &init_mm.  Want to
> fix that part or should I?
> 
Looks like there might not be anything to do here, after all.

On to the lazy TLB mm_struct refcounting stuff :)