Re: [PATCH] mm/hugetlb: sort out global lock annotations

[Andrew Morton]

On Wed, 28 Aug 2024 22:13:49 +0200 Mateusz Guzik <mjguzik@xxxxxxxxx> wrote:

> > 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;
>       |                               ^~~~~~~~~~~~~~~~~~~~~~~~~~

Well that's annoying.  It's because DEFINE_SPINLOCK includes an initializer.

--- a/mm/hugetlb.c~mm-hugetlb-sort-out-global-lock-annotations-fix-fix
+++ a/mm/hugetlb.c
@@ -72,7 +72,7 @@ static unsigned int default_hugepages_in
  * 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;
+spinlock_t hugetlb_lock __cacheline_aligned_in_smp = __SPIN_LOCK_UNLOCKED(hugetlb_lock);
 
 /*
  * Serializes faults on the same logical page.  This is used to
_

We'd need a new DEFINE_SPINLOCK_ALIGNED() or something.

Ho hum, I'll fix.