Re: linux-next: build failure after merge of the net-next tree
From: Jakub Kicinski <kuba@kernel.org>
Date: 2024-09-13 20:24:39
Also in:
linux-next, linuxppc-dev, lkml
Subsystem:
networking [general], page pool, the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jesper Dangaard Brouer, Ilias Apalodimas, Linus Torvalds
On Fri, 13 Sep 2024 13:05:32 -0700 Mina Almasry wrote:
Change, got NAKed: https://lore.kernel.org/netdev/ZuSQ9BT9Vg7O2kXv@casper.infradead.org/ (local)
Humpf.
quoted hunk ↗ jump to hunk
But AFAICT we don't really need to do this inside of mm, affecting things like compound_head. This equivalent change also makes the build pass. Does this look good?diff --git a/include/net/netmem.h b/include/net/netmem.h index 8a6e20be4b9d..58f2120cd392 100644 --- a/include/net/netmem.h +++ b/include/net/netmem.h@@ -100,7 +100,15 @@ static inline netmem_ref net_iov_to_netmem(structnet_iov *niov) static inline netmem_ref page_to_netmem(struct page *page) { - return (__force netmem_ref)page; + /* page* exported from the mm stack would not have the LSB set, but the + * GCC 14 powerpc compiler will optimize reads into this pointer into + * unaligned reads as it sees address arthemetic in _compound_head(). + * + * Explicitly clear the LSB until what looks like a GCC compiler issue + * is resolved. + */ + DEBUG_NET_WARN_ON_ONCE((unsigned long)page & 1UL); + return (__force netmem_ref)page & ~1UL; }
Hmm. Not really, the math this is doing is a bit of a cargo cult, AFAIU the operation itself is meaningless. It works because it achieves breaking the optimization/register chain in the compiler. But the exact ALU op doesn't matter. So pretending LSB is meaningful could be confusing to the reader. I think this will achieve the same effect without the spurious ALU operations (apologies for broken whitespace):
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index a813d30d2135..b7e0acaed933 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c@@ -864,7 +864,11 @@ void page_pool_put_page_bulk(struct page_pool *pool, void **data, allow_direct = page_pool_napi_local(pool); for (i = 0; i < count; i++) { - netmem_ref netmem = page_to_netmem(virt_to_head_page(data[i])); + struct page *page = virt_to_head_page(data[i]); + netmem_ref netmem; + + /* $explanation */ + netmem = page_to_netmem(READ_ONCE(page)); /* It is not the last user for the page frag case */ if (!page_pool_is_last_ref(netmem))
If it makes sense could you polish it up and submit?