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 (1.0)
Subject: Re: [PATCH] x86: Align TLB invalidation info
From:   Andy Lutomirski <luto@amacapital.net>
In-Reply-To: <20180131201118.1694-1-namit@vmware.com>
Date:   Wed, 31 Jan 2018 12:24:44 -0800
Cc:     x86@kernel.org, Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        "H. Peter Anvin" <hpa@zytor.com>, linux-kernel@vger.kernel.org,
        Peter Zijlstra <peterz@infradead.org>,
        Nadav Amit <nadav.amit@gmail.com>,
        Andy Lutomirski <luto@kernel.org>,
        Dave Hansen <dave.hansen@linux.intel.com>
Content-Transfer-Encoding: quoted-printable
Message-Id: <BD28D2CF-CAD7-45A1-87D0-DE38F7B0EFE5@amacapital.net>
References: <20180131201118.1694-1-namit@vmware.com>
To:     Nadav Amit <namit@vmware.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


> On Jan 31, 2018, at 12:11 PM, Nadav Amit <namit@vmware.com> wrote:
>=20
> The TLB invalidation info is allocated on the stack, which might cause
> it to be unaligned. Since this information may be transferred to
> different cores for TLB shootdown, this might result in an additional
> cache-line bouncing between the cores.
>=20
> GCC provides a way to deal with it by using
> __builtin_alloca_with_align(). Use it to avoid the bouncing cache lines.
>=20

Eww.  How about __aligned?


> Signed-off-by: Nadav Amit <namit@vmware.com>
>=20
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> ---
> arch/x86/mm/tlb.c              | 21 +++++++++++----------
> include/linux/compiler-gcc.h   |  5 +++++
> include/linux/compiler_types.h |  4 ++++
> 3 files changed, 20 insertions(+), 10 deletions(-)
>=20
> diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
> index 5bfe61a5e8e3..bab7bb5d982f 100644
> --- a/arch/x86/mm/tlb.c
> +++ b/arch/x86/mm/tlb.c
> @@ -574,37 +574,38 @@ static unsigned long tlb_single_page_flush_ceiling _=
_read_mostly =3D 33;
> void flush_tlb_mm_range(struct mm_struct *mm, unsigned long start,
>                unsigned long end, unsigned long vmflag)
> {
> +    struct flush_tlb_info *info;
>    int cpu;
>=20
> -    struct flush_tlb_info info =3D {
> -        .mm =3D mm,
> -    };
> +    info =3D __alloca_with_align(sizeof(*info),
> +                   SMP_CACHE_BYTES * BITS_PER_BYTE);
> +    info->mm =3D mm;
>=20
>    cpu =3D get_cpu();
>=20
>    /* This is also a barrier that synchronizes with switch_mm(). */
> -    info.new_tlb_gen =3D inc_mm_tlb_gen(mm);
> +    info->new_tlb_gen =3D inc_mm_tlb_gen(mm);
>=20
>    /* Should we flush just the requested range? */
>    if ((end !=3D TLB_FLUSH_ALL) &&
>        !(vmflag & VM_HUGETLB) &&
>        ((end - start) >> PAGE_SHIFT) <=3D tlb_single_page_flush_ceiling) {=

> -        info.start =3D start;
> -        info.end =3D end;
> +        info->start =3D start;
> +        info->end =3D end;
>    } else {
> -        info.start =3D 0UL;
> -        info.end =3D TLB_FLUSH_ALL;
> +        info->start =3D 0UL;
> +        info->end =3D TLB_FLUSH_ALL;
>    }
>=20
>    if (mm =3D=3D this_cpu_read(cpu_tlbstate.loaded_mm)) {
>        VM_WARN_ON(irqs_disabled());
>        local_irq_disable();
> -        flush_tlb_func_local(&info, TLB_LOCAL_MM_SHOOTDOWN);
> +        flush_tlb_func_local(info, TLB_LOCAL_MM_SHOOTDOWN);
>        local_irq_enable();
>    }
>=20
>    if (cpumask_any_but(mm_cpumask(mm), cpu) < nr_cpu_ids)
> -        flush_tlb_others(mm_cpumask(mm), &info);
> +        flush_tlb_others(mm_cpumask(mm), info);
>=20
>    put_cpu();
> }
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 631354acfa72..aea9a2e69417 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -314,6 +314,11 @@
> #define __designated_init __attribute__((designated_init))
> #endif
>=20
> +#if GCC_VERSION >=3D 60100
> +#define __alloca_with_align(size, alignment)                \
> +    __builtin_alloca_with_align(size, alignment)
> +#endif
> +
> #endif    /* gcc version >=3D 40000 specific checks */
>=20
> #if !defined(__noclone)
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types=
.h
> index 6b79a9bba9a7..c71297d95c74 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -271,4 +271,8 @@ struct ftrace_likely_data {
> # define __native_word(t) (sizeof(t) =3D=3D sizeof(char) || sizeof(t) =3D=3D=
 sizeof(short) || sizeof(t) =3D=3D sizeof(int) || sizeof(t) =3D=3D sizeof(lo=
ng))
> #endif
>=20
> +#ifndef __alloca_with_align
> +#define __alloca_with_align(size, alignment) __builtin_alloca(size)
> +#endif
> +
> #endif /* __LINUX_COMPILER_TYPES_H */
> --=20
> 2.14.1
>=20