| Message ID | 20211130172954.129587-2-vladimir.murzin@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | percpu: km: ensure it is used with NOMMU (either UP or SMP) | expand |
Hello, On Tue, Nov 30, 2021 at 05:29:54PM +0000, Vladimir Murzin wrote: > Currently, NOMMU pull km allocator via !SMP dependency because most of > them are UP, yet for SMP+NOMMU vm allocator gets pulled which: > > * may lead to broken build [1] > * ...or not working runtime due to [2] > > It looks like SMP+NOMMU case was overlooked in bbddff054587 ("percpu: > use percpu allocator on UP too") so restore that. > > [1] > For ARM SMP+NOMMU (R-class cores) > > arm-none-linux-gnueabihf-ld: mm/percpu.o: in function `pcpu_post_unmap_tlb_flush': > mm/percpu-vm.c:188: undefined reference to `flush_tlb_kernel_range' > > [2] > static inline > int vmap_pages_range_noflush(unsigned long addr, unsigned long end, > pgprot_t prot, struct page **pages, unsigned int page_shift) > { > return -EINVAL; > } > > Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > --- > mm/Kconfig | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/mm/Kconfig b/mm/Kconfig > index d16ba92..66331e0 100644 > --- a/mm/Kconfig > +++ b/mm/Kconfig > @@ -425,9 +425,8 @@ config THP_SWAP > # UP and nommu archs use km based percpu allocator > # > config NEED_PER_CPU_KM > - depends on !SMP > bool > - default y > + default !SMP || !MMU > Should this be `depends on !SMP || !MMU` with default yes? Because with SMP && MMU, it shouldn't be an option to run with percpu-km. > config CLEANCACHE > bool "Enable cleancache driver to cache clean pages if tmem is present" > -- > 2.7.4 > It's interesting to me that this is all coming up at once. Earlier this month I had the same conversation with people involved with sh [1]. [1] https://lore.kernel.org/linux-sh/YY7tp5attRyK42Zk@fedora/ I can pull this shortly once I see whatever happened to linux-sh. Thanks, Dennis
Hi, On 11/30/21 5:41 PM, Dennis Zhou wrote: > Hello, > > On Tue, Nov 30, 2021 at 05:29:54PM +0000, Vladimir Murzin wrote: >> Currently, NOMMU pull km allocator via !SMP dependency because most of >> them are UP, yet for SMP+NOMMU vm allocator gets pulled which: >> >> * may lead to broken build [1] >> * ...or not working runtime due to [2] >> >> It looks like SMP+NOMMU case was overlooked in bbddff054587 ("percpu: >> use percpu allocator on UP too") so restore that. >> >> [1] >> For ARM SMP+NOMMU (R-class cores) >> >> arm-none-linux-gnueabihf-ld: mm/percpu.o: in function `pcpu_post_unmap_tlb_flush': >> mm/percpu-vm.c:188: undefined reference to `flush_tlb_kernel_range' >> >> [2] >> static inline >> int vmap_pages_range_noflush(unsigned long addr, unsigned long end, >> pgprot_t prot, struct page **pages, unsigned int page_shift) >> { >> return -EINVAL; >> } >> >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> >> --- >> mm/Kconfig | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/mm/Kconfig b/mm/Kconfig >> index d16ba92..66331e0 100644 >> --- a/mm/Kconfig >> +++ b/mm/Kconfig >> @@ -425,9 +425,8 @@ config THP_SWAP >> # UP and nommu archs use km based percpu allocator >> # >> config NEED_PER_CPU_KM >> - depends on !SMP >> bool >> - default y >> + default !SMP || !MMU >> > > Should this be `depends on !SMP || !MMU` with default yes? Because with > SMP && MMU, it shouldn't be an option to run with percpu-km. IIUC these are equivalent, truth table would not change if is under "depends" or "default" SMP MMU NEED_PER_CPU_KM y y !y || !y => n || n => n y n !y || !n => n || y => y n y !n || !y => y || n => y n n !n || !n => y || y => y > >> config CLEANCACHE >> bool "Enable cleancache driver to cache clean pages if tmem is present" >> -- >> 2.7.4 >> > > It's interesting to me that this is all coming up at once. Earlier this > month I had the same conversation with people involved with sh [1]. > > [1] https://lore.kernel.org/linux-sh/YY7tp5attRyK42Zk@fedora/ > > I can pull this shortly once I see whatever happened to linux-sh. Ahh, good to know! Adding SH folks here (start of discussion [0]). I see you came to the same conclusion, right? IIRC, RISC-V also have SMP+NOMMU, so adding them as well. [0] https://lore.kernel.org/linux-mm/20211130172954.129587-1-vladimir.murzin@arm.com/T/ Cheers Vladimir > > Thanks, > Dennis >
On Wed, Dec 01, 2021 at 11:51:04AM +0000, Vladimir Murzin wrote: > Hi, > > On 11/30/21 5:41 PM, Dennis Zhou wrote: > > Hello, > > > > On Tue, Nov 30, 2021 at 05:29:54PM +0000, Vladimir Murzin wrote: > >> Currently, NOMMU pull km allocator via !SMP dependency because most of > >> them are UP, yet for SMP+NOMMU vm allocator gets pulled which: > >> > >> * may lead to broken build [1] > >> * ...or not working runtime due to [2] > >> > >> It looks like SMP+NOMMU case was overlooked in bbddff054587 ("percpu: > >> use percpu allocator on UP too") so restore that. > >> > >> [1] > >> For ARM SMP+NOMMU (R-class cores) > >> > >> arm-none-linux-gnueabihf-ld: mm/percpu.o: in function `pcpu_post_unmap_tlb_flush': > >> mm/percpu-vm.c:188: undefined reference to `flush_tlb_kernel_range' > >> > >> [2] > >> static inline > >> int vmap_pages_range_noflush(unsigned long addr, unsigned long end, > >> pgprot_t prot, struct page **pages, unsigned int page_shift) > >> { > >> return -EINVAL; > >> } > >> > >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > >> --- > >> mm/Kconfig | 3 +-- > >> 1 file changed, 1 insertion(+), 2 deletions(-) > >> > >> diff --git a/mm/Kconfig b/mm/Kconfig > >> index d16ba92..66331e0 100644 > >> --- a/mm/Kconfig > >> +++ b/mm/Kconfig > >> @@ -425,9 +425,8 @@ config THP_SWAP > >> # UP and nommu archs use km based percpu allocator > >> # > >> config NEED_PER_CPU_KM > >> - depends on !SMP > >> bool > >> - default y > >> + default !SMP || !MMU > >> > > > > Should this be `depends on !SMP || !MMU` with default yes? Because with > > SMP && MMU, it shouldn't be an option to run with percpu-km. > > IIUC these are equivalent, truth table would not change if is under "depends" > or "default" > > SMP MMU NEED_PER_CPU_KM > y y !y || !y => n || n => n > y n !y || !n => n || y => y > n y !n || !y => y || n => y > n n !n || !n => y || y => y > I may be wrong, but I think this is slightly different as we're using #ifdef / #if defined(). > > > >> config CLEANCACHE > >> bool "Enable cleancache driver to cache clean pages if tmem is present" > >> -- > >> 2.7.4 > >> > > > > It's interesting to me that this is all coming up at once. Earlier this > > month I had the same conversation with people involved with sh [1]. > > > > [1] https://lore.kernel.org/linux-sh/YY7tp5attRyK42Zk@fedora/ > > > > I can pull this shortly once I see whatever happened to linux-sh. > > Ahh, good to know! Adding SH folks here (start of discussion [0]). I see you came > to the same conclusion, right? > Yeah, I don't see anything else from linux-sh. So I'll go ahead and apply this with my change if you're fine with that. > IIRC, RISC-V also have SMP+NOMMU, so adding them as well. > > [0] https://lore.kernel.org/linux-mm/20211130172954.129587-1-vladimir.murzin@arm.com/T/ > > Cheers > Vladimir > Thanks, Dennis
On 12/3/21 9:02 PM, Dennis Zhou wrote: > On Wed, Dec 01, 2021 at 11:51:04AM +0000, Vladimir Murzin wrote: >> Hi, >> >> On 11/30/21 5:41 PM, Dennis Zhou wrote: >>> Hello, >>> >>> On Tue, Nov 30, 2021 at 05:29:54PM +0000, Vladimir Murzin wrote: >>>> Currently, NOMMU pull km allocator via !SMP dependency because most of >>>> them are UP, yet for SMP+NOMMU vm allocator gets pulled which: >>>> >>>> * may lead to broken build [1] >>>> * ...or not working runtime due to [2] >>>> >>>> It looks like SMP+NOMMU case was overlooked in bbddff054587 ("percpu: >>>> use percpu allocator on UP too") so restore that. >>>> >>>> [1] >>>> For ARM SMP+NOMMU (R-class cores) >>>> >>>> arm-none-linux-gnueabihf-ld: mm/percpu.o: in function `pcpu_post_unmap_tlb_flush': >>>> mm/percpu-vm.c:188: undefined reference to `flush_tlb_kernel_range' >>>> >>>> [2] >>>> static inline >>>> int vmap_pages_range_noflush(unsigned long addr, unsigned long end, >>>> pgprot_t prot, struct page **pages, unsigned int page_shift) >>>> { >>>> return -EINVAL; >>>> } >>>> >>>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> >>>> --- >>>> mm/Kconfig | 3 +-- >>>> 1 file changed, 1 insertion(+), 2 deletions(-) >>>> >>>> diff --git a/mm/Kconfig b/mm/Kconfig >>>> index d16ba92..66331e0 100644 >>>> --- a/mm/Kconfig >>>> +++ b/mm/Kconfig >>>> @@ -425,9 +425,8 @@ config THP_SWAP >>>> # UP and nommu archs use km based percpu allocator >>>> # >>>> config NEED_PER_CPU_KM >>>> - depends on !SMP >>>> bool >>>> - default y >>>> + default !SMP || !MMU >>>> >>> >>> Should this be `depends on !SMP || !MMU` with default yes? Because with >>> SMP && MMU, it shouldn't be an option to run with percpu-km. >> >> IIUC these are equivalent, truth table would not change if is under "depends" >> or "default" >> >> SMP MMU NEED_PER_CPU_KM >> y y !y || !y => n || n => n >> y n !y || !n => n || y => y >> n y !n || !y => y || n => y >> n n !n || !n => y || y => y >> > > I may be wrong, but I think this is slightly different as we're using > #ifdef / #if defined(). > >>> >>>> config CLEANCACHE >>>> bool "Enable cleancache driver to cache clean pages if tmem is present" >>>> -- >>>> 2.7.4 >>>> >>> >>> It's interesting to me that this is all coming up at once. Earlier this >>> month I had the same conversation with people involved with sh [1]. >>> >>> [1] https://lore.kernel.org/linux-sh/YY7tp5attRyK42Zk@fedora/ >>> >>> I can pull this shortly once I see whatever happened to linux-sh. >> >> Ahh, good to know! Adding SH folks here (start of discussion [0]). I see you came >> to the same conclusion, right? >> > > Yeah, I don't see anything else from linux-sh. So I'll go ahead and > apply this with my change if you're fine with that. depends on !SMP || !MMU, also works for me, so feel free to amend. Thanks Vladimir > >> IIRC, RISC-V also have SMP+NOMMU, so adding them as well. >> >> [0] https://lore.kernel.org/linux-mm/20211130172954.129587-1-vladimir.murzin@arm.com/T/ >> >> Cheers >> Vladimir >> > > Thanks, > Dennis >
On 12/3/21 3:02 PM, Dennis Zhou wrote: > On Wed, Dec 01, 2021 at 11:51:04AM +0000, Vladimir Murzin wrote: >> Hi, >> >> On 11/30/21 5:41 PM, Dennis Zhou wrote: >> > Hello, >> > >> > On Tue, Nov 30, 2021 at 05:29:54PM +0000, Vladimir Murzin wrote: >> >> Currently, NOMMU pull km allocator via !SMP dependency because most of >> >> them are UP, yet for SMP+NOMMU vm allocator gets pulled which: >> >> >> >> * may lead to broken build [1] >> >> * ...or not working runtime due to [2] >> >> >> >> It looks like SMP+NOMMU case was overlooked in bbddff054587 ("percpu: >> >> use percpu allocator on UP too") so restore that. >> >> >> >> [1] >> >> For ARM SMP+NOMMU (R-class cores) >> >> >> >> arm-none-linux-gnueabihf-ld: mm/percpu.o: in function `pcpu_post_unmap_tlb_flush': >> >> mm/percpu-vm.c:188: undefined reference to `flush_tlb_kernel_range' >> >> >> >> [2] >> >> static inline >> >> int vmap_pages_range_noflush(unsigned long addr, unsigned long end, >> >> pgprot_t prot, struct page **pages, unsigned int page_shift) >> >> { >> >> return -EINVAL; >> >> } >> >> >> >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> >> >> --- >> >> mm/Kconfig | 3 +-- >> >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> >> >> diff --git a/mm/Kconfig b/mm/Kconfig >> >> index d16ba92..66331e0 100644 >> >> --- a/mm/Kconfig >> >> +++ b/mm/Kconfig >> >> @@ -425,9 +425,8 @@ config THP_SWAP >> >> # UP and nommu archs use km based percpu allocator >> >> # >> >> config NEED_PER_CPU_KM >> >> - depends on !SMP >> >> bool >> >> - default y >> >> + default !SMP || !MMU >> >> >> > >> > Should this be `depends on !SMP || !MMU` with default yes? Because with >> > SMP && MMU, it shouldn't be an option to run with percpu-km. >> >> IIUC these are equivalent, truth table would not change if is under "depends" >> or "default" >> >> SMP MMU NEED_PER_CPU_KM >> y y !y || !y => n || n => n >> y n !y || !n => n || y => y >> n y !n || !y => y || n => y >> n n !n || !n => y || y => y >> > > I may be wrong, but I think this is slightly different as we're using > #ifdef / #if defined(). > >> > >> >> config CLEANCACHE >> >> bool "Enable cleancache driver to cache clean pages if tmem is present" >> >> -- >> >> 2.7.4 >> >> >> > >> > It's interesting to me that this is all coming up at once. Earlier this >> > month I had the same conversation with people involved with sh [1]. >> > >> > [1] https://lore.kernel.org/linux-sh/YY7tp5attRyK42Zk@fedora/ >> > >> > I can pull this shortly once I see whatever happened to linux-sh. >> >> Ahh, good to know! Adding SH folks here (start of discussion [0]). I see you came >> to the same conclusion, right? > > Yeah, I don't see anything else from linux-sh. So I'll go ahead and > apply this with my change if you're fine with that. I can't test against current until I get some unrelated fixes from Rich Felker (who's been busy over the weekend), but I tested the "depends" version on 5.10 and got a shell prompt on my "make ARCH=sh j2_defconfig" board. Tested-by: Rob Landley <rob@landley.net> Rob
On Mon, Dec 06, 2021 at 06:01:59AM -0600, Rob Landley wrote: > On 12/3/21 3:02 PM, Dennis Zhou wrote: > > On Wed, Dec 01, 2021 at 11:51:04AM +0000, Vladimir Murzin wrote: > >> Hi, > >> > >> On 11/30/21 5:41 PM, Dennis Zhou wrote: > >> > Hello, > >> > > >> > On Tue, Nov 30, 2021 at 05:29:54PM +0000, Vladimir Murzin wrote: > >> >> Currently, NOMMU pull km allocator via !SMP dependency because most of > >> >> them are UP, yet for SMP+NOMMU vm allocator gets pulled which: > >> >> > >> >> * may lead to broken build [1] > >> >> * ...or not working runtime due to [2] > >> >> > >> >> It looks like SMP+NOMMU case was overlooked in bbddff054587 ("percpu: > >> >> use percpu allocator on UP too") so restore that. > >> >> > >> >> [1] > >> >> For ARM SMP+NOMMU (R-class cores) > >> >> > >> >> arm-none-linux-gnueabihf-ld: mm/percpu.o: in function `pcpu_post_unmap_tlb_flush': > >> >> mm/percpu-vm.c:188: undefined reference to `flush_tlb_kernel_range' > >> >> > >> >> [2] > >> >> static inline > >> >> int vmap_pages_range_noflush(unsigned long addr, unsigned long end, > >> >> pgprot_t prot, struct page **pages, unsigned int page_shift) > >> >> { > >> >> return -EINVAL; > >> >> } > >> >> > >> >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > >> >> --- > >> >> mm/Kconfig | 3 +-- > >> >> 1 file changed, 1 insertion(+), 2 deletions(-) > >> >> > >> >> diff --git a/mm/Kconfig b/mm/Kconfig > >> >> index d16ba92..66331e0 100644 > >> >> --- a/mm/Kconfig > >> >> +++ b/mm/Kconfig > >> >> @@ -425,9 +425,8 @@ config THP_SWAP > >> >> # UP and nommu archs use km based percpu allocator > >> >> # > >> >> config NEED_PER_CPU_KM > >> >> - depends on !SMP > >> >> bool > >> >> - default y > >> >> + default !SMP || !MMU > >> >> > >> > > >> > Should this be `depends on !SMP || !MMU` with default yes? Because with > >> > SMP && MMU, it shouldn't be an option to run with percpu-km. > >> > >> IIUC these are equivalent, truth table would not change if is under "depends" > >> or "default" > >> > >> SMP MMU NEED_PER_CPU_KM > >> y y !y || !y => n || n => n > >> y n !y || !n => n || y => y > >> n y !n || !y => y || n => y > >> n n !n || !n => y || y => y > >> > > > > I may be wrong, but I think this is slightly different as we're using > > #ifdef / #if defined(). > > > >> > > >> >> config CLEANCACHE > >> >> bool "Enable cleancache driver to cache clean pages if tmem is present" > >> >> -- > >> >> 2.7.4 > >> >> > >> > > >> > It's interesting to me that this is all coming up at once. Earlier this > >> > month I had the same conversation with people involved with sh [1]. > >> > > >> > [1] https://lore.kernel.org/linux-sh/YY7tp5attRyK42Zk@fedora/ > >> > > >> > I can pull this shortly once I see whatever happened to linux-sh. > >> > >> Ahh, good to know! Adding SH folks here (start of discussion [0]). I see you came > >> to the same conclusion, right? > > > > Yeah, I don't see anything else from linux-sh. So I'll go ahead and > > apply this with my change if you're fine with that. > > I can't test against current until I get some unrelated fixes from Rich Felker > (who's been busy over the weekend), but I tested the "depends" version on 5.10 > and got a shell prompt on my "make ARCH=sh j2_defconfig" board. > > Tested-by: Rob Landley <rob@landley.net> Tested-by: Rich Felker <dalias@libc.org> I've tested the version as originally posted and it works for j2 (sh2) (both build and runtime checked). I'd tested the other (depends version) before and I'm fine with either going upstream. Rich
On Mon, Dec 06, 2021 at 11:21:46AM -0500, Rich Felker wrote: > On Mon, Dec 06, 2021 at 06:01:59AM -0600, Rob Landley wrote: > > On 12/3/21 3:02 PM, Dennis Zhou wrote: > > > On Wed, Dec 01, 2021 at 11:51:04AM +0000, Vladimir Murzin wrote: > > >> Hi, > > >> > > >> On 11/30/21 5:41 PM, Dennis Zhou wrote: > > >> > Hello, > > >> > > > >> > On Tue, Nov 30, 2021 at 05:29:54PM +0000, Vladimir Murzin wrote: > > >> >> Currently, NOMMU pull km allocator via !SMP dependency because most of > > >> >> them are UP, yet for SMP+NOMMU vm allocator gets pulled which: > > >> >> > > >> >> * may lead to broken build [1] > > >> >> * ...or not working runtime due to [2] > > >> >> > > >> >> It looks like SMP+NOMMU case was overlooked in bbddff054587 ("percpu: > > >> >> use percpu allocator on UP too") so restore that. > > >> >> > > >> >> [1] > > >> >> For ARM SMP+NOMMU (R-class cores) > > >> >> > > >> >> arm-none-linux-gnueabihf-ld: mm/percpu.o: in function `pcpu_post_unmap_tlb_flush': > > >> >> mm/percpu-vm.c:188: undefined reference to `flush_tlb_kernel_range' > > >> >> > > >> >> [2] > > >> >> static inline > > >> >> int vmap_pages_range_noflush(unsigned long addr, unsigned long end, > > >> >> pgprot_t prot, struct page **pages, unsigned int page_shift) > > >> >> { > > >> >> return -EINVAL; > > >> >> } > > >> >> > > >> >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > > >> >> --- > > >> >> mm/Kconfig | 3 +-- > > >> >> 1 file changed, 1 insertion(+), 2 deletions(-) > > >> >> > > >> >> diff --git a/mm/Kconfig b/mm/Kconfig > > >> >> index d16ba92..66331e0 100644 > > >> >> --- a/mm/Kconfig > > >> >> +++ b/mm/Kconfig > > >> >> @@ -425,9 +425,8 @@ config THP_SWAP > > >> >> # UP and nommu archs use km based percpu allocator > > >> >> # > > >> >> config NEED_PER_CPU_KM > > >> >> - depends on !SMP > > >> >> bool > > >> >> - default y > > >> >> + default !SMP || !MMU > > >> >> > > >> > > > >> > Should this be `depends on !SMP || !MMU` with default yes? Because with > > >> > SMP && MMU, it shouldn't be an option to run with percpu-km. > > >> > > >> IIUC these are equivalent, truth table would not change if is under "depends" > > >> or "default" > > >> > > >> SMP MMU NEED_PER_CPU_KM > > >> y y !y || !y => n || n => n > > >> y n !y || !n => n || y => y > > >> n y !n || !y => y || n => y > > >> n n !n || !n => y || y => y > > >> > > > > > > I may be wrong, but I think this is slightly different as we're using > > > #ifdef / #if defined(). > > > > > >> > > > >> >> config CLEANCACHE > > >> >> bool "Enable cleancache driver to cache clean pages if tmem is present" > > >> >> -- > > >> >> 2.7.4 > > >> >> > > >> > > > >> > It's interesting to me that this is all coming up at once. Earlier this > > >> > month I had the same conversation with people involved with sh [1]. > > >> > > > >> > [1] https://lore.kernel.org/linux-sh/YY7tp5attRyK42Zk@fedora/ > > >> > > > >> > I can pull this shortly once I see whatever happened to linux-sh. > > >> > > >> Ahh, good to know! Adding SH folks here (start of discussion [0]). I see you came > > >> to the same conclusion, right? > > > > > > Yeah, I don't see anything else from linux-sh. So I'll go ahead and > > > apply this with my change if you're fine with that. > > > > I can't test against current until I get some unrelated fixes from Rich Felker > > (who's been busy over the weekend), but I tested the "depends" version on 5.10 > > and got a shell prompt on my "make ARCH=sh j2_defconfig" board. > > > > Tested-by: Rob Landley <rob@landley.net> > > Tested-by: Rich Felker <dalias@libc.org> > > I've tested the version as originally posted and it works for j2 (sh2) > (both build and runtime checked). I'd tested the other (depends > version) before and I'm fine with either going upstream. > > Rich > Thank you all for testing and bearing with me. I've applied this to for-5.16-fixes. Will send it up later in the week. Thanks, Dennis
Hi Vladimir and Dennis, On Wed, Dec 1, 2021 at 12:53 PM Vladimir Murzin <vladimir.murzin@arm.com> wrote: > On 11/30/21 5:41 PM, Dennis Zhou wrote: > > On Tue, Nov 30, 2021 at 05:29:54PM +0000, Vladimir Murzin wrote: > >> Currently, NOMMU pull km allocator via !SMP dependency because most of > >> them are UP, yet for SMP+NOMMU vm allocator gets pulled which: > >> > >> * may lead to broken build [1] > >> * ...or not working runtime due to [2] > >> > >> It looks like SMP+NOMMU case was overlooked in bbddff054587 ("percpu: > >> use percpu allocator on UP too") so restore that. > >> > >> [1] > >> For ARM SMP+NOMMU (R-class cores) > >> > >> arm-none-linux-gnueabihf-ld: mm/percpu.o: in function `pcpu_post_unmap_tlb_flush': > >> mm/percpu-vm.c:188: undefined reference to `flush_tlb_kernel_range' > >> > >> [2] > >> static inline > >> int vmap_pages_range_noflush(unsigned long addr, unsigned long end, > >> pgprot_t prot, struct page **pages, unsigned int page_shift) > >> { > >> return -EINVAL; > >> } > >> > >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > >> --- > >> mm/Kconfig | 3 +-- > >> 1 file changed, 1 insertion(+), 2 deletions(-) > >> > >> diff --git a/mm/Kconfig b/mm/Kconfig > >> index d16ba92..66331e0 100644 > >> --- a/mm/Kconfig > >> +++ b/mm/Kconfig > >> @@ -425,9 +425,8 @@ config THP_SWAP > >> # UP and nommu archs use km based percpu allocator > >> # > >> config NEED_PER_CPU_KM > >> - depends on !SMP > >> bool > >> - default y > >> + default !SMP || !MMU > >> > > > > Should this be `depends on !SMP || !MMU` with default yes? Because with > > SMP && MMU, it shouldn't be an option to run with percpu-km. > > IIUC these are equivalent, truth table would not change if is under "depends" > or "default" > > SMP MMU NEED_PER_CPU_KM > y y !y || !y => n || n => n > y n !y || !n => n || y => y > n y !n || !y => y || n => y > n n !n || !n => y || y => y > > > > >> config CLEANCACHE > >> bool "Enable cleancache driver to cache clean pages if tmem is present" > >> -- > >> 2.7.4 > >> > > > > It's interesting to me that this is all coming up at once. Earlier this > > month I had the same conversation with people involved with sh [1]. > > > > [1] https://lore.kernel.org/linux-sh/YY7tp5attRyK42Zk@fedora/ > > > > I can pull this shortly once I see whatever happened to linux-sh. > > Ahh, good to know! Adding SH folks here (start of discussion [0]). I see you came > to the same conclusion, right? > > IIRC, RISC-V also have SMP+NOMMU, so adding them as well. I had seen the j-Core thread, but completely forgot about Canaan K210 (RV64 SMP+NOMMU). This became commit 3583521aabac76e5 ("percpu: km: ensure it is used with NOMMU (either UP or SMP)"). And now booting K210 prints: percpu: wasting 10 pages per chunk a) Is this bad? b) What exactly was this fixing, and how would I trigger the bad case on K210 before, if it was affected at all? > > [0] https://lore.kernel.org/linux-mm/20211130172954.129587-1-vladimir.murzin@arm.com/T/ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hello, On Tue, Dec 14, 2021 at 05:29:22PM +0100, Geert Uytterhoeven wrote: > Hi Vladimir and Dennis, > > On Wed, Dec 1, 2021 at 12:53 PM Vladimir Murzin <vladimir.murzin@arm.com> wrote: > > On 11/30/21 5:41 PM, Dennis Zhou wrote: > > > On Tue, Nov 30, 2021 at 05:29:54PM +0000, Vladimir Murzin wrote: > > >> Currently, NOMMU pull km allocator via !SMP dependency because most of > > >> them are UP, yet for SMP+NOMMU vm allocator gets pulled which: > > >> > > >> * may lead to broken build [1] > > >> * ...or not working runtime due to [2] > > >> > > >> It looks like SMP+NOMMU case was overlooked in bbddff054587 ("percpu: > > >> use percpu allocator on UP too") so restore that. > > >> > > >> [1] > > >> For ARM SMP+NOMMU (R-class cores) > > >> > > >> arm-none-linux-gnueabihf-ld: mm/percpu.o: in function `pcpu_post_unmap_tlb_flush': > > >> mm/percpu-vm.c:188: undefined reference to `flush_tlb_kernel_range' > > >> > > >> [2] > > >> static inline > > >> int vmap_pages_range_noflush(unsigned long addr, unsigned long end, > > >> pgprot_t prot, struct page **pages, unsigned int page_shift) > > >> { > > >> return -EINVAL; > > >> } > > >> > > >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > > >> --- > > >> mm/Kconfig | 3 +-- > > >> 1 file changed, 1 insertion(+), 2 deletions(-) > > >> > > >> diff --git a/mm/Kconfig b/mm/Kconfig > > >> index d16ba92..66331e0 100644 > > >> --- a/mm/Kconfig > > >> +++ b/mm/Kconfig > > >> @@ -425,9 +425,8 @@ config THP_SWAP > > >> # UP and nommu archs use km based percpu allocator > > >> # > > >> config NEED_PER_CPU_KM > > >> - depends on !SMP > > >> bool > > >> - default y > > >> + default !SMP || !MMU > > >> > > > > > > Should this be `depends on !SMP || !MMU` with default yes? Because with > > > SMP && MMU, it shouldn't be an option to run with percpu-km. > > > > IIUC these are equivalent, truth table would not change if is under "depends" > > or "default" > > > > SMP MMU NEED_PER_CPU_KM > > y y !y || !y => n || n => n > > y n !y || !n => n || y => y > > n y !n || !y => y || n => y > > n n !n || !n => y || y => y > > > > > > > >> config CLEANCACHE > > >> bool "Enable cleancache driver to cache clean pages if tmem is present" > > >> -- > > >> 2.7.4 > > >> > > > > > > It's interesting to me that this is all coming up at once. Earlier this > > > month I had the same conversation with people involved with sh [1]. > > > > > > [1] https://lore.kernel.org/linux-sh/YY7tp5attRyK42Zk@fedora/ > > > > > > I can pull this shortly once I see whatever happened to linux-sh. > > > > Ahh, good to know! Adding SH folks here (start of discussion [0]). I see you came > > to the same conclusion, right? > > > > IIRC, RISC-V also have SMP+NOMMU, so adding them as well. > > I had seen the j-Core thread, but completely forgot about > Canaan K210 (RV64 SMP+NOMMU). > > This became commit 3583521aabac76e5 ("percpu: km: ensure it is used > with NOMMU (either UP or SMP)"). And now booting K210 prints: > > percpu: wasting 10 pages per chunk > > a) Is this bad? It's not great.. Can you share the line on boot with the following prefix: pcpu-alloc [1]. I'm a little surprised here because this means it's allocating not against the right atomic size. I don't necesarily think it's an issue of switching from percpu-vm to percpu-km. > b) What exactly was this fixing, and how would I trigger the bad case > on K210 before, if it was affected at all? > In v5.14, I merged Roman's request for percpu depopulation [2]. This required calls to flush the tlb. There is an abstraction layer: percpu-vm vs percpu-km. So if an architecture is using percpu-vm but doesn't have an MMU AND doesn't map out appropriately the tlb flush call, then it fails. This happened on arm + sh architectures. Now RV might be mapping it out appropriately so they never saw the issue. Now, there is also a bigger caveat with using percpu-vm without an MMU. In percpu-vm, we allocate pages on demand and map them in into pre-allocated vmas. This means there are 2 scenarios that I haven't looked into deeper. 1, the vma alloc maps to allocating physical pages. 2, we're lucky percpu allocates backwards in the vma so we haven't had a collision problem yet. [1] https://git.kernel.org/pub/scm/linux/kernel/git/dennis/percpu.git/tree/mm/percpu.c#n2507 [2] https://lore.kernel.org/lkml/20210419225047.3415425-1-dennis@kernel.org/ Thanks, Dennis > > > > [0] https://lore.kernel.org/linux-mm/20211130172954.129587-1-vladimir.murzin@arm.com/T/ > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
Hi Dennis, On Tue, Dec 14, 2021 at 6:26 PM Dennis Zhou <dennis@kernel.org> wrote: > On Tue, Dec 14, 2021 at 05:29:22PM +0100, Geert Uytterhoeven wrote: > > On Wed, Dec 1, 2021 at 12:53 PM Vladimir Murzin <vladimir.murzin@arm.com> wrote: > > > On 11/30/21 5:41 PM, Dennis Zhou wrote: > > > > On Tue, Nov 30, 2021 at 05:29:54PM +0000, Vladimir Murzin wrote: > > > >> Currently, NOMMU pull km allocator via !SMP dependency because most of > > > >> them are UP, yet for SMP+NOMMU vm allocator gets pulled which: > > > >> > > > >> * may lead to broken build [1] > > > >> * ...or not working runtime due to [2] > > > >> > > > >> It looks like SMP+NOMMU case was overlooked in bbddff054587 ("percpu: > > > >> use percpu allocator on UP too") so restore that. > > > >> > > > >> [1] > > > >> For ARM SMP+NOMMU (R-class cores) > > > >> > > > >> arm-none-linux-gnueabihf-ld: mm/percpu.o: in function `pcpu_post_unmap_tlb_flush': > > > >> mm/percpu-vm.c:188: undefined reference to `flush_tlb_kernel_range' > > > >> > > > >> [2] > > > >> static inline > > > >> int vmap_pages_range_noflush(unsigned long addr, unsigned long end, > > > >> pgprot_t prot, struct page **pages, unsigned int page_shift) > > > >> { > > > >> return -EINVAL; > > > >> } > > > >> > > > >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > > > >> --- > > > >> mm/Kconfig | 3 +-- > > > >> 1 file changed, 1 insertion(+), 2 deletions(-) > > > >> > > > >> diff --git a/mm/Kconfig b/mm/Kconfig > > > >> index d16ba92..66331e0 100644 > > > >> --- a/mm/Kconfig > > > >> +++ b/mm/Kconfig > > > >> @@ -425,9 +425,8 @@ config THP_SWAP > > > >> # UP and nommu archs use km based percpu allocator > > > >> # > > > >> config NEED_PER_CPU_KM > > > >> - depends on !SMP > > > >> bool > > > >> - default y > > > >> + default !SMP || !MMU > > > >> > > > > > > > > Should this be `depends on !SMP || !MMU` with default yes? Because with > > > > SMP && MMU, it shouldn't be an option to run with percpu-km. > > > > > > IIUC these are equivalent, truth table would not change if is under "depends" > > > or "default" > > > > > > SMP MMU NEED_PER_CPU_KM > > > y y !y || !y => n || n => n > > > y n !y || !n => n || y => y > > > n y !n || !y => y || n => y > > > n n !n || !n => y || y => y > > > > > > > > > > >> config CLEANCACHE > > > >> bool "Enable cleancache driver to cache clean pages if tmem is present" > > > >> -- > > > >> 2.7.4 > > > >> > > > > > > > > It's interesting to me that this is all coming up at once. Earlier this > > > > month I had the same conversation with people involved with sh [1]. > > > > > > > > [1] https://lore.kernel.org/linux-sh/YY7tp5attRyK42Zk@fedora/ > > > > > > > > I can pull this shortly once I see whatever happened to linux-sh. > > > > > > Ahh, good to know! Adding SH folks here (start of discussion [0]). I see you came > > > to the same conclusion, right? > > > > > > IIRC, RISC-V also have SMP+NOMMU, so adding them as well. > > > > I had seen the j-Core thread, but completely forgot about > > Canaan K210 (RV64 SMP+NOMMU). > > > > This became commit 3583521aabac76e5 ("percpu: km: ensure it is used > > with NOMMU (either UP or SMP)"). And now booting K210 prints: > > > > percpu: wasting 10 pages per chunk > > > > a) Is this bad? > > It's not great.. Can you share the line on boot with the following > prefix: pcpu-alloc [1]. There are no such lines. "make mm/percpu.i mm/percpu.s" and inspecting the generated files, and vmlinux, proves the code is there. But apparently it's not called. So there may be no issue on my system? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Tue, Dec 14, 2021 at 08:02:58PM +0100, Geert Uytterhoeven wrote: > Hi Dennis, > > On Tue, Dec 14, 2021 at 6:26 PM Dennis Zhou <dennis@kernel.org> wrote: > > On Tue, Dec 14, 2021 at 05:29:22PM +0100, Geert Uytterhoeven wrote: > > > On Wed, Dec 1, 2021 at 12:53 PM Vladimir Murzin <vladimir.murzin@arm.com> wrote: > > > > On 11/30/21 5:41 PM, Dennis Zhou wrote: > > > > > On Tue, Nov 30, 2021 at 05:29:54PM +0000, Vladimir Murzin wrote: > > > > >> Currently, NOMMU pull km allocator via !SMP dependency because most of > > > > >> them are UP, yet for SMP+NOMMU vm allocator gets pulled which: > > > > >> > > > > >> * may lead to broken build [1] > > > > >> * ...or not working runtime due to [2] > > > > >> > > > > >> It looks like SMP+NOMMU case was overlooked in bbddff054587 ("percpu: > > > > >> use percpu allocator on UP too") so restore that. > > > > >> > > > > >> [1] > > > > >> For ARM SMP+NOMMU (R-class cores) > > > > >> > > > > >> arm-none-linux-gnueabihf-ld: mm/percpu.o: in function `pcpu_post_unmap_tlb_flush': > > > > >> mm/percpu-vm.c:188: undefined reference to `flush_tlb_kernel_range' > > > > >> > > > > >> [2] > > > > >> static inline > > > > >> int vmap_pages_range_noflush(unsigned long addr, unsigned long end, > > > > >> pgprot_t prot, struct page **pages, unsigned int page_shift) > > > > >> { > > > > >> return -EINVAL; > > > > >> } > > > > >> > > > > >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > > > > >> --- > > > > >> mm/Kconfig | 3 +-- > > > > >> 1 file changed, 1 insertion(+), 2 deletions(-) > > > > >> > > > > >> diff --git a/mm/Kconfig b/mm/Kconfig > > > > >> index d16ba92..66331e0 100644 > > > > >> --- a/mm/Kconfig > > > > >> +++ b/mm/Kconfig > > > > >> @@ -425,9 +425,8 @@ config THP_SWAP > > > > >> # UP and nommu archs use km based percpu allocator > > > > >> # > > > > >> config NEED_PER_CPU_KM > > > > >> - depends on !SMP > > > > >> bool > > > > >> - default y > > > > >> + default !SMP || !MMU > > > > >> > > > > > > > > > > Should this be `depends on !SMP || !MMU` with default yes? Because with > > > > > SMP && MMU, it shouldn't be an option to run with percpu-km. > > > > > > > > IIUC these are equivalent, truth table would not change if is under "depends" > > > > or "default" > > > > > > > > SMP MMU NEED_PER_CPU_KM > > > > y y !y || !y => n || n => n > > > > y n !y || !n => n || y => y > > > > n y !n || !y => y || n => y > > > > n n !n || !n => y || y => y > > > > > > > > > > > > > >> config CLEANCACHE > > > > >> bool "Enable cleancache driver to cache clean pages if tmem is present" > > > > >> -- > > > > >> 2.7.4 > > > > >> > > > > > > > > > > It's interesting to me that this is all coming up at once. Earlier this > > > > > month I had the same conversation with people involved with sh [1]. > > > > > > > > > > [1] https://lore.kernel.org/linux-sh/YY7tp5attRyK42Zk@fedora/ > > > > > > > > > > I can pull this shortly once I see whatever happened to linux-sh. > > > > > > > > Ahh, good to know! Adding SH folks here (start of discussion [0]). I see you came > > > > to the same conclusion, right? > > > > > > > > IIRC, RISC-V also have SMP+NOMMU, so adding them as well. > > > > > > I had seen the j-Core thread, but completely forgot about > > > Canaan K210 (RV64 SMP+NOMMU). > > > > > > This became commit 3583521aabac76e5 ("percpu: km: ensure it is used > > > with NOMMU (either UP or SMP)"). And now booting K210 prints: > > > > > > percpu: wasting 10 pages per chunk > > > > > > a) Is this bad? > > > > It's not great.. Can you share the line on boot with the following > > prefix: pcpu-alloc [1]. > > There are no such lines. > "make mm/percpu.i mm/percpu.s" and inspecting the generated files, > and vmlinux, proves the code is there. But apparently it's not called. > > So there may be no issue on my system? > I might be missing something, but that can't be right. Percpu calls pcpu_dump_alloc_info() from pcpu_setup_first_chunk() which is called by both embed/page first chunk code. Ummm. That can't be right. Percpu call pcpu_dump_alloc_info() from pcpu_setup_first_chunk() which everyone should call. On my machine: $ dmesg | grep "pcpu-alloc" [ 0.065118] pcpu-alloc: s184320 r8192 d28672 u262144 alloc=1*2097152 Thanks, Dennis
Hi Dennis, On Tue, Dec 14, 2021 at 8:18 PM Dennis Zhou <dennis@kernel.org> wrote: > On Tue, Dec 14, 2021 at 08:02:58PM +0100, Geert Uytterhoeven wrote: > > On Tue, Dec 14, 2021 at 6:26 PM Dennis Zhou <dennis@kernel.org> wrote: > > > On Tue, Dec 14, 2021 at 05:29:22PM +0100, Geert Uytterhoeven wrote: > > > > On Wed, Dec 1, 2021 at 12:53 PM Vladimir Murzin <vladimir.murzin@arm.com> wrote: > > > > > On 11/30/21 5:41 PM, Dennis Zhou wrote: > > > > > > On Tue, Nov 30, 2021 at 05:29:54PM +0000, Vladimir Murzin wrote: > > > > > >> Currently, NOMMU pull km allocator via !SMP dependency because most of > > > > > >> them are UP, yet for SMP+NOMMU vm allocator gets pulled which: > > > > > >> > > > > > >> * may lead to broken build [1] > > > > > >> * ...or not working runtime due to [2] > > > > > >> > > > > > >> It looks like SMP+NOMMU case was overlooked in bbddff054587 ("percpu: > > > > > >> use percpu allocator on UP too") so restore that. > > > > > >> > > > > > >> [1] > > > > > >> For ARM SMP+NOMMU (R-class cores) > > > > > >> > > > > > >> arm-none-linux-gnueabihf-ld: mm/percpu.o: in function `pcpu_post_unmap_tlb_flush': > > > > > >> mm/percpu-vm.c:188: undefined reference to `flush_tlb_kernel_range' > > > > > >> > > > > > >> [2] > > > > > >> static inline > > > > > >> int vmap_pages_range_noflush(unsigned long addr, unsigned long end, > > > > > >> pgprot_t prot, struct page **pages, unsigned int page_shift) > > > > > >> { > > > > > >> return -EINVAL; > > > > > >> } > > > > > >> > > > > > >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > > > > > IIRC, RISC-V also have SMP+NOMMU, so adding them as well. > > > > > > > > I had seen the j-Core thread, but completely forgot about > > > > Canaan K210 (RV64 SMP+NOMMU). > > > > > > > > This became commit 3583521aabac76e5 ("percpu: km: ensure it is used > > > > with NOMMU (either UP or SMP)"). And now booting K210 prints: > > > > > > > > percpu: wasting 10 pages per chunk > > > > > > > > a) Is this bad? > > > > > > It's not great.. Can you share the line on boot with the following > > > prefix: pcpu-alloc [1]. > > > > There are no such lines. > > "make mm/percpu.i mm/percpu.s" and inspecting the generated files, > > and vmlinux, proves the code is there. But apparently it's not called. > > > > So there may be no issue on my system? > > I might be missing something, but that can't be right. Percpu calls > pcpu_dump_alloc_info() from pcpu_setup_first_chunk() which is called by > both embed/page first chunk code. > > Ummm. That can't be right. Percpu call pcpu_dump_alloc_info() from > pcpu_setup_first_chunk() which everyone should call. On my machine: > > $ dmesg | grep "pcpu-alloc" > [ 0.065118] pcpu-alloc: s184320 r8192 d28672 u262144 alloc=1*2097152 Doh, it wasn't printed to the console, due to KERN_DEBUG. Dmesg does have it: <7>[ 0.000000] pcpu-alloc: s15520 r0 d29536 u45056 alloc=11*4096 <7>[ 0.000000] pcpu-alloc: [0] 0 [0] 1 Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Tue, Dec 14, 2021 at 09:12:06PM +0100, Geert Uytterhoeven wrote: > Hi Dennis, > > On Tue, Dec 14, 2021 at 8:18 PM Dennis Zhou <dennis@kernel.org> wrote: > > On Tue, Dec 14, 2021 at 08:02:58PM +0100, Geert Uytterhoeven wrote: > > > On Tue, Dec 14, 2021 at 6:26 PM Dennis Zhou <dennis@kernel.org> wrote: > > > > On Tue, Dec 14, 2021 at 05:29:22PM +0100, Geert Uytterhoeven wrote: > > > > > On Wed, Dec 1, 2021 at 12:53 PM Vladimir Murzin <vladimir.murzin@arm.com> wrote: > > > > > > On 11/30/21 5:41 PM, Dennis Zhou wrote: > > > > > > > On Tue, Nov 30, 2021 at 05:29:54PM +0000, Vladimir Murzin wrote: > > > > > > >> Currently, NOMMU pull km allocator via !SMP dependency because most of > > > > > > >> them are UP, yet for SMP+NOMMU vm allocator gets pulled which: > > > > > > >> > > > > > > >> * may lead to broken build [1] > > > > > > >> * ...or not working runtime due to [2] > > > > > > >> > > > > > > >> It looks like SMP+NOMMU case was overlooked in bbddff054587 ("percpu: > > > > > > >> use percpu allocator on UP too") so restore that. > > > > > > >> > > > > > > >> [1] > > > > > > >> For ARM SMP+NOMMU (R-class cores) > > > > > > >> > > > > > > >> arm-none-linux-gnueabihf-ld: mm/percpu.o: in function `pcpu_post_unmap_tlb_flush': > > > > > > >> mm/percpu-vm.c:188: undefined reference to `flush_tlb_kernel_range' > > > > > > >> > > > > > > >> [2] > > > > > > >> static inline > > > > > > >> int vmap_pages_range_noflush(unsigned long addr, unsigned long end, > > > > > > >> pgprot_t prot, struct page **pages, unsigned int page_shift) > > > > > > >> { > > > > > > >> return -EINVAL; > > > > > > >> } > > > > > > >> > > > > > > >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > > > > > > > IIRC, RISC-V also have SMP+NOMMU, so adding them as well. > > > > > > > > > > I had seen the j-Core thread, but completely forgot about > > > > > Canaan K210 (RV64 SMP+NOMMU). > > > > > > > > > > This became commit 3583521aabac76e5 ("percpu: km: ensure it is used > > > > > with NOMMU (either UP or SMP)"). And now booting K210 prints: > > > > > > > > > > percpu: wasting 10 pages per chunk > > > > > > > > > > a) Is this bad? > > > > > > > > It's not great.. Can you share the line on boot with the following > > > > prefix: pcpu-alloc [1]. > > > > > > There are no such lines. > > > "make mm/percpu.i mm/percpu.s" and inspecting the generated files, > > > and vmlinux, proves the code is there. But apparently it's not called. > > > > > > So there may be no issue on my system? > > > > I might be missing something, but that can't be right. Percpu calls > > pcpu_dump_alloc_info() from pcpu_setup_first_chunk() which is called by > > both embed/page first chunk code. > > > > Ummm. That can't be right. Percpu call pcpu_dump_alloc_info() from > > pcpu_setup_first_chunk() which everyone should call. On my machine: > > > > $ dmesg | grep "pcpu-alloc" > > [ 0.065118] pcpu-alloc: s184320 r8192 d28672 u262144 alloc=1*2097152 > > Doh, it wasn't printed to the console, due to KERN_DEBUG. Dmesg > does have it: > > <7>[ 0.000000] pcpu-alloc: s15520 r0 d29536 u45056 alloc=11*4096 > <7>[ 0.000000] pcpu-alloc: [0] 0 [0] 1 > I see, so what's happening is we're allocating 11 pages * 2, and due to percpu-km we round up to a contiguous 32 pages for backing pages. This results in the warning of wasting 10 pages. Given the size of the static region, I'm not too worried for now. I can't imagine the config would use that much percpu memory. We can massage the discrepancy for-v5.17. Basically in percpu-km, we align to 4k even though our allocation gets rounded up to the next power of 2. I don't have a lot of bandwidth right now, but I might be able to think about it over the next few weeks. Thanks, Dennis
Hi Dennis, On Tue, Dec 14, 2021 at 9:50 PM Dennis Zhou <dennis@kernel.org> wrote: > On Tue, Dec 14, 2021 at 09:12:06PM +0100, Geert Uytterhoeven wrote: > > On Tue, Dec 14, 2021 at 8:18 PM Dennis Zhou <dennis@kernel.org> wrote: > > > On Tue, Dec 14, 2021 at 08:02:58PM +0100, Geert Uytterhoeven wrote: > > > > On Tue, Dec 14, 2021 at 6:26 PM Dennis Zhou <dennis@kernel.org> wrote: > > > > > On Tue, Dec 14, 2021 at 05:29:22PM +0100, Geert Uytterhoeven wrote: > > > > > > On Wed, Dec 1, 2021 at 12:53 PM Vladimir Murzin <vladimir.murzin@arm.com> wrote: > > > > > > > On 11/30/21 5:41 PM, Dennis Zhou wrote: > > > > > > > > On Tue, Nov 30, 2021 at 05:29:54PM +0000, Vladimir Murzin wrote: > > > > > > > >> Currently, NOMMU pull km allocator via !SMP dependency because most of > > > > > > > >> them are UP, yet for SMP+NOMMU vm allocator gets pulled which: > > > > > > > >> > > > > > > > >> * may lead to broken build [1] > > > > > > > >> * ...or not working runtime due to [2] > > > > > > > >> > > > > > > > >> It looks like SMP+NOMMU case was overlooked in bbddff054587 ("percpu: > > > > > > > >> use percpu allocator on UP too") so restore that. > > > > > > > >> > > > > > > > >> [1] > > > > > > > >> For ARM SMP+NOMMU (R-class cores) > > > > > > > >> > > > > > > > >> arm-none-linux-gnueabihf-ld: mm/percpu.o: in function `pcpu_post_unmap_tlb_flush': > > > > > > > >> mm/percpu-vm.c:188: undefined reference to `flush_tlb_kernel_range' > > > > > > > >> > > > > > > > >> [2] > > > > > > > >> static inline > > > > > > > >> int vmap_pages_range_noflush(unsigned long addr, unsigned long end, > > > > > > > >> pgprot_t prot, struct page **pages, unsigned int page_shift) > > > > > > > >> { > > > > > > > >> return -EINVAL; > > > > > > > >> } > > > > > > > >> > > > > > > > >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > > > > > > > > > IIRC, RISC-V also have SMP+NOMMU, so adding them as well. > > > > > > > > > > > > I had seen the j-Core thread, but completely forgot about > > > > > > Canaan K210 (RV64 SMP+NOMMU). > > > > > > > > > > > > This became commit 3583521aabac76e5 ("percpu: km: ensure it is used > > > > > > with NOMMU (either UP or SMP)"). And now booting K210 prints: > > > > > > > > > > > > percpu: wasting 10 pages per chunk > > > > > > > > > > > > a) Is this bad? > > > > > > > > > > It's not great.. Can you share the line on boot with the following > > > > > prefix: pcpu-alloc [1]. > > > > > > > > There are no such lines. > > > > "make mm/percpu.i mm/percpu.s" and inspecting the generated files, > > > > and vmlinux, proves the code is there. But apparently it's not called. > > > > > > > > So there may be no issue on my system? > > > > > > I might be missing something, but that can't be right. Percpu calls > > > pcpu_dump_alloc_info() from pcpu_setup_first_chunk() which is called by > > > both embed/page first chunk code. > > > > > > Ummm. That can't be right. Percpu call pcpu_dump_alloc_info() from > > > pcpu_setup_first_chunk() which everyone should call. On my machine: > > > > > > $ dmesg | grep "pcpu-alloc" > > > [ 0.065118] pcpu-alloc: s184320 r8192 d28672 u262144 alloc=1*2097152 > > > > Doh, it wasn't printed to the console, due to KERN_DEBUG. Dmesg > > does have it: > > > > <7>[ 0.000000] pcpu-alloc: s15520 r0 d29536 u45056 alloc=11*4096 > > <7>[ 0.000000] pcpu-alloc: [0] 0 [0] 1 > > > > I see, so what's happening is we're allocating 11 pages * 2, and due to > percpu-km we round up to a contiguous 32 pages for backing pages. This > results in the warning of wasting 10 pages. Given the size of the static > region, I'm not too worried for now. I can't imagine the config would > use that much percpu memory. > > We can massage the discrepancy for-v5.17. Basically in percpu-km, we > align to 4k even though our allocation gets rounded up to the next power > of 2. I don't have a lot of bandwidth right now, but I might be able to > think about it over the next few weeks. Note that K210 has only 8 MiB of SRAM, so wasting 10 pages means wasting 0.5% of RAM. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
diff --git a/mm/Kconfig b/mm/Kconfig index d16ba92..66331e0 100644 --- a/mm/Kconfig +++ b/mm/Kconfig @@ -425,9 +425,8 @@ config THP_SWAP # UP and nommu archs use km based percpu allocator # config NEED_PER_CPU_KM - depends on !SMP bool - default y + default !SMP || !MMU config CLEANCACHE bool "Enable cleancache driver to cache clean pages if tmem is present"
Currently, NOMMU pull km allocator via !SMP dependency because most of them are UP, yet for SMP+NOMMU vm allocator gets pulled which: * may lead to broken build [1] * ...or not working runtime due to [2] It looks like SMP+NOMMU case was overlooked in bbddff054587 ("percpu: use percpu allocator on UP too") so restore that. [1] For ARM SMP+NOMMU (R-class cores) arm-none-linux-gnueabihf-ld: mm/percpu.o: in function `pcpu_post_unmap_tlb_flush': mm/percpu-vm.c:188: undefined reference to `flush_tlb_kernel_range' [2] static inline int vmap_pages_range_noflush(unsigned long addr, unsigned long end, pgprot_t prot, struct page **pages, unsigned int page_shift) { return -EINVAL; } Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> --- mm/Kconfig | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)