On Wed, Aug 28, 2024 at 9:49 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, 28 Aug 2024 18:07:04 +0200 Mateusz Guzik <mjguzik@xxxxxxxxx> wrote:
>
> > The mutex array pointer shares a cacheline with the spinlock:
> > ffffffff84187480 B hugetlb_fault_mutex_table
> > ffffffff84187488 B hugetlb_lock
>
> Fair enough. My x86_64 defconfig now has
>
> num_fault_mutexes:
> .zero 4
> .globl hugetlb_lock
> .section .data..cacheline_aligned,"aw"
> .align 64
> .type hugetlb_lock, @object
> .size hugetlb_lock, 4
> hugetlb_lock:
> .zero 4
> .section .init.data
> .align 32
> .type default_hugepages_in_node, @object
> .size default_hugepages_in_node, 256
> default_hugepages_in_node:
> .zero 256
> .type parsed_default_hugepagesz, @object
> .size parsed_default_hugepagesz, 1
>
> which looks good.
>
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -72,14 +72,14 @@ static unsigned int default_hugepages_in_node[MAX_NUMNODES] __initdata;
> > * Protects updates to hugepage_freelists, hugepage_activelist, nr_huge_pages,
> > * free_huge_pages, and surplus_huge_pages.
> > */
> > -DEFINE_SPINLOCK(hugetlb_lock);
> > +__cacheline_aligned_in_smp DEFINE_SPINLOCK(hugetlb_lock);
> >
> > /*
> > * Serializes faults on the same logical page. This is used to
> > * prevent spurious OOMs when the hugepage pool is fully utilized.
> > */
> > -static int num_fault_mutexes;
> > -struct mutex *hugetlb_fault_mutex_table ____cacheline_aligned_in_smp;
> > +static __ro_after_init int num_fault_mutexes;
> > +__ro_after_init struct mutex *hugetlb_fault_mutex_table;
>
> It's conventional (within MM, at least) to put the section thing at the
> end of the definition, so tweak:
>
> --- a/mm/hugetlb.c~mm-hugetlb-sort-out-global-lock-annotations-fix
> +++ a/mm/hugetlb.c
> @@ -72,14 +72,14 @@ static unsigned int default_hugepages_in
> * Protects updates to hugepage_freelists, hugepage_activelist, nr_huge_pages,
> * free_huge_pages, and surplus_huge_pages.
> */
> -__cacheline_aligned_in_smp DEFINE_SPINLOCK(hugetlb_lock);
> +DEFINE_SPINLOCK(hugetlb_lock) __cacheline_aligned_in_smp;
>
I tried things in this order and this does not compile for me:
In file included from ./arch/x86/include/asm/current.h:10,
from ./arch/x86/include/asm/preempt.h:7,
from ./include/linux/preempt.h:79,
from ./include/linux/spinlock.h:56,
from ./include/linux/mmzone.h:8,
from ./include/linux/gfp.h:7,
from ./include/linux/mm.h:7,
from mm/hugetlb.c:8:
./include/linux/cache.h:80:3: error: expected ‘,’ or ‘;’ before ‘__attribute__’
80 | __attribute__((__aligned__(SMP_CACHE_BYTES), \
| ^~~~~~~~~~~~~
./include/linux/cache.h:86:36: note: in expansion of macro ‘__cacheline_aligned’
86 | #define __cacheline_aligned_in_smp __cacheline_aligned
| ^~~~~~~~~~~~~~~~~~~
mm/hugetlb.c:75:31: note: in expansion of macro ‘__cacheline_aligned_in_smp’
75 | DEFINE_SPINLOCK(hugetlb_lock) __cacheline_aligned_in_smp;
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
I'm at next-20240828 with gcc 13.2.0
> /*
> * Serializes faults on the same logical page. This is used to
> * prevent spurious OOMs when the hugepage pool is fully utilized.
> */
> -static __ro_after_init int num_fault_mutexes;
> -__ro_after_init struct mutex *hugetlb_fault_mutex_table;
> +static int num_fault_mutexes __ro_after_init;
> +struct mutex *hugetlb_fault_mutex_table __ro_after_init;
>
> /* Forward declaration */
> static int hugetlb_acct_memory(struct hstate *h, long delta);
> _
>
>
--
Mateusz Guzik <mjguzik gmail.com>
