Re: [PATCH] percpu: km: ensure it is used with NOMMU (either UP or SMP)

Re: [PATCH] percpu: km: ensure it is used with NOMMU (either UP or SMP)

[Vladimir Murzin]

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
> 

Re: [PATCH] percpu: km: ensure it is used with NOMMU (either UP or SMP)

[Dennis Zhou]

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

Re: [PATCH] percpu: km: ensure it is used with NOMMU (either UP or SMP)

[Vladimir Murzin]

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
> 

Re: [PATCH] percpu: km: ensure it is used with NOMMU (either UP or SMP)

[Rob Landley]

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

Re: [PATCH] percpu: km: ensure it is used with NOMMU (either UP or SMP)

[Rich Felker]

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

Re: [PATCH] percpu: km: ensure it is used with NOMMU (either UP or SMP)

[Dennis Zhou]

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

Re: [PATCH] percpu: km: ensure it is used with NOMMU (either UP or SMP)

[Geert Uytterhoeven]

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

Re: [PATCH] percpu: km: ensure it is used with NOMMU (either UP or SMP)

[Dennis Zhou]

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

Re: [PATCH] percpu: km: ensure it is used with NOMMU (either UP or SMP)

[Geert Uytterhoeven]

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

Re: [PATCH] percpu: km: ensure it is used with NOMMU (either UP or SMP)

[Dennis Zhou]

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

Re: [PATCH] percpu: km: ensure it is used with NOMMU (either UP or SMP)

[Geert Uytterhoeven]

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

Re: [PATCH] percpu: km: ensure it is used with NOMMU (either UP or SMP)

[Dennis Zhou]

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

Re: [PATCH] percpu: km: ensure it is used with NOMMU (either UP or SMP)

[Geert Uytterhoeven]

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