On Wed, Aug 28, 2024 at 10:44 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > > 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. that would be a nice addition so as is this triviality grew to 3 patches which I consider rather extreme, and the middle one breaks the build In the vfs land this would get squashed into one commit with a maintainer note that some tweaking was performed, which I would suggest here alternatively, given the trivial nature of the entire thing, if you add DEFINE_SPINLOCK_ALIGNED and do the annotation tweak, you may as well commit this as your own patch. I don't need any credit -- Mateusz Guzik <mjguzik gmail.com>
