[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <913e2fbd-d318-4c9b-aed2-4d333a1d5cf0@cs-soprasteria.com>
Date: Fri, 13 Sep 2024 16:13:17 +0000
From: LEROY Christophe <christophe.leroy2@...soprasteria.com>
To: Jakub Kicinski <kuba@...nel.org>, Stephen Rothwell <sfr@...b.auug.org.au>
CC: David Miller <davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>, Mina
Almasry <almasrymina@...gle.com>, Networking <netdev@...r.kernel.org>, Linux
Kernel Mailing List <linux-kernel@...r.kernel.org>, Linux Next Mailing List
<linux-next@...r.kernel.org>, Arnd Bergmann <arnd@...db.de>,
"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: linux-next: build failure after merge of the net-next tree
Le 13/09/2024 à 17:49, Jakub Kicinski a écrit :
> On Fri, 13 Sep 2024 08:34:26 -0700 Jakub Kicinski wrote:
>>> 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;
Christophe
Powered by blists - more mailing lists