Re: linux-next: build failure after merge of the net-next tree

On Fri, Sep 13, 2024 at 9:13 AM LEROY Christophe
[off-list ref] wrote:

Le 13/09/2024 à 17:49, Jakub Kicinski a écrit :

quoted

On Fri, 13 Sep 2024 08:34:26 -0700 Jakub Kicinski wrote:

quoted

quoted

The second "asm" above (CONFIG_PPC_KERNEL_PREFIXED is not set).  I am
guessing by searching for "39" in net/core/page_pool.s

This is maybe called from page_pool_unref_netmem()

Thanks! The compiler version helped, I can repro with GCC 14.

It's something special about compound page handling on powerpc64,
AFAICT. I'm guessing that the assembler is mad that we're doing
an unaligned read:

    3300         ld 8,39(8)       # MEM[(const struct atomic64_t *)_29].counter, t

which does indeed look unaligned to a naked eye. If I replace
virt_to_head_page() with virt_to_page() on line 867 in net/core/page_pool.c
I get:

    2982         ld 8,40(10)      # MEM[(const struct atomic64_t *)_94].counter, t

and that's what we'd expect. It's reading pp_ref_count which is at
offset 40 in struct net_iov. I'll try to take a closer look at
the compound page handling, with powerpc assembly book in hand,
but perhaps this rings a bell for someone?

Oh, okay, I think I understand now. My lack of MM knowledge showing.
So if it's a compound head we do:

static inline unsigned long _compound_head(const struct page *page)
{
         unsigned long head = READ_ONCE(page->compound_head);

         if (unlikely(head & 1))
                 return head - 1;
         return (unsigned long)page_fixed_fake_head(page);
}

Presumably page->compound_head stores the pointer to the head page.
I'm guessing the compiler is "smart" and decides "why should I do
ld (page - 1) + 40, when I can do ld page + 39 :|

I think it's a compiler bug...

Would it work if you replace it with following ?

        return head & ~1;

I was able to reproduce with the correct compiler version, and yes,
this fixes the build for me. Thanks!

Probably healthy to add UL, yes?

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 5769fe6e4950..ea4005d2d1a9 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h

@@ -239,8 +239,8 @@ static inline unsigned long _compound_head(const

struct page *page)
 {
        unsigned long head = READ_ONCE(page->compound_head);

-       if (unlikely(head & 1))
-               return head - 1;
+       if (unlikely(head & 1UL))
+               return head & ~1UL;
        return (unsigned long)page_fixed_fake_head(page);
 }

Other than that I think this is a correct fix. Jakub, what to do here.
Do I send this fix to the mm tree or to net-next?

-- 
Thanks,
Mina