MIME-Version: 1.0
In-Reply-To: <20161215164240.904945084@linutronix.de>
References: <20161215162648.061449202@linutronix.de> <20161215164240.904945084@linutronix.de>
From: Andy Lutomirski <luto@amacapital.net>
Date: Thu, 15 Dec 2016 09:31:39 -0800
Message-ID: <CALCETrUbzDAfjnLqsFpAdOkx9qK0ircasXXqUzQ4ON-oq31fRg@mail.gmail.com>
Subject: Re: [patch 3/3] x86/process: Optimize TIF_NOTSC switch
To: Thomas Gleixner <tglx@linutronix.de>
Cc: LKML <linux-kernel@vger.kernel.org>, X86 ML <x86@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>, Kyle Huey <khuey@kylehuey.com>,
        Andy Lutomirski <luto@kernel.org>
Content-Type: text/plain; charset=UTF-8
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1296
Lines: 45

On Thu, Dec 15, 2016 at 8:44 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> Provide and use a toggle helper instead of doing it with a branch.
>
> x86_64:
> 3662       8505      16   12183    2f97 Before
> 3646       8505      16   12167    2f87 After
>
> i386:
> 5906       9388    1804   17098    42ca Before
> 5834       9324    1740   16898    4202 After
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/x86/include/asm/tlbflush.h |   10 ++++++++++
>  arch/x86/kernel/process.c       |   22 ++++------------------
>  2 files changed, 14 insertions(+), 18 deletions(-)
>
> --- a/arch/x86/include/asm/tlbflush.h
> +++ b/arch/x86/include/asm/tlbflush.h
> @@ -110,6 +110,16 @@ static inline void cr4_clear_bits(unsign
>         }
>  }
>
> +static inline void cr4_toggle_bits(unsigned long mask)
> +{
> +       unsigned long cr4;
> +
> +       cr4 = this_cpu_read(cpu_tlbstate.cr4);
> +       cr4 ^= mask;
> +       this_cpu_write(cpu_tlbstate.cr4, cr4);
> +       __write_cr4(cr4);
> +}

This scares me for the same reason as BTF, although this should at
least be less fragile.  But how about:

static inline void cr4_set_bit_to(unsigned long mask, bool set)
{
  ...
  cr4 &= ~mask;
  cr4 ^= (set << ilog2(mask));
  ...
}

This should generate code that's nearly as good.