[RFC]: Maybe revert 'MIPS: add PMD table accounting into MIPS'pmd_alloc_one' from 5.4 LTS?

[RFC]: Maybe revert 'MIPS: add PMD table accounting into MIPS'pmd_alloc_one' from 5.4 LTS?

[Joshua Kinard]

(resend using the correct list address)

Hi all,

I think there may be an issue with upstream commit
ed914d48b6a1040d1039d371b56273d422c0081e (MIPS: add PMD table accounting
into MIPS'pmd_alloc_one), which got backported to 5.4 LTS.

I am running my SGI Octane on the 5.4 LTS kernel series w/ Gentoo's IP30
patches, since basic IP30 support didn't get fully mainlined until ~5.7 and
I am just behind on kernels for the time being.  Generally, the 5.4 series
is stable on this machine, but as of using 5.4.133 yesterday and 5.4.134
today, I have had three kernel panics, all with completely different stack
traces, while building a multilib-enabled glibc-2.33 package.

Since the stack traces are all completely different, I am suspecting an
issue with memory allocation, which picking through the 5.4.133 changelog,
got me to look at commit ed914d48b6a1.  My initial thought is it looks like
this patch should not have been backported to 5.4, because 5.4 is missing
the asm-generic version of pmd_free(), which means the MIPS-specific version
is likely used instead, and that function was not updated alongside
pmd_alloc_one().

Now, I am not a memory person, but some quick reading indicates there may be
some other issues with this patch as well:

- use of 'GFP_KERNEL | __GFP_ACCOUNT' when there is GFP_KERNEL_ACCOUNT
available.

- the return value of pgtable_pmd_page_ctor(pg); is not checked to make sure
it succeeded.  Per Documentation/vm/split_page_table_lock, "NOTE:
pgtable_page_ctor() and pgtable_pmd_page_ctor() can fail -- it must
be handled properly."

- there is no call to pgtable_pmd_page_dtor() in pmd_free() in MIPS'
pgalloc.  The commit message indicates that pmd_free in asm-generic will
handle this, but that function doesn't exist in 5.4, leading to mismatched
implementations.

I think the fix is to either revert ed914d48b6a1 on the 5.4 stable series
(and any earlier series that are missing asm-generic pmd_free), or the MIPS
pmd_free() needs to be updated to call pgtable_pmd_page_dtor() in a similar
manner to x86 and arm64.  I'd opt for reverting.

Currently testing a build of 5.4.134 with this patch reverted running some
compile jobs.  If it survives building multilib glibc, then I think my
suspicions may be valid.

-- 
Joshua Kinard
Gentoo/MIPS
kumba@gentoo.org
rsa6144/5C63F4E3F5C6C943 2015-04-27
177C 1972 1FB8 F254 BAD0 3E72 5C63 F4E3 F5C6 C943

"The past tempts us, the present confuses us, the future frightens us.  And
our lives slip away, moment by moment, lost in that vast, terrible in-between."

--Emperor Turhan, Centauri Republic

Re: [RFC]: Maybe revert 'MIPS: add PMD table accounting into MIPS'pmd_alloc_one' from 5.4 LTS?

[Huang Pei]

On Tue, Jul 20, 2021 at 08:45:45PM -0400, Joshua Kinard wrote:
Hi, 
> (resend using the correct list address)
> 
> Hi all,
> 
> I think there may be an issue with upstream commit
> ed914d48b6a1040d1039d371b56273d422c0081e (MIPS: add PMD table accounting
> into MIPS'pmd_alloc_one), which got backported to 5.4 LTS.
> 
> I am running my SGI Octane on the 5.4 LTS kernel series w/ Gentoo's IP30
> patches, since basic IP30 support didn't get fully mainlined until ~5.7 and
> I am just behind on kernels for the time being.  Generally, the 5.4 series
> is stable on this machine, but as of using 5.4.133 yesterday and 5.4.134
> today, I have had three kernel panics, all with completely different stack
> traces, while building a multilib-enabled glibc-2.33 package.
> 
> Since the stack traces are all completely different, I am suspecting an
> issue with memory allocation, which picking through the 5.4.133 changelog,
> got me to look at commit ed914d48b6a1.  My initial thought is it looks like
> this patch should not have been backported to 5.4, because 5.4 is missing
> the asm-generic version of pmd_free(), which means the MIPS-specific version
> is likely used instead, and that function was not updated alongside
> pmd_alloc_one().
> 
> Now, I am not a memory person, but some quick reading indicates there may be
> some other issues with this patch as well:
> 
> - use of 'GFP_KERNEL | __GFP_ACCOUNT' when there is GFP_KERNEL_ACCOUNT
> available.
Yes, this is better
> 
> - the return value of pgtable_pmd_page_ctor(pg); is not checked to make sure
> it succeeded.  Per Documentation/vm/split_page_table_lock, "NOTE:
> pgtable_page_ctor() and pgtable_pmd_page_ctor() can fail -- it must
> be handled properly."

Yes, I missed this one
> 
> - there is no call to pgtable_pmd_page_dtor() in pmd_free() in MIPS'
> pgalloc.  The commit message indicates that pmd_free in asm-generic will
> handle this, but that function doesn't exist in 5.4, leading to mismatched
> implementations.

Yes, my fix is to fix the mismatch. 

I think all stable branches before 5.10 should *not* apply this my patch
> 
> I think the fix is to either revert ed914d48b6a1 on the 5.4 stable series
> (and any earlier series that are missing asm-generic pmd_free), or the MIPS
> pmd_free() needs to be updated to call pgtable_pmd_page_dtor() in a similar
> manner to x86 and arm64.  I'd opt for reverting.
> 
> Currently testing a build of 5.4.134 with this patch reverted running some
> compile jobs.  If it survives building multilib glibc, then I think my
> suspicions may be valid.
> 
> -- 
> Joshua Kinard
> Gentoo/MIPS
> kumba@gentoo.org
> rsa6144/5C63F4E3F5C6C943 2015-04-27
> 177C 1972 1FB8 F254 BAD0 3E72 5C63 F4E3 F5C6 C943
> 
> "The past tempts us, the present confuses us, the future frightens us.  And
> our lives slip away, moment by moment, lost in that vast, terrible in-between."
> 
> --Emperor Turhan, Centauri Republic

Re: [RFC]: Maybe revert 'MIPS: add PMD table accounting into MIPS'pmd_alloc_one' from 5.4 LTS?

[Joshua Kinard]

On 7/20/2021 22:11, Huang Pei wrote:
> On Tue, Jul 20, 2021 at 08:45:45PM -0400, Joshua Kinard wrote:
> Hi, 

[snip]

>>
>> - there is no call to pgtable_pmd_page_dtor() in pmd_free() in MIPS'
>> pgalloc.  The commit message indicates that pmd_free in asm-generic will
>> handle this, but that function doesn't exist in 5.4, leading to mismatched
>> implementations.
> 
> Yes, my fix is to fix the mismatch. 
> 
> I think all stable branches before 5.10 should *not* apply this my patch

Can you submit a revert of this commit to stable@vger.kernel.org so that
Greg can include it in an upcoming update to the stable kernels <5.10?

Thanks!,

-- 
Joshua Kinard
Gentoo/MIPS
kumba@gentoo.org
rsa6144/5C63F4E3F5C6C943 2015-04-27
177C 1972 1FB8 F254 BAD0 3E72 5C63 F4E3 F5C6 C943

"The past tempts us, the present confuses us, the future frightens us.  And
our lives slip away, moment by moment, lost in that vast, terrible in-between."

--Emperor Turhan, Centauri Republic

Re: [RFC]: Maybe revert 'MIPS: add PMD table accounting into MIPS'pmd_alloc_one' from 5.4 LTS?

[loongson]

Sure, it is done


> 在 2021年7月25日，07:22，Joshua Kinard <kumba@gentoo.org> 写道：
> 
> On 7/20/2021 22:11, Huang Pei wrote:
>> On Tue, Jul 20, 2021 at 08:45:45PM -0400, Joshua Kinard wrote:
>> Hi, 
> 
> [snip]
> 
>>> 
>>> - there is no call to pgtable_pmd_page_dtor() in pmd_free() in MIPS'
>>> pgalloc.  The commit message indicates that pmd_free in asm-generic will
>>> handle this, but that function doesn't exist in 5.4, leading to mismatched
>>> implementations.
>> 
>> Yes, my fix is to fix the mismatch. 
>> 
>> I think all stable branches before 5.10 should *not* apply this my patch
> 
> Can you submit a revert of this commit to stable@vger.kernel.org so that
> Greg can include it in an upcoming update to the stable kernels <5.10?
> 
> Thanks!,
> 
> -- 
> Joshua Kinard
> Gentoo/MIPS
> kumba@gentoo.org
> rsa6144/5C63F4E3F5C6C943 2015-04-27
> 177C 1972 1FB8 F254 BAD0 3E72 5C63 F4E3 F5C6 C943
> 
> "The past tempts us, the present confuses us, the future frightens us.  And
> our lives slip away, moment by moment, lost in that vast, terrible in-between."
> 
> --Emperor Turhan, Centauri Republic