| Message ID | 20210721034335.2015914-1-huangpei@loongson.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MIPS: check return value of pgtable_pmd_page_ctor | expand |
On 7/20/2021 23:43, Huang Pei wrote: > +. According to Documentation/vm/split_page_table_lock, handle failure > of pgtable_pmd_page_ctor > > +. use GFP_KERNEL_ACCOUNT instead of GFP_KERNEL|__GFP_ACCOUNT > > Reported-by: Joshua Kinard <kumba@gentoo.org> > Signed-off-by: Huang Pei <huangpei@loongson.cn> > --- > arch/mips/include/asm/pgalloc.h | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/arch/mips/include/asm/pgalloc.h b/arch/mips/include/asm/pgalloc.h > index d0cf997b4ba8..5c9597a6c60c 100644 > --- a/arch/mips/include/asm/pgalloc.h > +++ b/arch/mips/include/asm/pgalloc.h > @@ -62,11 +62,15 @@ static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long address) > pmd_t *pmd = NULL; > struct page *pg; > > - pg = alloc_pages(GFP_KERNEL | __GFP_ACCOUNT, PMD_ORDER); > + pg = alloc_pages(GFP_KERNEL_ACCOUNT, PMD_ORDER); > if (pg) { > - pgtable_pmd_page_ctor(pg); > - pmd = (pmd_t *)page_address(pg); > - pmd_init((unsigned long)pmd, (unsigned long)invalid_pte_table); > + if(pgtable_pmd_page_ctor(pg)) { > + pmd = (pmd_t *)page_address(pg); > + pmd_init((unsigned long)pmd, (unsigned long)invalid_pte_table); > + } else { > + __free_pages(pg, PMD_ORDER); > + } > + > } > return pmd; > } > Instead of the nested if statements, why not go with something that looks more like what is in arch/x86/include/asm/pgalloc.h? Note, I don't have a full kernel tree in front of me at the moment, so this is just the refactored function of pmd_alloc_one: static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long address) { pmd_t *pmd; struct page *page; page = alloc_pages(GFP_KERNEL_ACCOUNT, PMD_ORDER); if (!page) return NULL; if (!pgtable_pmd_page_ctor(page)) { __free_pages(page, PMD_ORDER); return NULL; } pmd = (pmd_t *)page_address(page); pmd_init((unsigned long)pmd, (unsigned long)invalid_pte_table); return pmd; }
On Wed, Jul 21, 2021 at 12:23:39AM -0400, Joshua Kinard wrote: > On 7/20/2021 23:43, Huang Pei wrote: > > +. According to Documentation/vm/split_page_table_lock, handle failure > > of pgtable_pmd_page_ctor > > > > +. use GFP_KERNEL_ACCOUNT instead of GFP_KERNEL|__GFP_ACCOUNT > > > > Reported-by: Joshua Kinard <kumba@gentoo.org> > > Signed-off-by: Huang Pei <huangpei@loongson.cn> > > --- > > arch/mips/include/asm/pgalloc.h | 12 ++++++++---- > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/arch/mips/include/asm/pgalloc.h b/arch/mips/include/asm/pgalloc.h > > index d0cf997b4ba8..5c9597a6c60c 100644 > > --- a/arch/mips/include/asm/pgalloc.h > > +++ b/arch/mips/include/asm/pgalloc.h > > @@ -62,11 +62,15 @@ static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long address) > > pmd_t *pmd = NULL; > > struct page *pg; > > > > - pg = alloc_pages(GFP_KERNEL | __GFP_ACCOUNT, PMD_ORDER); > > + pg = alloc_pages(GFP_KERNEL_ACCOUNT, PMD_ORDER); > > if (pg) { > > - pgtable_pmd_page_ctor(pg); > > - pmd = (pmd_t *)page_address(pg); > > - pmd_init((unsigned long)pmd, (unsigned long)invalid_pte_table); > > + if(pgtable_pmd_page_ctor(pg)) { > > + pmd = (pmd_t *)page_address(pg); > > + pmd_init((unsigned long)pmd, (unsigned long)invalid_pte_table); > > + } else { > > + __free_pages(pg, PMD_ORDER); > > + } > > + > > } > > return pmd; > > } > > > Instead of the nested if statements, why not go with something that looks more > like what is in arch/x86/include/asm/pgalloc.h? > > Note, I don't have a full kernel tree in front of me at the moment, so this is > just the refactored function of pmd_alloc_one: > > static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long address) > { > pmd_t *pmd; > struct page *page; > > page = alloc_pages(GFP_KERNEL_ACCOUNT, PMD_ORDER); > if (!page) > return NULL; > > if (!pgtable_pmd_page_ctor(page)) { > __free_pages(page, PMD_ORDER); > return NULL; > } > > pmd = (pmd_t *)page_address(page); > pmd_init((unsigned long)pmd, (unsigned long)invalid_pte_table); > > return pmd; > } > Much more readable, V2 just resend, thank you! PS: Latest Gentoo/MIPS stage3 is only available for big endian, is there any little endian stage3 available? > -- > 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
On 7/21/2021 04:13, Huang Pei wrote: > On Wed, Jul 21, 2021 at 12:23:39AM -0400, Joshua Kinard wrote: >> On 7/20/2021 23:43, Huang Pei wrote: >>> +. According to Documentation/vm/split_page_table_lock, handle failure >>> of pgtable_pmd_page_ctor >>> >>> +. use GFP_KERNEL_ACCOUNT instead of GFP_KERNEL|__GFP_ACCOUNT >>> >>> Reported-by: Joshua Kinard <kumba@gentoo.org> >>> Signed-off-by: Huang Pei <huangpei@loongson.cn> [snip] > PS: > > Latest Gentoo/MIPS stage3 is only available for big endian, is there any > little endian stage3 available? At this time from us, unfortunately nothing recent. I only focus on SGI big-endian systems, as these are still the most widely-available (eBay) MIPS systems that can still compile code within reasonable timeframes on Linux. The Gentoo Embedded team has mipsel3 stages based on uclibc-ng from ~2018 available here: https://gentoo.osuosl.org/experimental/mips/uclibc/mipsel3/ And some uclibc-ng mips32r2 stages, also 2018, here: https://gentoo.osuosl.org/experimental/mips/uclibc/mips32r2/ Unfortunately, our support for more recent uclibc-ng-based builds has fallen off the radar due to lack of upstream uclibc-ng activity. I believe the Embedded team is only focusing on musl libc-based build from now on. There are some unofficial MIPS32 little-endian softfloat stages provided by Manuel Lauss on his website here, albeit from 2018: http://mlau.at/files/mips32-linux/ Last, if you truly need something to work with, I have had luck using the OpenADK project's build system to coax musl-based big-endian rootfs tarballs out of it in the past. It's a very flexible build system and should be capable of also generating mips32r* and little-endian builds as well. It is somewhat fickle, though, so you may have to kludge the build process at times to get it to complete. Depends very heavily on what packages you want to build in.
diff --git a/arch/mips/include/asm/pgalloc.h b/arch/mips/include/asm/pgalloc.h index d0cf997b4ba8..5c9597a6c60c 100644 --- a/arch/mips/include/asm/pgalloc.h +++ b/arch/mips/include/asm/pgalloc.h @@ -62,11 +62,15 @@ static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long address) pmd_t *pmd = NULL; struct page *pg; - pg = alloc_pages(GFP_KERNEL | __GFP_ACCOUNT, PMD_ORDER); + pg = alloc_pages(GFP_KERNEL_ACCOUNT, PMD_ORDER); if (pg) { - pgtable_pmd_page_ctor(pg); - pmd = (pmd_t *)page_address(pg); - pmd_init((unsigned long)pmd, (unsigned long)invalid_pte_table); + if(pgtable_pmd_page_ctor(pg)) { + pmd = (pmd_t *)page_address(pg); + pmd_init((unsigned long)pmd, (unsigned long)invalid_pte_table); + } else { + __free_pages(pg, PMD_ORDER); + } + } return pmd; }
+. According to Documentation/vm/split_page_table_lock, handle failure of pgtable_pmd_page_ctor +. use GFP_KERNEL_ACCOUNT instead of GFP_KERNEL|__GFP_ACCOUNT Reported-by: Joshua Kinard <kumba@gentoo.org> Signed-off-by: Huang Pei <huangpei@loongson.cn> --- arch/mips/include/asm/pgalloc.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)