On Thu, May 27, 2021 at 09:04:24AM +0100, Christoph Hellwig wrote:

On Wed, May 26, 2021 at 09:07:41AM +0100, Mel Gorman wrote:

quoted

+    !defined(CONFIG_DEBUG_LOCK_ALLOC) &&		\
+    !defined(CONFIG_PAHOLE_HAS_ZEROSIZE_PERCPU_SUPPORT)
+	/*
+	 * pahole 1.21 and earlier gets confused by zero-sized per-CPU
+	 * variables and produces invalid BTF. Ensure that
+	 * sizeof(struct pagesets) != 0 for older versions of pahole.
+	 */
+	char __pahole_hack;
+	#warning "pahole too old to support zero-sized struct pagesets"
+#endif

Err, hell no.  We should not mess up the kernel for broken tools that
are not relevant to the kernel build itself ever.

What do you suggest as an alternative?

I added Arnaldo to the cc as he tagged the last released version of
pahole (1.21) and may be able to tag a 1.22 with Andrii's fix for pahole
included.

The most obvious alternative fix for this issue is to require pahole
1.22 to set CONFIG_DEBUG_INFO_BTF but obviously a version 1.22 that works
needs to exist first and right now it does not. I'd be ok with this but
users of DEBUG_INFO_BTF may object given that it'll be impossible to set
the option until there is a release.

The second alternative fix is to embed the local_lock
within struct per_cpu_pages. It was shown this was possible in
https://lore.kernel.org/linux-rt-users/20210419141341.26047-1-mgorman@techsingularity.net/T/#md1001d7af52ac0d6d214b95e98fe051f9399de64 (local)
but I dropped it because it makes the locking protocol complex e.g.
config-specific lock-switchin in free_unref_page_list.

The last one is wrapping local_lock behind #defines and only defining the
per-cpu structures when local_lock_t is a non-zero size. That is simply
too ugly for words, the locking patterns should always be the same.


-- 
Mel Gorman
SUSE Labs