* Re: linux-next: manual merge of the akpm-current tree with the powerpc tree
From: Christophe Leroy @ 2021-04-15 10:07 UTC (permalink / raw)
To: Stephen Rothwell, Andrew Morton, Michael Ellerman, PowerPC
Cc: Linux Next Mailing List, Linux Kernel Mailing List,
Nicholas Piggin
In-Reply-To: <20210415195814.0dc4ced9@canb.auug.org.au>
Le 15/04/2021 à 11:58, Stephen Rothwell a écrit :
> Hi all,
>
> On Thu, 15 Apr 2021 19:44:17 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>
>> Today's linux-next merge of the akpm-current tree got a conflict in:
>>
>> arch/powerpc/kernel/module.c
>>
>> between commit:
>>
>> 2ec13df16704 ("powerpc/modules: Load modules closer to kernel text")
>>
>> from the powerpc tree and commit:
>>
>> 4930ba789f8d ("powerpc/64s/radix: enable huge vmalloc mappings")
>>
>> from the akpm-current tree.
>>
>> I fixed it up (I think - see below) and can carry the fix as
>> necessary. This is now fixed as far as linux-next is concerned, but any
>> non trivial conflicts should be mentioned to your upstream maintainer
>> when your tree is submitted for merging. You may also want to consider
>> cooperating with the maintainer of the conflicting tree to minimise any
>> particularly complex conflicts.
>>
>> --
>> Cheers,
>> Stephen Rothwell
>>
>> diff --cc arch/powerpc/kernel/module.c
>> index fab84024650c,cdb2d88c54e7..000000000000
>> --- a/arch/powerpc/kernel/module.c
>> +++ b/arch/powerpc/kernel/module.c
>> @@@ -88,29 -88,26 +89,42 @@@ int module_finalize(const Elf_Ehdr *hdr
>> return 0;
>> }
>>
>> - #ifdef MODULES_VADDR
>> -void *module_alloc(unsigned long size)
>> +static __always_inline void *
>> +__module_alloc(unsigned long size, unsigned long start, unsigned long end)
>> {
>> - unsigned long start = VMALLOC_START;
>> - unsigned long end = VMALLOC_END;
>> -
>> -#ifdef MODULES_VADDR
>> - BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR);
>> - start = MODULES_VADDR;
>> - end = MODULES_END;
>> -#endif
>> -
>> + /*
>> + * Don't do huge page allocations for modules yet until more testing
>> + * is done. STRICT_MODULE_RWX may require extra work to support this
>> + * too.
>> + */
>> +
>> return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL,
>> - PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
>> + PAGE_KERNEL_EXEC,
>> + VM_NO_HUGE_VMAP | VM_FLUSH_RESET_PERMS,
>> + NUMA_NO_NODE,
>> __builtin_return_address(0));
>> }
>> +
>> ++
>> +void *module_alloc(unsigned long size)
>> +{
>> ++ unsigned long start = VMALLOC_START;
>> ++ unsigned long end = VMALLOC_END;
>> + unsigned long limit = (unsigned long)_etext - SZ_32M;
>> + void *ptr = NULL;
>> +
>> ++#ifdef MODULES_VADDR
>> + BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR);
>> ++ start = MODULES_VADDR;
>> ++ end = MODULES_END;
The #endif should be here.
>> +
>> + /* First try within 32M limit from _etext to avoid branch trampolines */
>> + if (MODULES_VADDR < PAGE_OFFSET && MODULES_END > limit)
>> - ptr = __module_alloc(size, limit, MODULES_END);
>> ++ ptr = __module_alloc(size, limit, end);
>> +
>> + if (!ptr)
>> - ptr = __module_alloc(size, MODULES_VADDR, MODULES_END);
>> ++#endif
>> ++ ptr = __module_alloc(size, start, end);
>> +
>> + return ptr;
>> +}
>> - #endif
>
> Unfortunately, it also needs this:
Before the #endif is too far.
>
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Thu, 15 Apr 2021 19:53:58 +1000
> Subject: [PATCH] merge fix up for powerpc merge fix
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
> arch/powerpc/kernel/module.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
> index d8ab1ad2eb05..c060f99afd4d 100644
> --- a/arch/powerpc/kernel/module.c
> +++ b/arch/powerpc/kernel/module.c
> @@ -110,7 +110,9 @@ void *module_alloc(unsigned long size)
> {
> unsigned long start = VMALLOC_START;
> unsigned long end = VMALLOC_END;
> +#ifdef MODULES_VADDR
> unsigned long limit = (unsigned long)_etext - SZ_32M;
> +#endif
> void *ptr = NULL;
>
> #ifdef MODULES_VADDR
>
^ permalink raw reply
* Re: linux-next: manual merge of the akpm-current tree with the powerpc tree
From: Christophe Leroy @ 2021-04-15 10:08 UTC (permalink / raw)
To: Stephen Rothwell, Andrew Morton, Michael Ellerman, PowerPC
Cc: Linux Next Mailing List, Linux Kernel Mailing List,
Nicholas Piggin
In-Reply-To: <9bc1b8fd-8051-54ed-b9d8-198fe1f4c348@csgroup.eu>
Le 15/04/2021 à 12:07, Christophe Leroy a écrit :
>
>
> Le 15/04/2021 à 11:58, Stephen Rothwell a écrit :
>> Hi all,
>>
>> On Thu, 15 Apr 2021 19:44:17 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>>
>>> Today's linux-next merge of the akpm-current tree got a conflict in:
>>>
>>> arch/powerpc/kernel/module.c
>>>
>>> between commit:
>>>
>>> 2ec13df16704 ("powerpc/modules: Load modules closer to kernel text")
>>>
>>> from the powerpc tree and commit:
>>>
>>> 4930ba789f8d ("powerpc/64s/radix: enable huge vmalloc mappings")
>>>
>>> from the akpm-current tree.
>>>
>>> I fixed it up (I think - see below) and can carry the fix as
>>> necessary. This is now fixed as far as linux-next is concerned, but any
>>> non trivial conflicts should be mentioned to your upstream maintainer
>>> when your tree is submitted for merging. You may also want to consider
>>> cooperating with the maintainer of the conflicting tree to minimise any
>>> particularly complex conflicts.
>>>
>>> --
>>> Cheers,
>>> Stephen Rothwell
>>>
>>> diff --cc arch/powerpc/kernel/module.c
>>> index fab84024650c,cdb2d88c54e7..000000000000
>>> --- a/arch/powerpc/kernel/module.c
>>> +++ b/arch/powerpc/kernel/module.c
>>> @@@ -88,29 -88,26 +89,42 @@@ int module_finalize(const Elf_Ehdr *hdr
>>> return 0;
>>> }
>>> - #ifdef MODULES_VADDR
>>> -void *module_alloc(unsigned long size)
>>> +static __always_inline void *
>>> +__module_alloc(unsigned long size, unsigned long start, unsigned long end)
>>> {
>>> - unsigned long start = VMALLOC_START;
>>> - unsigned long end = VMALLOC_END;
>>> -
>>> -#ifdef MODULES_VADDR
>>> - BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR);
>>> - start = MODULES_VADDR;
>>> - end = MODULES_END;
>>> -#endif
>>> -
>>> + /*
>>> + * Don't do huge page allocations for modules yet until more testing
>>> + * is done. STRICT_MODULE_RWX may require extra work to support this
>>> + * too.
>>> + */
>>> +
>>> return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL,
>>> - PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
>>> + PAGE_KERNEL_EXEC,
>>> + VM_NO_HUGE_VMAP | VM_FLUSH_RESET_PERMS,
>>> + NUMA_NO_NODE,
>>> __builtin_return_address(0));
>>> }
>>> +
>>> ++
>>> +void *module_alloc(unsigned long size)
>>> +{
>>> ++ unsigned long start = VMALLOC_START;
>>> ++ unsigned long end = VMALLOC_END;
>>> + unsigned long limit = (unsigned long)_etext - SZ_32M;
>>> + void *ptr = NULL;
>>> +
>>> ++#ifdef MODULES_VADDR
>>> + BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR);
>>> ++ start = MODULES_VADDR;
>>> ++ end = MODULES_END;
>
> The #endif should be here.
>
>
>>> +
>>> + /* First try within 32M limit from _etext to avoid branch trampolines */
>>> + if (MODULES_VADDR < PAGE_OFFSET && MODULES_END > limit)
Should also use start and end here instead of MODULES_VADDR and MODULES_END
>>> - ptr = __module_alloc(size, limit, MODULES_END);
>>> ++ ptr = __module_alloc(size, limit, end);
>>> +
>>> + if (!ptr)
>>> - ptr = __module_alloc(size, MODULES_VADDR, MODULES_END);
>>> ++#endif
>>> ++ ptr = __module_alloc(size, start, end);
>>> +
>>> + return ptr;
>>> +}
>>> - #endif
>>
>> Unfortunately, it also needs this:
>
> Before the #endif is too far.
>
>>
>> From: Stephen Rothwell <sfr@canb.auug.org.au>
>> Date: Thu, 15 Apr 2021 19:53:58 +1000
>> Subject: [PATCH] merge fix up for powerpc merge fix
>>
>> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
>> ---
>> arch/powerpc/kernel/module.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
>> index d8ab1ad2eb05..c060f99afd4d 100644
>> --- a/arch/powerpc/kernel/module.c
>> +++ b/arch/powerpc/kernel/module.c
>> @@ -110,7 +110,9 @@ void *module_alloc(unsigned long size)
>> {
>> unsigned long start = VMALLOC_START;
>> unsigned long end = VMALLOC_END;
>> +#ifdef MODULES_VADDR
>> unsigned long limit = (unsigned long)_etext - SZ_32M;
>> +#endif
>> void *ptr = NULL;
>> #ifdef MODULES_VADDR
>>
^ permalink raw reply
* Re: linux-next: manual merge of the akpm-current tree with the powerpc tree
From: Christophe Leroy @ 2021-04-15 10:15 UTC (permalink / raw)
To: Stephen Rothwell, Andrew Morton, Michael Ellerman, PowerPC
Cc: Linux Next Mailing List, Linux Kernel Mailing List,
Nicholas Piggin
In-Reply-To: <1d3b44a8-f19c-b52b-ce44-20c5e5b706ad@csgroup.eu>
Le 15/04/2021 à 12:08, Christophe Leroy a écrit :
>
>
> Le 15/04/2021 à 12:07, Christophe Leroy a écrit :
>>
>>
>> Le 15/04/2021 à 11:58, Stephen Rothwell a écrit :
>>> Hi all,
>>>
>>> On Thu, 15 Apr 2021 19:44:17 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>>>
>>>> Today's linux-next merge of the akpm-current tree got a conflict in:
>>>>
>>>> arch/powerpc/kernel/module.c
>>>>
>>>> between commit:
>>>>
>>>> 2ec13df16704 ("powerpc/modules: Load modules closer to kernel text")
>>>>
>>>> from the powerpc tree and commit:
>>>>
>>>> 4930ba789f8d ("powerpc/64s/radix: enable huge vmalloc mappings")
>>>>
>>>> from the akpm-current tree.
>>>>
>>>> I fixed it up (I think - see below) and can carry the fix as
>>>> necessary. This is now fixed as far as linux-next is concerned, but any
>>>> non trivial conflicts should be mentioned to your upstream maintainer
>>>> when your tree is submitted for merging. You may also want to consider
>>>> cooperating with the maintainer of the conflicting tree to minimise any
>>>> particularly complex conflicts.
>>>>
>>>> --
>>>> Cheers,
>>>> Stephen Rothwell
>>>>
>>>> diff --cc arch/powerpc/kernel/module.c
>>>> index fab84024650c,cdb2d88c54e7..000000000000
>>>> --- a/arch/powerpc/kernel/module.c
>>>> +++ b/arch/powerpc/kernel/module.c
>>>> @@@ -88,29 -88,26 +89,42 @@@ int module_finalize(const Elf_Ehdr *hdr
>>>> return 0;
>>>> }
>>>> - #ifdef MODULES_VADDR
>>>> -void *module_alloc(unsigned long size)
>>>> +static __always_inline void *
>>>> +__module_alloc(unsigned long size, unsigned long start, unsigned long end)
>>>> {
>>>> - unsigned long start = VMALLOC_START;
>>>> - unsigned long end = VMALLOC_END;
>>>> -
>>>> -#ifdef MODULES_VADDR
>>>> - BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR);
>>>> - start = MODULES_VADDR;
>>>> - end = MODULES_END;
>>>> -#endif
>>>> -
>>>> + /*
>>>> + * Don't do huge page allocations for modules yet until more testing
>>>> + * is done. STRICT_MODULE_RWX may require extra work to support this
>>>> + * too.
>>>> + */
>>>> +
>>>> return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL,
>>>> - PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
>>>> + PAGE_KERNEL_EXEC,
>>>> + VM_NO_HUGE_VMAP | VM_FLUSH_RESET_PERMS,
>>>> + NUMA_NO_NODE,
>>>> __builtin_return_address(0));
>>>> }
>>>> +
>>>> ++
>>>> +void *module_alloc(unsigned long size)
>>>> +{
>>>> ++ unsigned long start = VMALLOC_START;
>>>> ++ unsigned long end = VMALLOC_END;
>>>> + unsigned long limit = (unsigned long)_etext - SZ_32M;
>>>> + void *ptr = NULL;
>>>> +
>>>> ++#ifdef MODULES_VADDR
>>>> + BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR);
>>>> ++ start = MODULES_VADDR;
>>>> ++ end = MODULES_END;
>>
>> The #endif should be here.
>>
>>
>>>> +
>>>> + /* First try within 32M limit from _etext to avoid branch trampolines */
>>>> + if (MODULES_VADDR < PAGE_OFFSET && MODULES_END > limit)
>
> Should also use start and end here instead of MODULES_VADDR and MODULES_END
The cleanest however should be to define MODULES_VADDR and MODULES_END all the time with a fallback
to VMALLOC_START/VMALLOC_END, to avoid the #ifdef.
The #ifdef was OK when we wanted to define modules_alloc() only when module area was different from
vmalloc area, but now that we want modules_alloc() at all time, MODULES_VADDR and MODULES_END should
be defined all the time.
>
>>>> - ptr = __module_alloc(size, limit, MODULES_END);
>>>> ++ ptr = __module_alloc(size, limit, end);
>>>> +
>>>> + if (!ptr)
>>>> - ptr = __module_alloc(size, MODULES_VADDR, MODULES_END);
>>>> ++#endif
>>>> ++ ptr = __module_alloc(size, start, end);
>>>> +
>>>> + return ptr;
>>>> +}
>>>> - #endif
>>>
>>> Unfortunately, it also needs this:
>>
>> Before the #endif is too far.
>>
>>>
>>> From: Stephen Rothwell <sfr@canb.auug.org.au>
>>> Date: Thu, 15 Apr 2021 19:53:58 +1000
>>> Subject: [PATCH] merge fix up for powerpc merge fix
>>>
>>> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
>>> ---
>>> arch/powerpc/kernel/module.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
>>> index d8ab1ad2eb05..c060f99afd4d 100644
>>> --- a/arch/powerpc/kernel/module.c
>>> +++ b/arch/powerpc/kernel/module.c
>>> @@ -110,7 +110,9 @@ void *module_alloc(unsigned long size)
>>> {
>>> unsigned long start = VMALLOC_START;
>>> unsigned long end = VMALLOC_END;
>>> +#ifdef MODULES_VADDR
>>> unsigned long limit = (unsigned long)_etext - SZ_32M;
>>> +#endif
>>> void *ptr = NULL;
>>> #ifdef MODULES_VADDR
>>>
^ permalink raw reply
* Re: [PATCH v13 14/14] powerpc/64s/radix: Enable huge vmalloc mappings
From: Christophe Leroy @ 2021-04-15 10:23 UTC (permalink / raw)
To: Nicholas Piggin, linux-mm, Andrew Morton
Cc: linux-arch, Stephen Rothwell, Jonathan Cameron, linux-kernel,
Christoph Hellwig, Ding Tianhong, Rick Edgecombe, linuxppc-dev
In-Reply-To: <20210317062402.533919-15-npiggin@gmail.com>
Hi Nick,
Le 17/03/2021 à 07:24, Nicholas Piggin a écrit :
> This reduces TLB misses by nearly 30x on a `git diff` workload on a
> 2-node POWER9 (59,800 -> 2,100) and reduces CPU cycles by 0.54%, due
> to vfs hashes being allocated with 2MB pages.
>
> Cc: linuxppc-dev@lists.ozlabs.org
> Acked-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> .../admin-guide/kernel-parameters.txt | 2 ++
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/kernel/module.c | 22 +++++++++++++++----
> 3 files changed, 21 insertions(+), 4 deletions(-)
>
> --- a/arch/powerpc/kernel/module.c
> +++ b/arch/powerpc/kernel/module.c
> @@ -8,6 +8,7 @@
> #include <linux/moduleloader.h>
> #include <linux/err.h>
> #include <linux/vmalloc.h>
> +#include <linux/mm.h>
> #include <linux/bug.h>
> #include <asm/module.h>
> #include <linux/uaccess.h>
> @@ -87,13 +88,26 @@ int module_finalize(const Elf_Ehdr *hdr,
> return 0;
> }
>
> -#ifdef MODULES_VADDR
> void *module_alloc(unsigned long size)
> {
> + unsigned long start = VMALLOC_START;
> + unsigned long end = VMALLOC_END;
> +
> +#ifdef MODULES_VADDR
> BUILD_BUG_ON(TASK_SIZE > MODULES_VADDR);
> + start = MODULES_VADDR;
> + end = MODULES_END;
> +#endif
> +
> + /*
> + * Don't do huge page allocations for modules yet until more testing
> + * is done. STRICT_MODULE_RWX may require extra work to support this
> + * too.
> + */
>
> - return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END, GFP_KERNEL,
> - PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
I think you should add the following in <asm/pgtable.h>
#ifndef MODULES_VADDR
#define MODULES_VADDR VMALLOC_START
#define MODULES_END VMALLOC_END
#endif
And leave module_alloc() as is (just removing the enclosing #ifdef MODULES_VADDR and adding the
VM_NO_HUGE_VMAP flag)
This would minimise the conflits with the changes I did in powerpc/next reported by Stephen R.
> + return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL,
> + PAGE_KERNEL_EXEC,
> + VM_NO_HUGE_VMAP | VM_FLUSH_RESET_PERMS,
> + NUMA_NO_NODE,
> __builtin_return_address(0));
> }
> -#endif
>
^ permalink raw reply
* Re: [PATCH] powerpc/papr_scm: Reduce error severity if nvdimm stats inaccessible
From: Vaibhav Jain @ 2021-04-15 11:43 UTC (permalink / raw)
To: Dan Williams
Cc: Aneesh Kumar K . V, Santosh Sivaraj, Ira Weiny, linuxppc-dev,
linux-nvdimm
In-Reply-To: <CAPcyv4iU3cmjRsDevDJmJc72xo-QffUu3SGCwvRh5bitG-facw@mail.gmail.com>
Thanks for looking into this Dan,
Dan Williams <dan.j.williams@intel.com> writes:
> On Wed, Apr 14, 2021 at 5:40 AM Vaibhav Jain <vaibhav@linux.ibm.com> wrote:
>>
>> Currently drc_pmem_qeury_stats() generates a dev_err in case
>> "Enable Performance Information Collection" feature is disabled from
>> HMC. The error is of the form below:
>>
>> papr_scm ibm,persistent-memory:ibm,pmemory@44104001: Failed to query
>> performance stats, Err:-10
>>
>> This error message confuses users as it implies a possible problem
>> with the nvdimm even though its due to a disabled feature.
>>
>> So we fix this by explicitly handling the H_AUTHORITY error from the
>> H_SCM_PERFORMANCE_STATS hcall and generating a warning instead of an
>> error, saying that "Performance stats in-accessible".
>>
>> Fixes: 2d02bf835e57('powerpc/papr_scm: Fetch nvdimm performance stats from PHYP')
>> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
>> ---
>> arch/powerpc/platforms/pseries/papr_scm.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
>> index 835163f54244..9216424f8be3 100644
>> --- a/arch/powerpc/platforms/pseries/papr_scm.c
>> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
>> @@ -277,6 +277,9 @@ static ssize_t drc_pmem_query_stats(struct papr_scm_priv *p,
>> dev_err(&p->pdev->dev,
>> "Unknown performance stats, Err:0x%016lX\n", ret[0]);
>> return -ENOENT;
>> + } else if (rc == H_AUTHORITY) {
>> + dev_warn(&p->pdev->dev, "Performance stats in-accessible");
>> + return -EPERM;
>
> So userspace can spam the kernel log? Why is kernel log message needed
> at all? EPERM told the caller what happened.
Currently this error message is only reported during probe of the
nvdimm. So userspace cannot directly spam kernel log.
The callsite for this function in papr_scm_nvdimm_init() doesnt handle
specific error codes. Instead in case of an error it only reports that
"Dimm performance stats are unavailable". The log message just
preceeding that mentions the real cause of failure. Thats why just
returning -EPERM wont be usefui.
Alternatively I can update papr_scm_nvdimm_init() to report the error
code returned from drc_pmem_query_stats().
--
Cheers
~ Vaibhav
^ permalink raw reply
* Re: [PATCH] powerpc/papr_scm: Reduce error severity if nvdimm stats inaccessible
From: Vaibhav Jain @ 2021-04-15 11:48 UTC (permalink / raw)
To: Ira Weiny
Cc: Aneesh Kumar K . V, Santosh Sivaraj, linuxppc-dev, Dan Williams,
linux-nvdimm
In-Reply-To: <20210414212417.GC1904484@iweiny-DESK2.sc.intel.com>
Ira Weiny <ira.weiny@intel.com> writes:
> On Wed, Apr 14, 2021 at 09:51:40PM +0530, Vaibhav Jain wrote:
>> Thanks for looking into this patch Ira,
>>
>> Ira Weiny <ira.weiny@intel.com> writes:
>>
>> > On Wed, Apr 14, 2021 at 06:10:26PM +0530, Vaibhav Jain wrote:
>> >> Currently drc_pmem_qeury_stats() generates a dev_err in case
>> >> "Enable Performance Information Collection" feature is disabled from
>> >> HMC. The error is of the form below:
>> >>
>> >> papr_scm ibm,persistent-memory:ibm,pmemory@44104001: Failed to query
>> >> performance stats, Err:-10
>> >>
>> >> This error message confuses users as it implies a possible problem
>> >> with the nvdimm even though its due to a disabled feature.
>> >>
>> >> So we fix this by explicitly handling the H_AUTHORITY error from the
>> >> H_SCM_PERFORMANCE_STATS hcall and generating a warning instead of an
>> >> error, saying that "Performance stats in-accessible".
>> >>
>> >> Fixes: 2d02bf835e57('powerpc/papr_scm: Fetch nvdimm performance stats from PHYP')
>> >> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
>> >> ---
>> >> arch/powerpc/platforms/pseries/papr_scm.c | 3 +++
>> >> 1 file changed, 3 insertions(+)
>> >>
>> >> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
>> >> index 835163f54244..9216424f8be3 100644
>> >> --- a/arch/powerpc/platforms/pseries/papr_scm.c
>> >> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
>> >> @@ -277,6 +277,9 @@ static ssize_t drc_pmem_query_stats(struct papr_scm_priv *p,
>> >> dev_err(&p->pdev->dev,
>> >> "Unknown performance stats, Err:0x%016lX\n", ret[0]);
>> >> return -ENOENT;
>> >> + } else if (rc == H_AUTHORITY) {
>> >> + dev_warn(&p->pdev->dev, "Performance stats in-accessible");
>> >> + return -EPERM;
>> >
>> > Is this because of a disabled feature or because of permissions?
>>
>> Its because of a disabled feature that revokes permission for a guest to
>> retrieve performance statistics.
>>
>> The feature is called "Enable Performance Information Collection" and
>> once disabled the hcall H_SCM_PERFORMANCE_STATS returns an error
>> H_AUTHORITY indicating that the guest doesn't have permission to retrieve
>> performance statistics.
>
> In that case would it be appropriate to have the error message indicate a
> permission issue?
>
> Something like 'permission denied'?
Yes, Something like "Permission denied while accessing performance
stats" might be more clear and intuitive.
Will update the warn message in v2.
>
> Ira
>
--
Cheers
~ Vaibhav
^ permalink raw reply
* Re: [PATCHv5 2/2] powerpc/pseries: update device tree before ejecting hotplug uevents
From: Michal Suchánek @ 2021-04-15 12:09 UTC (permalink / raw)
To: Pingfan Liu
Cc: Nathan Lynch, Kexec Mailing List, Nathan Fontenot, Laurent Dufour,
linuxppc-dev, Hari Bathini
In-Reply-To: <CAFgQCTsTWiYcuiipWmnieuNTWTar30aAUOsE3L9CcrVBiQix3A@mail.gmail.com>
Hello,
On Wed, Apr 14, 2021 at 11:08:19AM +0800, Pingfan Liu wrote:
> On Sat, Apr 10, 2021 at 12:33 AM Michal Suchánek <msuchanek@suse.de> wrote:
> >
> > Hello,
> >
> > On Fri, Aug 28, 2020 at 04:10:09PM +0800, Pingfan Liu wrote:
> > > On Thu, Aug 27, 2020 at 3:53 PM Laurent Dufour <ldufour@linux.ibm.com> wrote:
> > > >
> > > > Le 10/08/2020 à 10:52, Pingfan Liu a écrit :
> > > > > A bug is observed on pseries by taking the following steps on rhel:
> > > > > -1. drmgr -c mem -r -q 5
> > > > > -2. echo c > /proc/sysrq-trigger
> > > > >
> > > > > And then, the failure looks like:
> > > > > kdump: saving to /sysroot//var/crash/127.0.0.1-2020-01-16-02:06:14/
> > > > > kdump: saving vmcore-dmesg.txt
> > > > > kdump: saving vmcore-dmesg.txt complete
> > > > > kdump: saving vmcore
> > > > > Checking for memory holes : [ 0.0 %] / Checking for memory holes : [100.0 %] | Excluding unnecessary pages : [100.0 %] \ Copying data : [ 0.3 %] - eta: 38s[ 44.337636] hash-mmu: mm: Hashing failure ! EA=0x7fffba400000 access=0x8000000000000004 current=makedumpfile
> > > > > [ 44.337663] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc000000050000504
> > > > > [ 44.337677] hash-mmu: mm: Hashing failure ! EA=0x7fffba400000 access=0x8000000000000004 current=makedumpfile
> > > > > [ 44.337692] hash-mmu: trap=0x300 vsid=0x13a109c ssize=1 base psize=2 psize 2 pte=0xc000000050000504
> > > > > [ 44.337708] makedumpfile[469]: unhandled signal 7 at 00007fffba400000 nip 00007fffbbc4d7fc lr 000000011356ca3c code 2
> > > > > [ 44.338548] Core dump to |/bin/false pipe failed
> > > > > /lib/kdump-lib-initramfs.sh: line 98: 469 Bus error $CORE_COLLECTOR /proc/vmcore $_mp/$KDUMP_PATH/$HOST_IP-$DATEDIR/vmcore-incomplete
> > > > > kdump: saving vmcore failed
> > > > >
> > > > > * Root cause *
> > > > > After analyzing, it turns out that in the current implementation,
> > > > > when hot-removing lmb, the KOBJ_REMOVE event ejects before the dt updating as
> > > > > the code __remove_memory() comes before drmem_update_dt().
> > > > > So in kdump kernel, when read_from_oldmem() resorts to
> > > > > pSeries_lpar_hpte_insert() to install hpte, but fails with -2 due to
> > > > > non-exist pfn. And finally, low_hash_fault() raise SIGBUS to process, as it
> > > > > can be observed "Bus error"
> > > > >
> > > > > From a viewpoint of listener and publisher, the publisher notifies the
> > > > > listener before data is ready. This introduces a problem where udev
> > > > > launches kexec-tools (due to KOBJ_REMOVE) and loads a stale dt before
> > > > > updating. And in capture kernel, makedumpfile will access the memory based
> > > > > on the stale dt info, and hit a SIGBUS error due to an un-existed lmb.
> > > > >
> > > > > * Fix *
> > > > > This bug is introduced by commit 063b8b1251fd
> > > > > ("powerpc/pseries/memory-hotplug: Only update DT once per memory DLPAR
> > > > > request"), which tried to combine all the dt updating into one.
> > > > >
> > > > > To fix this issue, meanwhile not to introduce a quadratic runtime
> > > > > complexity by the model:
> > > > > dlpar_memory_add_by_count
> > > > > for_each_drmem_lmb <--
> > > > > dlpar_add_lmb
> > > > > drmem_update_dt(_v1|_v2)
> > > > > for_each_drmem_lmb <--
> > > > > The dt should still be only updated once, and just before the last memory
> > > > > online/offline event is ejected to user space. Achieve this by tracing the
> > > > > num of lmb added or removed.
> > > > >
> > > > > Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> > > > > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > > > > Cc: Hari Bathini <hbathini@linux.ibm.com>
> > > > > Cc: Nathan Lynch <nathanl@linux.ibm.com>
> > > > > Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> > > > > Cc: Laurent Dufour <ldufour@linux.ibm.com>
> > > > > To: linuxppc-dev@lists.ozlabs.org
> > > > > Cc: kexec@lists.infradead.org
> > > > > ---
> > > > > v4 -> v5: change dlpar_add_lmb()/dlpar_remove_lmb() prototype to report
> > > > > whether dt is updated successfully.
> > > > > Fix a condition boundary check bug
> > > > > v3 -> v4: resolve a quadratic runtime complexity issue.
> > > > > This series is applied on next-test branch
> > > > > arch/powerpc/platforms/pseries/hotplug-memory.c | 102 +++++++++++++++++++-----
> > > > > 1 file changed, 80 insertions(+), 22 deletions(-)
> > > > >
> > > > > diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> > > > > index 46cbcd1..1567d9f 100644
> > > > > --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> > > > > +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> > > > > @@ -350,13 +350,22 @@ static bool lmb_is_removable(struct drmem_lmb *lmb)
> > > > > return true;
> > > > > }
> > > > >
> > > > > -static int dlpar_add_lmb(struct drmem_lmb *);
> > > > > +enum dt_update_status {
> > > > > + DT_NOUPDATE,
> > > > > + DT_TOUPDATE,
> > > > > + DT_UPDATED,
> > > > > +};
> > > > > +
> > > > > +/* "*dt_update" returns DT_UPDATED if updated */
> > > > > +static int dlpar_add_lmb(struct drmem_lmb *lmb,
> > > > > + enum dt_update_status *dt_update);
> > > > >
> > > > > -static int dlpar_remove_lmb(struct drmem_lmb *lmb)
> > > > > +static int dlpar_remove_lmb(struct drmem_lmb *lmb,
> > > > > + enum dt_update_status *dt_update)
> > > > > {
> > > > > unsigned long block_sz;
> > > > > phys_addr_t base_addr;
> > > > > - int rc, nid;
> > > > > + int rc, ret, nid;
> > > > >
> > > > > if (!lmb_is_removable(lmb))
> > > > > return -EINVAL;
> > > > > @@ -372,6 +381,13 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
> > > > > invalidate_lmb_associativity_index(lmb);
> > > > > lmb_clear_nid(lmb);
> > > > > lmb->flags &= ~DRCONF_MEM_ASSIGNED;
> > > > > + if (*dt_update) {
> > > Original, I plan to use it to exclude the case of DT_NOUPDATE, which is value 0.
> > > And I think it looks better by using if (*dt_update == DT_TOUPDATE)
> > > >
> > > > That test is wrong, you should do:
> > > > if (*dt_update && *dt_update == DT_TOUPDATE) {
> > > I think you mean if (dt_update && *dt_update == DT_TOUPDATE) {
> > > >
> > > > With the current code, the device tree is updated all the time.
> > > >
> > > > Another option would be to pass a valid pointer (!= NULL) only when DT update is
> > > > required, this way you don't need the DT_TOUPDATE value. The caller would have
> > > > to set the pointer accordingly. The advantage with this option is the caller is
> > > > guaranteed that its variable is not touched by the callee when no device tree is
> > > > requested. A simple boolean pointer would be enough without the need to this enum.
> > > It is expected that dlpar_remove_lmb/dlpar_add_lmb can report whether
> > > they successfully update dt or not. So the caller can handle the
> > > different cases.
> >
> > Is there any plan to refresh this patch to apply to master?
> >
> Spreading the failure and rollback logic around the code looks urgly.
> Even myself dislike it. But I have not found a better way out.
>
> > I am using an older revision of this patch so I am not in the position
> > to repost an updated version.
> >
> > I lack some otimization in my patch so I probably have the quadratic
> > coplexity of the add mentioned above.
> >
> Do you have any good ideas?
I am not very familiar with the code and I am not sure I will have time
to look closer at it anytime soon.
Computation complexity is traded for code complexity and it is not clear
if the tradeoff is good.
As a way forward I would suggest to split the patch into a minimal
correctness fix and an additional optimization that avoids needless
computation.
That way the extra code complexity and the speed improvement brought by
the optimization can be reviewed more easily.
I don't have myself access to any big machine so can't tell if the
optimization solves a real world problem either.
Thanks
Michal
^ permalink raw reply
* [PATCH 0/3] Reintroduce cpu_core_mask
From: Srikar Dronamraju @ 2021-04-15 12:09 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, Srikar Dronamraju
Daniel had reported that
QEMU is now unable to see requested topologies in a multi socket single
NUMA node configurations.
-smp 8,maxcpus=8,cores=2,threads=2,sockets=2
This patchset reintroduces cpu_core_mask so that users can see requested
topologies while still maintaining the boot time of very large system
configurations.
It includes caching the chip_id as suggested by Michael Ellermann
4 Threads/Core; 4 cores/Socket; 4 Sockets/Node, 2 Nodes in System
-numa node,nodeid=0,memdev=m0 \
-numa node,nodeid=1,memdev=m1 \
-smp 128,sockets=8,threads=4,maxcpus=128 \
5.12.0-rc5 (or any kernel with commit 4ca234a9cbd7)
---------------------------------------------------
srikar@cloudy:~$ lscpu
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 128
On-line CPU(s) list: 0-127
Thread(s) per core: 4
Core(s) per socket: 16
Socket(s): 2 <<<<<-----
NUMA node(s): 2
Model: 2.3 (pvr 004e 1203)
Model name: POWER9 (architected), altivec supported
Hypervisor vendor: KVM
Virtualization type: para
L1d cache: 1 MiB
L1i cache: 1 MiB
NUMA node0 CPU(s): 0-15,32-47,64-79,96-111
NUMA node1 CPU(s): 16-31,48-63,80-95,112-127
--
srikar@cloudy:~$ dmesg |grep smp
[ 0.010658] smp: Bringing up secondary CPUs ...
[ 0.424681] smp: Brought up 2 nodes, 128 CPUs
--
5.12.0-rc5 + 3 patches
----------------------
srikar@cloudy:~$ lscpu
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 128
On-line CPU(s) list: 0-127
Thread(s) per core: 4
Core(s) per socket: 4
Socket(s): 8 <<<<-----
NUMA node(s): 2
Model: 2.3 (pvr 004e 1203)
Model name: POWER9 (architected), altivec supported
Hypervisor vendor: KVM
Virtualization type: para
L1d cache: 1 MiB
L1i cache: 1 MiB
NUMA node0 CPU(s): 0-15,32-47,64-79,96-111
NUMA node1 CPU(s): 16-31,48-63,80-95,112-127
--
srikar@cloudy:~$ dmesg |grep smp
[ 0.010372] smp: Bringing up secondary CPUs ...
[ 0.417892] smp: Brought up 2 nodes, 128 CPUs
5.12.0-rc5
----------
srikar@cloudy:~$ lscpu
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 1024
On-line CPU(s) list: 0-1023
Thread(s) per core: 8
Core(s) per socket: 128
Socket(s): 1
NUMA node(s): 1
Model: 2.3 (pvr 004e 1203)
Model name: POWER9 (architected), altivec supported
Hypervisor vendor: KVM
Virtualization type: para
L1d cache: 4 MiB
L1i cache: 4 MiB
NUMA node0 CPU(s): 0-1023
srikar@cloudy:~$ dmesg | grep smp
[ 0.027753 ] smp: Bringing up secondary CPUs ...
[ 2.315193 ] smp: Brought up 1 node, 1024 CPUs
5.12.0-rc5 + 3 patches
----------------------
srikar@cloudy:~$ dmesg | grep smp
[ 0.027659 ] smp: Bringing up secondary CPUs ...
[ 2.532739 ] smp: Brought up 1 node, 1024 CPUs
I also have booted and tested the kernels on PowerVM and PowerNV and
even there I see a very negligible increase in the bringing up time of
secondary CPUs
Srikar Dronamraju (3):
powerpc/smp: Reintroduce cpu_core_mask
Revert "powerpc/topology: Update topology_core_cpumask"
powerpc/smp: Cache CPU to chip lookup
arch/powerpc/include/asm/smp.h | 6 ++++
arch/powerpc/include/asm/topology.h | 2 +-
arch/powerpc/kernel/prom.c | 19 +++++++---
arch/powerpc/kernel/smp.c | 56 +++++++++++++++++++++++++----
4 files changed, 71 insertions(+), 12 deletions(-)
--
2.25.1
^ permalink raw reply
* [PATCH 2/3] Revert "powerpc/topology: Update topology_core_cpumask"
From: Srikar Dronamraju @ 2021-04-15 12:09 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Srikar Dronamraju, Peter Zijlstra,
Daniel Henrique Barboza, Valentin Schneider, qemu-ppc,
Cedric Le Goater, linuxppc-dev, Ingo Molnar, David Gibson
In-Reply-To: <20210415120934.232271-1-srikar@linux.vnet.ibm.com>
Now that cpu_core_mask has been reintroduced, lets revert
commit 4bce545903fa ("powerpc/topology: Update topology_core_cpumask")
Post this commit, lscpu should reflect topologies as requested by a user
when a QEMU instance is launched with NUMA spanning multiple sockets.
Cc: linuxppc-dev@lists.ozlabs.org
Cc: qemu-ppc@nongnu.org
Cc: Cedric Le Goater <clg@kaod.org>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Reported-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/topology.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index 3beeb030cd78..e4db64c0e184 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -126,7 +126,7 @@ static inline int cpu_to_coregroup_id(int cpu)
#define topology_physical_package_id(cpu) (cpu_to_chip_id(cpu))
#define topology_sibling_cpumask(cpu) (per_cpu(cpu_sibling_map, cpu))
-#define topology_core_cpumask(cpu) (cpu_cpu_mask(cpu))
+#define topology_core_cpumask(cpu) (per_cpu(cpu_core_map, cpu))
#define topology_core_id(cpu) (cpu_to_core_id(cpu))
#endif
--
2.25.1
^ permalink raw reply related
* [PATCH 3/3] powerpc/smp: Cache CPU to chip lookup
From: Srikar Dronamraju @ 2021-04-15 12:09 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Srikar Dronamraju, Peter Zijlstra,
Daniel Henrique Barboza, Valentin Schneider, qemu-ppc,
Cedric Le Goater, linuxppc-dev, Ingo Molnar, David Gibson
In-Reply-To: <20210415120934.232271-1-srikar@linux.vnet.ibm.com>
On systems with large CPUs per node, even with the filtered matching of
related CPUs, there can be large number of calls to cpu_to_chip_id for
the same CPU. For example with 4096 vCPU, 1 node QEMU configuration,
with 4 threads per core, system could be see upto 1024 calls to
cpu_to_chip_id() for the same CPU. On a given system, cpu_to_chip_id()
for a given CPU would always return the same. Hence cache the result in
a lookup table for use in subsequent calls.
Since all CPUs sharing the same core will belong to the same chip, the
lookup_table has an entry for one CPU per core. chip_id_lookup_table is
not being freed and would be used on subsequent CPU online post CPU
offline.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: qemu-ppc@nongnu.org
Cc: Cedric Le Goater <clg@kaod.org>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Reported-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/smp.h | 1 +
arch/powerpc/kernel/prom.c | 19 +++++++++++++++----
arch/powerpc/kernel/smp.c | 21 +++++++++++++++++++--
3 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index 47081a9e13ca..03b3d010cbab 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -31,6 +31,7 @@ extern u32 *cpu_to_phys_id;
extern bool coregroup_enabled;
extern int cpu_to_chip_id(int cpu);
+extern int *chip_id_lookup_table;
#ifdef CONFIG_SMP
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 9a4797d1d40d..6d2e4a5bc471 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -65,6 +65,8 @@
#define DBG(fmt...)
#endif
+int *chip_id_lookup_table;
+
#ifdef CONFIG_PPC64
int __initdata iommu_is_off;
int __initdata iommu_force_on;
@@ -914,13 +916,22 @@ EXPORT_SYMBOL(of_get_ibm_chip_id);
int cpu_to_chip_id(int cpu)
{
struct device_node *np;
+ int ret = -1, idx;
+
+ idx = cpu / threads_per_core;
+ if (chip_id_lookup_table && chip_id_lookup_table[idx] != -1)
+ return chip_id_lookup_table[idx];
np = of_get_cpu_node(cpu, NULL);
- if (!np)
- return -1;
+ if (np) {
+ ret = of_get_ibm_chip_id(np);
+ of_node_put(np);
+
+ if (chip_id_lookup_table)
+ chip_id_lookup_table[idx] = ret;
+ }
- of_node_put(np);
- return of_get_ibm_chip_id(np);
+ return ret;
}
EXPORT_SYMBOL(cpu_to_chip_id);
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 5c7ce1d50631..50520fbea424 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1073,6 +1073,20 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
cpu_smallcore_mask(boot_cpuid));
}
+ if (cpu_to_chip_id(boot_cpuid) != -1) {
+ int idx = num_possible_cpus() / threads_per_core;
+
+ /*
+ * All threads of a core will all belong to the same core,
+ * chip_id_lookup_table will have one entry per core.
+ * Assumption: if boot_cpuid doesn't have a chip-id, then no
+ * other CPUs, will also not have chip-id.
+ */
+ chip_id_lookup_table = kcalloc(idx, sizeof(int), GFP_KERNEL);
+ if (chip_id_lookup_table)
+ memset(chip_id_lookup_table, -1, sizeof(int) * idx);
+ }
+
if (smp_ops && smp_ops->probe)
smp_ops->probe();
}
@@ -1468,8 +1482,8 @@ static void add_cpu_to_masks(int cpu)
{
struct cpumask *(*submask_fn)(int) = cpu_sibling_mask;
int first_thread = cpu_first_thread_sibling(cpu);
- int chip_id = cpu_to_chip_id(cpu);
cpumask_var_t mask;
+ int chip_id = -1;
bool ret;
int i;
@@ -1492,7 +1506,10 @@ static void add_cpu_to_masks(int cpu)
if (has_coregroup_support())
update_coregroup_mask(cpu, &mask);
- if (chip_id == -1 || !ret) {
+ if (chip_id_lookup_table && ret)
+ chip_id = cpu_to_chip_id(cpu);
+
+ if (chip_id == -1) {
cpumask_copy(per_cpu(cpu_core_map, cpu), cpu_cpu_mask(cpu));
goto out;
}
--
2.25.1
^ permalink raw reply related
* [PATCH 1/3] powerpc/smp: Reintroduce cpu_core_mask
From: Srikar Dronamraju @ 2021-04-15 12:09 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Gautham R Shenoy, Srikar Dronamraju, Peter Zijlstra,
Daniel Henrique Barboza, Valentin Schneider, qemu-ppc,
Cedric Le Goater, linuxppc-dev, Ingo Molnar, David Gibson
In-Reply-To: <20210415120934.232271-1-srikar@linux.vnet.ibm.com>
Daniel reported that with Commit 4ca234a9cbd7 ("powerpc/smp: Stop
updating cpu_core_mask") QEMU was unable to set single NUMA node SMP
topologies such as:
-smp 8,maxcpus=8,cores=2,threads=2,sockets=2
i.e he expected 2 sockets in one NUMA node.
The above commit helped to reduce boot time on Large Systems for
example 4096 vCPU single socket QEMU instance. PAPR is silent on
having more than one socket within a NUMA node.
cpu_core_mask and cpu_cpu_mask for any CPU would be same unless the
number of sockets is different from the number of NUMA nodes.
One option is to reintroduce cpu_core_mask but use a slightly
different method to arrive at the cpu_core_mask. Previously each CPU's
chip-id would be compared with all other CPU's chip-id to verify if
both the CPUs were related at the chip level. Now if a CPU 'A' is
found related / (unrelated) to another CPU 'B', all the thread
siblings of 'A' and thread siblings of 'B' are automatically marked as
related / (unrelated).
Also if a platform doesn't support ibm,chip-id property, i.e its
cpu_to_chip_id returns -1, cpu_core_map holds a copy of
cpu_cpu_mask().
Fixes: 4ca234a9cbd7 ("powerpc/smp: Stop updating cpu_core_mask")
Cc: linuxppc-dev@lists.ozlabs.org
Cc: qemu-ppc@nongnu.org
Cc: Cedric Le Goater <clg@kaod.org>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Valentin Schneider <valentin.schneider@arm.com>
Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
Reported-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/smp.h | 5 +++++
arch/powerpc/kernel/smp.c | 39 ++++++++++++++++++++++++++++------
2 files changed, 37 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index 7a13bc20f0a0..47081a9e13ca 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -121,6 +121,11 @@ static inline struct cpumask *cpu_sibling_mask(int cpu)
return per_cpu(cpu_sibling_map, cpu);
}
+static inline struct cpumask *cpu_core_mask(int cpu)
+{
+ return per_cpu(cpu_core_map, cpu);
+}
+
static inline struct cpumask *cpu_l2_cache_mask(int cpu)
{
return per_cpu(cpu_l2_cache_map, cpu);
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 5a4d59a1070d..5c7ce1d50631 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1057,17 +1057,12 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
local_memory_node(numa_cpu_lookup_table[cpu]));
}
#endif
- /*
- * cpu_core_map is now more updated and exists only since
- * its been exported for long. It only will have a snapshot
- * of cpu_cpu_mask.
- */
- cpumask_copy(per_cpu(cpu_core_map, cpu), cpu_cpu_mask(cpu));
}
/* Init the cpumasks so the boot CPU is related to itself */
cpumask_set_cpu(boot_cpuid, cpu_sibling_mask(boot_cpuid));
cpumask_set_cpu(boot_cpuid, cpu_l2_cache_mask(boot_cpuid));
+ cpumask_set_cpu(boot_cpuid, cpu_core_mask(boot_cpuid));
if (has_coregroup_support())
cpumask_set_cpu(boot_cpuid, cpu_coregroup_mask(boot_cpuid));
@@ -1408,6 +1403,9 @@ static void remove_cpu_from_masks(int cpu)
set_cpus_unrelated(cpu, i, cpu_smallcore_mask);
}
+ for_each_cpu(i, cpu_core_mask(cpu))
+ set_cpus_unrelated(cpu, i, cpu_core_mask);
+
if (has_coregroup_support()) {
for_each_cpu(i, cpu_coregroup_mask(cpu))
set_cpus_unrelated(cpu, i, cpu_coregroup_mask);
@@ -1468,8 +1466,11 @@ static void update_coregroup_mask(int cpu, cpumask_var_t *mask)
static void add_cpu_to_masks(int cpu)
{
+ struct cpumask *(*submask_fn)(int) = cpu_sibling_mask;
int first_thread = cpu_first_thread_sibling(cpu);
+ int chip_id = cpu_to_chip_id(cpu);
cpumask_var_t mask;
+ bool ret;
int i;
/*
@@ -1485,12 +1486,36 @@ static void add_cpu_to_masks(int cpu)
add_cpu_to_smallcore_masks(cpu);
/* In CPU-hotplug path, hence use GFP_ATOMIC */
- alloc_cpumask_var_node(&mask, GFP_ATOMIC, cpu_to_node(cpu));
+ ret = alloc_cpumask_var_node(&mask, GFP_ATOMIC, cpu_to_node(cpu));
update_mask_by_l2(cpu, &mask);
if (has_coregroup_support())
update_coregroup_mask(cpu, &mask);
+ if (chip_id == -1 || !ret) {
+ cpumask_copy(per_cpu(cpu_core_map, cpu), cpu_cpu_mask(cpu));
+ goto out;
+ }
+
+ if (shared_caches)
+ submask_fn = cpu_l2_cache_mask;
+
+ /* Update core_mask with all the CPUs that are part of submask */
+ or_cpumasks_related(cpu, cpu, submask_fn, cpu_core_mask);
+
+ /* Skip all CPUs already part of current CPU core mask */
+ cpumask_andnot(mask, cpu_online_mask, cpu_core_mask(cpu));
+
+ for_each_cpu(i, mask) {
+ if (chip_id == cpu_to_chip_id(i)) {
+ or_cpumasks_related(cpu, i, submask_fn, cpu_core_mask);
+ cpumask_andnot(mask, mask, submask_fn(i));
+ } else {
+ cpumask_andnot(mask, mask, cpu_core_mask(i));
+ }
+ }
+
+out:
free_cpumask_var(mask);
}
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 0/3] Reintroduce cpu_core_mask
From: Daniel Henrique Barboza @ 2021-04-15 12:17 UTC (permalink / raw)
To: Srikar Dronamraju, Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <20210415120934.232271-1-srikar@linux.vnet.ibm.com>
Hi,
Using a QEMU pseries guest with this follwing SMP topology, with a
single NUMA node:
(...) -smp 32,threads=4,cores=4,sockets=2, (...)
This is the output of lscpu with a guest running v5.12-rc5:
[root@localhost ~]# lscpu
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 32
On-line CPU(s) list: 0-31
Thread(s) per core: 4
Core(s) per socket: 8
Socket(s): 1
NUMA node(s): 1
Model: 2.2 (pvr 004e 1202)
Model name: POWER9 (architected), altivec supported
Hypervisor vendor: KVM
Virtualization type: para
L1d cache: 32K
L1i cache: 32K
NUMA node0 CPU(s): 0-31
[root@localhost ~]#
The changes with cpu_core_mask made the topology sockets matching NUMA nodes.
In this case, given that we have a single NUMA node, the SMP topology got
adjusted to have 8 cores instead of 4 so we can have a single socket as well.
Although sockets equal to NUMA nodes is true for Power hardware, QEMU doesn't
have this constraint and users expect sockets and NUMA nodes to be kind of
independent, regardless of how unpractical that would be with real hardware.
The same guest running a kernel with this series applied:
[root@localhost ~]# lscpu
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 32
On-line CPU(s) list: 0-31
Thread(s) per core: 4
Core(s) per socket: 4
Socket(s): 2
NUMA node(s): 1
Model: 2.2 (pvr 004e 1202)
Model name: POWER9 (architected), altivec supported
Hypervisor vendor: KVM
Virtualization type: para
L1d cache: 32K
L1i cache: 32K
NUMA node0 CPU(s): 0-31
The sockets and NUMA nodes are being represented separately, as intended via
the QEMU command line.
Thanks for the looking this up, Srikar. For all patches:
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
On 4/15/21 9:09 AM, Srikar Dronamraju wrote:
> Daniel had reported that
> QEMU is now unable to see requested topologies in a multi socket single
> NUMA node configurations.
> -smp 8,maxcpus=8,cores=2,threads=2,sockets=2
>
> This patchset reintroduces cpu_core_mask so that users can see requested
> topologies while still maintaining the boot time of very large system
> configurations.
>
> It includes caching the chip_id as suggested by Michael Ellermann
>
> 4 Threads/Core; 4 cores/Socket; 4 Sockets/Node, 2 Nodes in System
> -numa node,nodeid=0,memdev=m0 \
> -numa node,nodeid=1,memdev=m1 \
> -smp 128,sockets=8,threads=4,maxcpus=128 \
>
> 5.12.0-rc5 (or any kernel with commit 4ca234a9cbd7)
> ---------------------------------------------------
> srikar@cloudy:~$ lscpu
> Architecture: ppc64le
> Byte Order: Little Endian
> CPU(s): 128
> On-line CPU(s) list: 0-127
> Thread(s) per core: 4
> Core(s) per socket: 16
> Socket(s): 2 <<<<<-----
> NUMA node(s): 2
> Model: 2.3 (pvr 004e 1203)
> Model name: POWER9 (architected), altivec supported
> Hypervisor vendor: KVM
> Virtualization type: para
> L1d cache: 1 MiB
> L1i cache: 1 MiB
> NUMA node0 CPU(s): 0-15,32-47,64-79,96-111
> NUMA node1 CPU(s): 16-31,48-63,80-95,112-127
> --
> srikar@cloudy:~$ dmesg |grep smp
> [ 0.010658] smp: Bringing up secondary CPUs ...
> [ 0.424681] smp: Brought up 2 nodes, 128 CPUs
> --
>
> 5.12.0-rc5 + 3 patches
> ----------------------
> srikar@cloudy:~$ lscpu
> Architecture: ppc64le
> Byte Order: Little Endian
> CPU(s): 128
> On-line CPU(s) list: 0-127
> Thread(s) per core: 4
> Core(s) per socket: 4
> Socket(s): 8 <<<<-----
> NUMA node(s): 2
> Model: 2.3 (pvr 004e 1203)
> Model name: POWER9 (architected), altivec supported
> Hypervisor vendor: KVM
> Virtualization type: para
> L1d cache: 1 MiB
> L1i cache: 1 MiB
> NUMA node0 CPU(s): 0-15,32-47,64-79,96-111
> NUMA node1 CPU(s): 16-31,48-63,80-95,112-127
> --
> srikar@cloudy:~$ dmesg |grep smp
> [ 0.010372] smp: Bringing up secondary CPUs ...
> [ 0.417892] smp: Brought up 2 nodes, 128 CPUs
>
> 5.12.0-rc5
> ----------
> srikar@cloudy:~$ lscpu
> Architecture: ppc64le
> Byte Order: Little Endian
> CPU(s): 1024
> On-line CPU(s) list: 0-1023
> Thread(s) per core: 8
> Core(s) per socket: 128
> Socket(s): 1
> NUMA node(s): 1
> Model: 2.3 (pvr 004e 1203)
> Model name: POWER9 (architected), altivec supported
> Hypervisor vendor: KVM
> Virtualization type: para
> L1d cache: 4 MiB
> L1i cache: 4 MiB
> NUMA node0 CPU(s): 0-1023
> srikar@cloudy:~$ dmesg | grep smp
> [ 0.027753 ] smp: Bringing up secondary CPUs ...
> [ 2.315193 ] smp: Brought up 1 node, 1024 CPUs
>
> 5.12.0-rc5 + 3 patches
> ----------------------
> srikar@cloudy:~$ dmesg | grep smp
> [ 0.027659 ] smp: Bringing up secondary CPUs ...
> [ 2.532739 ] smp: Brought up 1 node, 1024 CPUs
>
> I also have booted and tested the kernels on PowerVM and PowerNV and
> even there I see a very negligible increase in the bringing up time of
> secondary CPUs
>
> Srikar Dronamraju (3):
> powerpc/smp: Reintroduce cpu_core_mask
> Revert "powerpc/topology: Update topology_core_cpumask"
> powerpc/smp: Cache CPU to chip lookup
>
> arch/powerpc/include/asm/smp.h | 6 ++++
> arch/powerpc/include/asm/topology.h | 2 +-
> arch/powerpc/kernel/prom.c | 19 +++++++---
> arch/powerpc/kernel/smp.c | 56 +++++++++++++++++++++++++----
> 4 files changed, 71 insertions(+), 12 deletions(-)
>
^ permalink raw reply
* Re: consolidate the flock uapi definitions
From: Heiko Carstens @ 2021-04-15 12:20 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, linux-s390, Thomas Bogendoerfer, Arnd Bergmann,
Vasily Gorbik, linuxppc-dev, Catalin Marinas, Helge Deller, x86,
linux-kernel, linux-mips, James E.J. Bottomley,
Christian Borntraeger, linux-parisc, sparclinux, Will Deacon,
David S. Miller, linux-arm-kernel
In-Reply-To: <20210412085545.2595431-1-hch@lst.de>
On Mon, Apr 12, 2021 at 10:55:40AM +0200, Christoph Hellwig wrote:
> Hi all,
>
> currently we deal with the slight differents in the various architecture
> variants of the flock and flock64 stuctures in a very cruft way. This
> series switches to just use small arch hooks and define the rest in
> asm-generic and linux/compat.h instead.
>
> Diffstat:
> arch/arm64/include/asm/compat.h | 20 --------------------
> arch/mips/include/asm/compat.h | 23 ++---------------------
> arch/mips/include/uapi/asm/fcntl.h | 28 +++-------------------------
> arch/parisc/include/asm/compat.h | 16 ----------------
> arch/powerpc/include/asm/compat.h | 20 --------------------
> arch/s390/include/asm/compat.h | 20 --------------------
> arch/sparc/include/asm/compat.h | 22 +---------------------
> arch/x86/include/asm/compat.h | 24 +++---------------------
> include/linux/compat.h | 31 +++++++++++++++++++++++++++++++
> include/uapi/asm-generic/fcntl.h | 21 +++++++--------------
> tools/include/uapi/asm-generic/fcntl.h | 21 +++++++--------------
> 11 files changed, 54 insertions(+), 192 deletions(-)
for the s390 bits:
Acked-by: Heiko Carstens <hca@linux.ibm.com>
^ permalink raw reply
* [PATCH v1 1/1] powerpc/papr_scm: Properly handle UUID types and API
From: Andy Shevchenko @ 2021-04-15 13:46 UTC (permalink / raw)
To: Vaibhav Jain, linuxppc-dev, linux-kernel
Cc: Oliver O'Halloran, Aneesh Kumar K . V, Paul Mackerras,
Andy Shevchenko
Parse to and export from UUID own type, before dereferencing.
This also fixes wrong comment (Little Endian UUID is something else)
and should fix Sparse warnings about assigning strict types to POD.
Fixes: 43001c52b603 ("powerpc/papr_scm: Use ibm,unit-guid as the iset cookie")
Fixes: 259a948c4ba1 ("powerpc/pseries/scm: Use a specific endian format for storing uuid from the device tree")
Cc: Oliver O'Halloran <oohall@gmail.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Not tested
arch/powerpc/platforms/pseries/papr_scm.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index ae6f5d80d5ce..4366e1902890 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -1085,8 +1085,9 @@ static int papr_scm_probe(struct platform_device *pdev)
u32 drc_index, metadata_size;
u64 blocks, block_size;
struct papr_scm_priv *p;
+ u8 uuid_raw[UUID_SIZE];
const char *uuid_str;
- u64 uuid[2];
+ uuid_t uuid;
int rc;
/* check we have all the required DT properties */
@@ -1129,16 +1130,18 @@ static int papr_scm_probe(struct platform_device *pdev)
p->hcall_flush_required = of_property_read_bool(dn, "ibm,hcall-flush-required");
/* We just need to ensure that set cookies are unique across */
- uuid_parse(uuid_str, (uuid_t *) uuid);
+ uuid_parse(uuid_str, &uuid);
+
/*
* cookie1 and cookie2 are not really little endian
- * we store a little endian representation of the
+ * we store a raw buffer representation of the
* uuid str so that we can compare this with the label
* area cookie irrespective of the endian config with which
* the kernel is built.
*/
- p->nd_set.cookie1 = cpu_to_le64(uuid[0]);
- p->nd_set.cookie2 = cpu_to_le64(uuid[1]);
+ export_uuid(uuid_raw, &uuid);
+ p->nd_set.cookie1 = get_unaligned_le64(&uuid_raw[0]);
+ p->nd_set.cookie2 = get_unaligned_le64(&uuid_raw[8]);
/* might be zero */
p->metadata_size = metadata_size;
--
2.30.2
^ permalink raw reply related
* Re: [PATCH] powerpc/papr_scm: Reduce error severity if nvdimm stats inaccessible
From: Dan Williams @ 2021-04-15 17:03 UTC (permalink / raw)
To: Vaibhav Jain
Cc: Aneesh Kumar K . V, Santosh Sivaraj, Ira Weiny, linuxppc-dev,
linux-nvdimm
In-Reply-To: <87k0p3lqmq.fsf@vajain21.in.ibm.com>
On Thu, Apr 15, 2021 at 4:44 AM Vaibhav Jain <vaibhav@linux.ibm.com> wrote:
>
> Thanks for looking into this Dan,
>
> Dan Williams <dan.j.williams@intel.com> writes:
>
> > On Wed, Apr 14, 2021 at 5:40 AM Vaibhav Jain <vaibhav@linux.ibm.com> wrote:
> >>
> >> Currently drc_pmem_qeury_stats() generates a dev_err in case
> >> "Enable Performance Information Collection" feature is disabled from
> >> HMC. The error is of the form below:
> >>
> >> papr_scm ibm,persistent-memory:ibm,pmemory@44104001: Failed to query
> >> performance stats, Err:-10
> >>
> >> This error message confuses users as it implies a possible problem
> >> with the nvdimm even though its due to a disabled feature.
> >>
> >> So we fix this by explicitly handling the H_AUTHORITY error from the
> >> H_SCM_PERFORMANCE_STATS hcall and generating a warning instead of an
> >> error, saying that "Performance stats in-accessible".
> >>
> >> Fixes: 2d02bf835e57('powerpc/papr_scm: Fetch nvdimm performance stats from PHYP')
> >> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> >> ---
> >> arch/powerpc/platforms/pseries/papr_scm.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> >> index 835163f54244..9216424f8be3 100644
> >> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> >> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> >> @@ -277,6 +277,9 @@ static ssize_t drc_pmem_query_stats(struct papr_scm_priv *p,
> >> dev_err(&p->pdev->dev,
> >> "Unknown performance stats, Err:0x%016lX\n", ret[0]);
> >> return -ENOENT;
> >> + } else if (rc == H_AUTHORITY) {
> >> + dev_warn(&p->pdev->dev, "Performance stats in-accessible");
> >> + return -EPERM;
> >
> > So userspace can spam the kernel log? Why is kernel log message needed
> > at all? EPERM told the caller what happened.
> Currently this error message is only reported during probe of the
> nvdimm. So userspace cannot directly spam kernel log.
Oh, ok, I saw things like papr_pdsm_fuel_gauge() in the call stack and
thought this was reachable through an ioctl. Sorry for the noise.
^ permalink raw reply
* Re: [PATCH v1 1/1] powerpc/papr_scm: Properly handle UUID types and API
From: Vaibhav Jain @ 2021-04-15 17:09 UTC (permalink / raw)
To: Andy Shevchenko, linuxppc-dev, linux-kernel
Cc: Oliver O'Halloran, Aneesh Kumar K . V, Paul Mackerras,
Andy Shevchenko
In-Reply-To: <20210415134637.17770-1-andriy.shevchenko@linux.intel.com>
Thanks for the patch Andy,
Unfortunately ran into a compilation issue due to missing "#include
<asm/unaligned.h>" that provides definition for
get_unaligned_le64(). Gcc reported following error:
error: implicit declaration of function ‘get_unaligned_le64’
After including the necessary header file, kernel compiled fine and I
was able to test & verify the patch.
--
Cheers
~ Vaibhav
Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:
> Parse to and export from UUID own type, before dereferencing.
> This also fixes wrong comment (Little Endian UUID is something else)
> and should fix Sparse warnings about assigning strict types to POD.
>
> Fixes: 43001c52b603 ("powerpc/papr_scm: Use ibm,unit-guid as the iset cookie")
> Fixes: 259a948c4ba1 ("powerpc/pseries/scm: Use a specific endian format for storing uuid from the device tree")
> Cc: Oliver O'Halloran <oohall@gmail.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> Not tested
> arch/powerpc/platforms/pseries/papr_scm.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index ae6f5d80d5ce..4366e1902890 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -1085,8 +1085,9 @@ static int papr_scm_probe(struct platform_device *pdev)
> u32 drc_index, metadata_size;
> u64 blocks, block_size;
> struct papr_scm_priv *p;
> + u8 uuid_raw[UUID_SIZE];
> const char *uuid_str;
> - u64 uuid[2];
> + uuid_t uuid;
> int rc;
>
> /* check we have all the required DT properties */
> @@ -1129,16 +1130,18 @@ static int papr_scm_probe(struct platform_device *pdev)
> p->hcall_flush_required = of_property_read_bool(dn, "ibm,hcall-flush-required");
>
> /* We just need to ensure that set cookies are unique across */
> - uuid_parse(uuid_str, (uuid_t *) uuid);
> + uuid_parse(uuid_str, &uuid);
> +
> /*
> * cookie1 and cookie2 are not really little endian
> - * we store a little endian representation of the
> + * we store a raw buffer representation of the
> * uuid str so that we can compare this with the label
> * area cookie irrespective of the endian config with which
> * the kernel is built.
> */
> - p->nd_set.cookie1 = cpu_to_le64(uuid[0]);
> - p->nd_set.cookie2 = cpu_to_le64(uuid[1]);
> + export_uuid(uuid_raw, &uuid);
> + p->nd_set.cookie1 = get_unaligned_le64(&uuid_raw[0]);
> + p->nd_set.cookie2 = get_unaligned_le64(&uuid_raw[8]);
>
> /* might be zero */
> p->metadata_size = metadata_size;
> --
> 2.30.2
>
^ permalink raw reply
* Re: [PATCH 1/3] powerpc/smp: Reintroduce cpu_core_mask
From: Gautham R Shenoy @ 2021-04-15 17:11 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Nathan Lynch, Gautham R Shenoy, Peter Zijlstra,
Daniel Henrique Barboza, Valentin Schneider, qemu-ppc,
Cedric Le Goater, linuxppc-dev, Ingo Molnar, David Gibson
In-Reply-To: <20210415120934.232271-2-srikar@linux.vnet.ibm.com>
Hi Srikar,
On Thu, Apr 15, 2021 at 05:39:32PM +0530, Srikar Dronamraju wrote:
[..snip..]
> @@ -1485,12 +1486,36 @@ static void add_cpu_to_masks(int cpu)
> add_cpu_to_smallcore_masks(cpu);
>
> /* In CPU-hotplug path, hence use GFP_ATOMIC */
> - alloc_cpumask_var_node(&mask, GFP_ATOMIC, cpu_to_node(cpu));
> + ret = alloc_cpumask_var_node(&mask, GFP_ATOMIC, cpu_to_node(cpu));
> update_mask_by_l2(cpu, &mask);
>
> if (has_coregroup_support())
> update_coregroup_mask(cpu, &mask);
>
> + if (chip_id == -1 || !ret) {
> + cpumask_copy(per_cpu(cpu_core_map, cpu), cpu_cpu_mask(cpu));
> + goto out;
> + }
> +
> + if (shared_caches)
> + submask_fn = cpu_l2_cache_mask;
> +
> + /* Update core_mask with all the CPUs that are part of submask */
> + or_cpumasks_related(cpu, cpu, submask_fn, cpu_core_mask);
>
If coregroups exist, we can add the cpus of the coregroup to the
cpu_core_mask thereby reducing the scope of the for_each_cpu() search
below. This will still cut down the time on Baremetal systems
supporting coregroups.
> + /* Skip all CPUs already part of current CPU core mask */
> + cpumask_andnot(mask, cpu_online_mask, cpu_core_mask(cpu));
> +
> + for_each_cpu(i, mask) {
> + if (chip_id == cpu_to_chip_id(i)) {
> + or_cpumasks_related(cpu, i, submask_fn, cpu_core_mask);
> + cpumask_andnot(mask, mask, submask_fn(i));
> + } else {
> + cpumask_andnot(mask, mask, cpu_core_mask(i));
> + }
> + }
> +
> +out:
> free_cpumask_var(mask);
> }
>
> --
> 2.25.1
>
^ permalink raw reply
* [PATCH v1 4/5] mm: ptdump: Support hugepd table entries
From: Christophe Leroy @ 2021-04-15 17:18 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Steven Price, akpm
Cc: linux-arch, linux-s390, x86, linux-kernel, linux-mm, linux-riscv,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <cover.1618506910.git.christophe.leroy@csgroup.eu>
Which hugepd, page table entries can be at any level
and can be of any size.
Add support for them.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
mm/ptdump.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/mm/ptdump.c b/mm/ptdump.c
index 61cd16afb1c8..6efdb8c15a7d 100644
--- a/mm/ptdump.c
+++ b/mm/ptdump.c
@@ -112,11 +112,24 @@ static int ptdump_pte_entry(pte_t *pte, unsigned long addr,
{
struct ptdump_state *st = walk->private;
pte_t val = ptep_get(pte);
+ unsigned long page_size = next - addr;
+ int level;
+
+ if (page_size >= PGDIR_SIZE)
+ level = 0;
+ else if (page_size >= P4D_SIZE)
+ level = 1;
+ else if (page_size >= PUD_SIZE)
+ level = 2;
+ else if (page_size >= PMD_SIZE)
+ level = 3;
+ else
+ level = 4;
if (st->effective_prot)
- st->effective_prot(st, 4, pte_val(val));
+ st->effective_prot(st, level, pte_val(val));
- st->note_page(st, addr, 4, pte_val(val), PAGE_SIZE);
+ st->note_page(st, addr, level, pte_val(val), page_size);
return 0;
}
--
2.25.0
^ permalink raw reply related
* [PATCH v1 0/5] Convert powerpc to GENERIC_PTDUMP
From: Christophe Leroy @ 2021-04-15 17:18 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Steven Price, akpm
Cc: linux-arch, linux-s390, x86, linux-kernel, linux-mm, linux-riscv,
linuxppc-dev, linux-arm-kernel
This series converts powerpc to generic PTDUMP.
For that, we first need to add missing hugepd support
to pagewalk and ptdump.
Christophe Leroy (5):
mm: pagewalk: Fix walk for hugepage tables
mm: ptdump: Fix build failure
mm: ptdump: Provide page size to notepage()
mm: ptdump: Support hugepd table entries
powerpc/mm: Convert powerpc to GENERIC_PTDUMP
arch/arm64/mm/ptdump.c | 2 +-
arch/powerpc/Kconfig | 2 +
arch/powerpc/Kconfig.debug | 30 ------
arch/powerpc/mm/Makefile | 2 +-
arch/powerpc/mm/mmu_decl.h | 2 +-
arch/powerpc/mm/ptdump/8xx.c | 6 +-
arch/powerpc/mm/ptdump/Makefile | 9 +-
arch/powerpc/mm/ptdump/book3s64.c | 6 +-
arch/powerpc/mm/ptdump/ptdump.c | 161 +++++++++---------------------
arch/powerpc/mm/ptdump/shared.c | 6 +-
arch/riscv/mm/ptdump.c | 2 +-
arch/s390/mm/dump_pagetables.c | 3 +-
arch/x86/mm/dump_pagetables.c | 2 +-
include/linux/ptdump.h | 2 +-
mm/pagewalk.c | 54 ++++++++--
mm/ptdump.c | 33 ++++--
16 files changed, 145 insertions(+), 177 deletions(-)
--
2.25.0
^ permalink raw reply
* [PATCH v1 1/5] mm: pagewalk: Fix walk for hugepage tables
From: Christophe Leroy @ 2021-04-15 17:18 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Steven Price, akpm
Cc: linux-arch, linux-s390, x86, linux-kernel, linux-mm, linux-riscv,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <cover.1618506910.git.christophe.leroy@csgroup.eu>
Pagewalk ignores hugepd entries and walk down the tables
as if it was traditionnal entries, leading to crazy result.
Add walk_hugepd_range() and use it to walk hugepage tables.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
mm/pagewalk.c | 54 +++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 48 insertions(+), 6 deletions(-)
diff --git a/mm/pagewalk.c b/mm/pagewalk.c
index e81640d9f177..410a9d8f7572 100644
--- a/mm/pagewalk.c
+++ b/mm/pagewalk.c
@@ -58,6 +58,32 @@ static int walk_pte_range(pmd_t *pmd, unsigned long addr, unsigned long end,
return err;
}
+static int walk_hugepd_range(hugepd_t *phpd, unsigned long addr,
+ unsigned long end, struct mm_walk *walk, int pdshift)
+{
+ int err = 0;
+#ifdef CONFIG_ARCH_HAS_HUGEPD
+ const struct mm_walk_ops *ops = walk->ops;
+ int shift = hugepd_shift(*phpd);
+ int page_size = 1 << shift;
+
+ if (addr & (page_size - 1))
+ return 0;
+
+ for (;;) {
+ pte_t *pte = hugepte_offset(*phpd, addr, pdshift);
+
+ err = ops->pte_entry(pte, addr, addr + page_size, walk);
+ if (err)
+ break;
+ if (addr >= end - page_size)
+ break;
+ addr += page_size;
+ }
+#endif
+ return err;
+}
+
static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end,
struct mm_walk *walk)
{
@@ -108,7 +134,10 @@ static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end,
goto again;
}
- err = walk_pte_range(pmd, addr, next, walk);
+ if (is_hugepd(__hugepd(pmd_val(*pmd))))
+ err = walk_hugepd_range((hugepd_t *)pmd, addr, next, walk, PMD_SHIFT);
+ else
+ err = walk_pte_range(pmd, addr, next, walk);
if (err)
break;
} while (pmd++, addr = next, addr != end);
@@ -157,7 +186,10 @@ static int walk_pud_range(p4d_t *p4d, unsigned long addr, unsigned long end,
if (pud_none(*pud))
goto again;
- err = walk_pmd_range(pud, addr, next, walk);
+ if (is_hugepd(__hugepd(pud_val(*pud))))
+ err = walk_hugepd_range((hugepd_t *)pud, addr, next, walk, PUD_SHIFT);
+ else
+ err = walk_pmd_range(pud, addr, next, walk);
if (err)
break;
} while (pud++, addr = next, addr != end);
@@ -189,8 +221,13 @@ static int walk_p4d_range(pgd_t *pgd, unsigned long addr, unsigned long end,
if (err)
break;
}
- if (ops->pud_entry || ops->pmd_entry || ops->pte_entry)
- err = walk_pud_range(p4d, addr, next, walk);
+ if (ops->pud_entry || ops->pmd_entry || ops->pte_entry) {
+ if (is_hugepd(__hugepd(p4d_val(*p4d))))
+ err = walk_hugepd_range((hugepd_t *)p4d, addr, next, walk,
+ P4D_SHIFT);
+ else
+ err = walk_pud_range(p4d, addr, next, walk);
+ }
if (err)
break;
} while (p4d++, addr = next, addr != end);
@@ -225,8 +262,13 @@ static int walk_pgd_range(unsigned long addr, unsigned long end,
break;
}
if (ops->p4d_entry || ops->pud_entry || ops->pmd_entry ||
- ops->pte_entry)
- err = walk_p4d_range(pgd, addr, next, walk);
+ ops->pte_entry) {
+ if (is_hugepd(__hugepd(pgd_val(*pgd))))
+ err = walk_hugepd_range((hugepd_t *)pgd, addr, next, walk,
+ PGDIR_SHIFT);
+ else
+ err = walk_p4d_range(pgd, addr, next, walk);
+ }
if (err)
break;
} while (pgd++, addr = next, addr != end);
--
2.25.0
^ permalink raw reply related
* [PATCH v1 3/5] mm: ptdump: Provide page size to notepage()
From: Christophe Leroy @ 2021-04-15 17:18 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Steven Price, akpm
Cc: linux-arch, linux-s390, x86, linux-kernel, linux-mm, linux-riscv,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <cover.1618506910.git.christophe.leroy@csgroup.eu>
In order to support large pages on powerpc, notepage()
needs to know the page size of the page.
Add a page_size argument to notepage().
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/arm64/mm/ptdump.c | 2 +-
arch/riscv/mm/ptdump.c | 2 +-
arch/s390/mm/dump_pagetables.c | 3 ++-
arch/x86/mm/dump_pagetables.c | 2 +-
include/linux/ptdump.h | 2 +-
mm/ptdump.c | 16 ++++++++--------
6 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/mm/ptdump.c b/arch/arm64/mm/ptdump.c
index 0e050d76b83a..ea1a1c3a3ea0 100644
--- a/arch/arm64/mm/ptdump.c
+++ b/arch/arm64/mm/ptdump.c
@@ -257,7 +257,7 @@ static void note_prot_wx(struct pg_state *st, unsigned long addr)
}
static void note_page(struct ptdump_state *pt_st, unsigned long addr, int level,
- u64 val)
+ u64 val, unsigned long page_size)
{
struct pg_state *st = container_of(pt_st, struct pg_state, ptdump);
static const char units[] = "KMGTPE";
diff --git a/arch/riscv/mm/ptdump.c b/arch/riscv/mm/ptdump.c
index ace74dec7492..0a7f276ba799 100644
--- a/arch/riscv/mm/ptdump.c
+++ b/arch/riscv/mm/ptdump.c
@@ -235,7 +235,7 @@ static void note_prot_wx(struct pg_state *st, unsigned long addr)
}
static void note_page(struct ptdump_state *pt_st, unsigned long addr,
- int level, u64 val)
+ int level, u64 val, unsigned long page_size)
{
struct pg_state *st = container_of(pt_st, struct pg_state, ptdump);
u64 pa = PFN_PHYS(pte_pfn(__pte(val)));
diff --git a/arch/s390/mm/dump_pagetables.c b/arch/s390/mm/dump_pagetables.c
index e40a30647d99..29673c38e773 100644
--- a/arch/s390/mm/dump_pagetables.c
+++ b/arch/s390/mm/dump_pagetables.c
@@ -116,7 +116,8 @@ static void note_prot_wx(struct pg_state *st, unsigned long addr)
#endif /* CONFIG_DEBUG_WX */
}
-static void note_page(struct ptdump_state *pt_st, unsigned long addr, int level, u64 val)
+static void note_page(struct ptdump_state *pt_st, unsigned long addr, int level,
+ u64 val, unsigned long page_size)
{
int width = sizeof(unsigned long) * 2;
static const char units[] = "KMGTPE";
diff --git a/arch/x86/mm/dump_pagetables.c b/arch/x86/mm/dump_pagetables.c
index e1b599ecbbc2..2ec76737c1f1 100644
--- a/arch/x86/mm/dump_pagetables.c
+++ b/arch/x86/mm/dump_pagetables.c
@@ -272,7 +272,7 @@ static void effective_prot(struct ptdump_state *pt_st, int level, u64 val)
* print what we collected so far.
*/
static void note_page(struct ptdump_state *pt_st, unsigned long addr, int level,
- u64 val)
+ u64 val, unsigned long page_size)
{
struct pg_state *st = container_of(pt_st, struct pg_state, ptdump);
pgprotval_t new_prot, new_eff;
diff --git a/include/linux/ptdump.h b/include/linux/ptdump.h
index 2a3a95586425..3a971fadc95e 100644
--- a/include/linux/ptdump.h
+++ b/include/linux/ptdump.h
@@ -13,7 +13,7 @@ struct ptdump_range {
struct ptdump_state {
/* level is 0:PGD to 4:PTE, or -1 if unknown */
void (*note_page)(struct ptdump_state *st, unsigned long addr,
- int level, u64 val);
+ int level, u64 val, unsigned long page_size);
void (*effective_prot)(struct ptdump_state *st, int level, u64 val);
const struct ptdump_range *range;
};
diff --git a/mm/ptdump.c b/mm/ptdump.c
index da751448d0e4..61cd16afb1c8 100644
--- a/mm/ptdump.c
+++ b/mm/ptdump.c
@@ -17,7 +17,7 @@ static inline int note_kasan_page_table(struct mm_walk *walk,
{
struct ptdump_state *st = walk->private;
- st->note_page(st, addr, 4, pte_val(kasan_early_shadow_pte[0]));
+ st->note_page(st, addr, 4, pte_val(kasan_early_shadow_pte[0]), PAGE_SIZE);
walk->action = ACTION_CONTINUE;
@@ -41,7 +41,7 @@ static int ptdump_pgd_entry(pgd_t *pgd, unsigned long addr,
st->effective_prot(st, 0, pgd_val(val));
if (pgd_leaf(val))
- st->note_page(st, addr, 0, pgd_val(val));
+ st->note_page(st, addr, 0, pgd_val(val), PGDIR_SIZE);
return 0;
}
@@ -62,7 +62,7 @@ static int ptdump_p4d_entry(p4d_t *p4d, unsigned long addr,
st->effective_prot(st, 1, p4d_val(val));
if (p4d_leaf(val))
- st->note_page(st, addr, 1, p4d_val(val));
+ st->note_page(st, addr, 1, p4d_val(val), P4D_SIZE);
return 0;
}
@@ -83,7 +83,7 @@ static int ptdump_pud_entry(pud_t *pud, unsigned long addr,
st->effective_prot(st, 2, pud_val(val));
if (pud_leaf(val))
- st->note_page(st, addr, 2, pud_val(val));
+ st->note_page(st, addr, 2, pud_val(val), PUD_SIZE);
return 0;
}
@@ -102,7 +102,7 @@ static int ptdump_pmd_entry(pmd_t *pmd, unsigned long addr,
if (st->effective_prot)
st->effective_prot(st, 3, pmd_val(val));
if (pmd_leaf(val))
- st->note_page(st, addr, 3, pmd_val(val));
+ st->note_page(st, addr, 3, pmd_val(val), PMD_SIZE);
return 0;
}
@@ -116,7 +116,7 @@ static int ptdump_pte_entry(pte_t *pte, unsigned long addr,
if (st->effective_prot)
st->effective_prot(st, 4, pte_val(val));
- st->note_page(st, addr, 4, pte_val(val));
+ st->note_page(st, addr, 4, pte_val(val), PAGE_SIZE);
return 0;
}
@@ -126,7 +126,7 @@ static int ptdump_hole(unsigned long addr, unsigned long next,
{
struct ptdump_state *st = walk->private;
- st->note_page(st, addr, depth, 0);
+ st->note_page(st, addr, depth, 0, 0);
return 0;
}
@@ -153,5 +153,5 @@ void ptdump_walk_pgd(struct ptdump_state *st, struct mm_struct *mm, pgd_t *pgd)
mmap_read_unlock(mm);
/* Flush out the last page */
- st->note_page(st, 0, -1, 0);
+ st->note_page(st, 0, -1, 0, 0);
}
--
2.25.0
^ permalink raw reply related
* [PATCH v1 2/5] mm: ptdump: Fix build failure
From: Christophe Leroy @ 2021-04-15 17:18 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Steven Price, akpm
Cc: linux-arch, linux-s390, x86, linux-kernel, linux-mm, linux-riscv,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <cover.1618506910.git.christophe.leroy@csgroup.eu>
CC mm/ptdump.o
In file included from <command-line>:
mm/ptdump.c: In function 'ptdump_pte_entry':
././include/linux/compiler_types.h:320:38: error: call to '__compiletime_assert_207' declared with attribute error: Unsupported access size for {READ,WRITE}_ONCE().
320 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^
././include/linux/compiler_types.h:301:4: note: in definition of macro '__compiletime_assert'
301 | prefix ## suffix(); \
| ^~~~~~
././include/linux/compiler_types.h:320:2: note: in expansion of macro '_compiletime_assert'
320 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
./include/asm-generic/rwonce.h:36:2: note: in expansion of macro 'compiletime_assert'
36 | compiletime_assert(__native_word(t) || sizeof(t) == sizeof(long long), \
| ^~~~~~~~~~~~~~~~~~
./include/asm-generic/rwonce.h:49:2: note: in expansion of macro 'compiletime_assert_rwonce_type'
49 | compiletime_assert_rwonce_type(x); \
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mm/ptdump.c:114:14: note: in expansion of macro 'READ_ONCE'
114 | pte_t val = READ_ONCE(*pte);
| ^~~~~~~~~
make[2]: *** [mm/ptdump.o] Error 1
READ_ONCE() cannot be used for reading PTEs. Use ptep_get()
instead. See commit 481e980a7c19 ("mm: Allow arches to provide ptep_get()")
and commit c0e1c8c22beb ("powerpc/8xx: Provide ptep_get() with 16k pages")
for details.
Fixes: 30d621f6723b ("mm: add generic ptdump")
Cc: Steven Price <steven.price@arm.com>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
mm/ptdump.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/ptdump.c b/mm/ptdump.c
index 4354c1422d57..da751448d0e4 100644
--- a/mm/ptdump.c
+++ b/mm/ptdump.c
@@ -111,7 +111,7 @@ static int ptdump_pte_entry(pte_t *pte, unsigned long addr,
unsigned long next, struct mm_walk *walk)
{
struct ptdump_state *st = walk->private;
- pte_t val = READ_ONCE(*pte);
+ pte_t val = ptep_get(pte);
if (st->effective_prot)
st->effective_prot(st, 4, pte_val(val));
--
2.25.0
^ permalink raw reply related
* [PATCH v1 5/5] powerpc/mm: Convert powerpc to GENERIC_PTDUMP
From: Christophe Leroy @ 2021-04-15 17:18 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Steven Price, akpm
Cc: linux-arch, linux-s390, x86, linux-kernel, linux-mm, linux-riscv,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <cover.1618506910.git.christophe.leroy@csgroup.eu>
This patch converts powerpc to the generic PTDUMP implementation.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/Kconfig | 2 +
arch/powerpc/Kconfig.debug | 30 ------
arch/powerpc/mm/Makefile | 2 +-
arch/powerpc/mm/mmu_decl.h | 2 +-
arch/powerpc/mm/ptdump/8xx.c | 6 +-
arch/powerpc/mm/ptdump/Makefile | 9 +-
arch/powerpc/mm/ptdump/book3s64.c | 6 +-
arch/powerpc/mm/ptdump/ptdump.c | 161 +++++++++---------------------
arch/powerpc/mm/ptdump/shared.c | 6 +-
9 files changed, 68 insertions(+), 156 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 475d77a6ebbe..40259437a28f 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -120,6 +120,7 @@ config PPC
select ARCH_32BIT_OFF_T if PPC32
select ARCH_HAS_DEBUG_VIRTUAL
select ARCH_HAS_DEBUG_VM_PGTABLE
+ select ARCH_HAS_DEBUG_WX if STRICT_KERNEL_RWX
select ARCH_HAS_DEVMEM_IS_ALLOWED
select ARCH_HAS_ELF_RANDOMIZE
select ARCH_HAS_FORTIFY_SOURCE
@@ -177,6 +178,7 @@ config PPC
select GENERIC_IRQ_SHOW
select GENERIC_IRQ_SHOW_LEVEL
select GENERIC_PCI_IOMAP if PCI
+ select GENERIC_PTDUMP
select GENERIC_SMP_IDLE_THREAD
select GENERIC_STRNCPY_FROM_USER
select GENERIC_STRNLEN_USER
diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index 6342f9da4545..05b1180ea502 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -360,36 +360,6 @@ config FAIL_IOMMU
If you are unsure, say N.
-config PPC_PTDUMP
- bool "Export kernel pagetable layout to userspace via debugfs"
- depends on DEBUG_KERNEL && DEBUG_FS
- help
- This option exports the state of the kernel pagetables to a
- debugfs file. This is only useful for kernel developers who are
- working in architecture specific areas of the kernel - probably
- not a good idea to enable this feature in a production kernel.
-
- If you are unsure, say N.
-
-config PPC_DEBUG_WX
- bool "Warn on W+X mappings at boot"
- depends on PPC_PTDUMP && STRICT_KERNEL_RWX
- help
- Generate a warning if any W+X mappings are found at boot.
-
- This is useful for discovering cases where the kernel is leaving
- W+X mappings after applying NX, as such mappings are a security risk.
-
- Note that even if the check fails, your kernel is possibly
- still fine, as W+X mappings are not a security hole in
- themselves, what they do is that they make the exploitation
- of other unfixed kernel bugs easier.
-
- There is no runtime or memory usage effect of this option
- once the kernel has booted up - it's a one time check.
-
- If in doubt, say "Y".
-
config PPC_FAST_ENDIAN_SWITCH
bool "Deprecated fast endian-switch syscall"
depends on DEBUG_KERNEL && PPC_BOOK3S_64
diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
index c3df3a8501d4..c90d58aaebe2 100644
--- a/arch/powerpc/mm/Makefile
+++ b/arch/powerpc/mm/Makefile
@@ -18,5 +18,5 @@ obj-$(CONFIG_PPC_MM_SLICES) += slice.o
obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o
obj-$(CONFIG_NOT_COHERENT_CACHE) += dma-noncoherent.o
obj-$(CONFIG_PPC_COPRO_BASE) += copro_fault.o
-obj-$(CONFIG_PPC_PTDUMP) += ptdump/
+obj-$(CONFIG_PTDUMP_CORE) += ptdump/
obj-$(CONFIG_KASAN) += kasan/
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index 7dac910c0b21..dd1cabc2ea0f 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -180,7 +180,7 @@ static inline void mmu_mark_rodata_ro(void) { }
void __init mmu_mapin_immr(void);
#endif
-#ifdef CONFIG_PPC_DEBUG_WX
+#ifdef CONFIG_DEBUG_WX
void ptdump_check_wx(void);
#else
static inline void ptdump_check_wx(void) { }
diff --git a/arch/powerpc/mm/ptdump/8xx.c b/arch/powerpc/mm/ptdump/8xx.c
index 86da2a669680..fac932eb8f9a 100644
--- a/arch/powerpc/mm/ptdump/8xx.c
+++ b/arch/powerpc/mm/ptdump/8xx.c
@@ -75,8 +75,10 @@ static const struct flag_info flag_array[] = {
};
struct pgtable_level pg_level[5] = {
- {
- }, { /* pgd */
+ { /* pgd */
+ .flag = flag_array,
+ .num = ARRAY_SIZE(flag_array),
+ }, { /* p4d */
.flag = flag_array,
.num = ARRAY_SIZE(flag_array),
}, { /* pud */
diff --git a/arch/powerpc/mm/ptdump/Makefile b/arch/powerpc/mm/ptdump/Makefile
index 712762be3cb1..4050cbb55acf 100644
--- a/arch/powerpc/mm/ptdump/Makefile
+++ b/arch/powerpc/mm/ptdump/Makefile
@@ -5,5 +5,10 @@ obj-y += ptdump.o
obj-$(CONFIG_4xx) += shared.o
obj-$(CONFIG_PPC_8xx) += 8xx.o
obj-$(CONFIG_PPC_BOOK3E_MMU) += shared.o
-obj-$(CONFIG_PPC_BOOK3S_32) += shared.o bats.o segment_regs.o
-obj-$(CONFIG_PPC_BOOK3S_64) += book3s64.o hashpagetable.o
+obj-$(CONFIG_PPC_BOOK3S_32) += shared.o
+obj-$(CONFIG_PPC_BOOK3S_64) += book3s64.o
+
+ifdef CONFIG_PTDUMP_DEBUGFS
+obj-$(CONFIG_PPC_BOOK3S_32) += bats.o segment_regs.o
+obj-$(CONFIG_PPC_BOOK3S_64) += hashpagetable.o
+endif
diff --git a/arch/powerpc/mm/ptdump/book3s64.c b/arch/powerpc/mm/ptdump/book3s64.c
index 14f73868db66..5ad92d9dc5d1 100644
--- a/arch/powerpc/mm/ptdump/book3s64.c
+++ b/arch/powerpc/mm/ptdump/book3s64.c
@@ -103,8 +103,10 @@ static const struct flag_info flag_array[] = {
};
struct pgtable_level pg_level[5] = {
- {
- }, { /* pgd */
+ { /* pgd */
+ .flag = flag_array,
+ .num = ARRAY_SIZE(flag_array),
+ }, { /* p4d */
.flag = flag_array,
.num = ARRAY_SIZE(flag_array),
}, { /* pud */
diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
index aca354fb670b..9fb1f4fd8af4 100644
--- a/arch/powerpc/mm/ptdump/ptdump.c
+++ b/arch/powerpc/mm/ptdump/ptdump.c
@@ -16,6 +16,7 @@
#include <linux/io.h>
#include <linux/mm.h>
#include <linux/highmem.h>
+#include <linux/ptdump.h>
#include <linux/sched.h>
#include <linux/seq_file.h>
#include <asm/fixmap.h>
@@ -54,13 +55,14 @@
*
*/
struct pg_state {
+ struct ptdump_state ptdump;
struct seq_file *seq;
const struct addr_marker *marker;
unsigned long start_address;
unsigned long start_pa;
unsigned long last_pa;
unsigned long page_size;
- unsigned int level;
+ int level;
u64 current_flags;
bool check_wx;
unsigned long wx_pages;
@@ -216,14 +218,18 @@ static void note_page_update_state(struct pg_state *st, unsigned long addr,
}
}
-static void note_page(struct pg_state *st, unsigned long addr,
- unsigned int level, u64 val, unsigned long page_size)
+static void note_page(struct ptdump_state *pt_st, unsigned long addr, int level,
+ u64 val, unsigned long page_size)
{
- u64 flag = val & pg_level[level].mask;
+ struct pg_state *st = container_of(pt_st, struct pg_state, ptdump);
+ u64 flag = 0;
u64 pa = val & PTE_RPN_MASK;
+ if (level >= 0)
+ flag = val & pg_level[level].mask;
+
/* At first no level is set */
- if (!st->level) {
+ if (st->level == -1) {
pt_dump_seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
note_page_update_state(st, addr, level, val, page_size);
/*
@@ -262,94 +268,6 @@ static void note_page(struct pg_state *st, unsigned long addr,
st->last_pa = pa;
}
-static void walk_pte(struct pg_state *st, pmd_t *pmd, unsigned long start)
-{
- pte_t *pte = pte_offset_kernel(pmd, 0);
- unsigned long addr;
- unsigned int i;
-
- for (i = 0; i < PTRS_PER_PTE; i++, pte++) {
- addr = start + i * PAGE_SIZE;
- note_page(st, addr, 4, pte_val(*pte), PAGE_SIZE);
-
- }
-}
-
-static void walk_hugepd(struct pg_state *st, hugepd_t *phpd, unsigned long start,
- int pdshift, int level)
-{
-#ifdef CONFIG_ARCH_HAS_HUGEPD
- unsigned int i;
- int shift = hugepd_shift(*phpd);
- int ptrs_per_hpd = pdshift - shift > 0 ? 1 << (pdshift - shift) : 1;
-
- if (start & ((1 << shift) - 1))
- return;
-
- for (i = 0; i < ptrs_per_hpd; i++) {
- unsigned long addr = start + (i << shift);
- pte_t *pte = hugepte_offset(*phpd, addr, pdshift);
-
- note_page(st, addr, level + 1, pte_val(*pte), 1 << shift);
- }
-#endif
-}
-
-static void walk_pmd(struct pg_state *st, pud_t *pud, unsigned long start)
-{
- pmd_t *pmd = pmd_offset(pud, 0);
- unsigned long addr;
- unsigned int i;
-
- for (i = 0; i < PTRS_PER_PMD; i++, pmd++) {
- addr = start + i * PMD_SIZE;
- if (!pmd_none(*pmd) && !pmd_is_leaf(*pmd))
- /* pmd exists */
- walk_pte(st, pmd, addr);
- else
- note_page(st, addr, 3, pmd_val(*pmd), PMD_SIZE);
- }
-}
-
-static void walk_pud(struct pg_state *st, p4d_t *p4d, unsigned long start)
-{
- pud_t *pud = pud_offset(p4d, 0);
- unsigned long addr;
- unsigned int i;
-
- for (i = 0; i < PTRS_PER_PUD; i++, pud++) {
- addr = start + i * PUD_SIZE;
- if (!pud_none(*pud) && !pud_is_leaf(*pud))
- /* pud exists */
- walk_pmd(st, pud, addr);
- else
- note_page(st, addr, 2, pud_val(*pud), PUD_SIZE);
- }
-}
-
-static void walk_pagetables(struct pg_state *st)
-{
- unsigned int i;
- unsigned long addr = st->start_address & PGDIR_MASK;
- pgd_t *pgd = pgd_offset_k(addr);
-
- /*
- * Traverse the linux pagetable structure and dump pages that are in
- * the hash pagetable.
- */
- for (i = pgd_index(addr); i < PTRS_PER_PGD; i++, pgd++, addr += PGDIR_SIZE) {
- p4d_t *p4d = p4d_offset(pgd, 0);
-
- if (p4d_none(*p4d) || p4d_is_leaf(*p4d))
- note_page(st, addr, 1, p4d_val(*p4d), PGDIR_SIZE);
- else if (is_hugepd(__hugepd(p4d_val(*p4d))))
- walk_hugepd(st, (hugepd_t *)p4d, addr, PGDIR_SHIFT, 1);
- else
- /* p4d exists */
- walk_pud(st, p4d, addr);
- }
-}
-
static void populate_markers(void)
{
int i = 0;
@@ -399,32 +317,29 @@ static int ptdump_show(struct seq_file *m, void *v)
struct pg_state st = {
.seq = m,
.marker = address_markers,
- .start_address = IS_ENABLED(CONFIG_PPC64) ? PAGE_OFFSET : TASK_SIZE,
+ .level = -1,
+ .ptdump = {
+ .note_page = note_page,
+ .range = (struct ptdump_range[]){
+ {TASK_SIZE, ~0UL},
+ {0, 0}
+ }
+ }
};
#ifdef CONFIG_PPC64
if (!radix_enabled())
- st.start_address = KERN_VIRT_START;
+ st.ptdump.range.start = KERN_VIRT_START;
+ else
+ st.ptdump.range.start = PAGE_OFFSET;
#endif
/* Traverse kernel page tables */
- walk_pagetables(&st);
- note_page(&st, 0, 0, 0, 0);
+ ptdump_walk_pgd(&st.ptdump, &init_mm, NULL);
return 0;
}
-
-static int ptdump_open(struct inode *inode, struct file *file)
-{
- return single_open(file, ptdump_show, NULL);
-}
-
-static const struct file_operations ptdump_fops = {
- .open = ptdump_open,
- .read = seq_read,
- .llseek = seq_lseek,
- .release = single_release,
-};
+DEFINE_SHOW_ATTRIBUTE(ptdump);
static void build_pgtable_complete_mask(void)
{
@@ -436,22 +351,34 @@ static void build_pgtable_complete_mask(void)
pg_level[i].mask |= pg_level[i].flag[j].mask;
}
-#ifdef CONFIG_PPC_DEBUG_WX
+#ifdef CONFIG_DEBUG_WX
void ptdump_check_wx(void)
{
struct pg_state st = {
.seq = NULL,
- .marker = address_markers,
+ .marker = (struct addr_marker[]) {
+ { 0, NULL},
+ { -1, NULL},
+ },
+ .level = -1,
.check_wx = true,
- .start_address = IS_ENABLED(CONFIG_PPC64) ? PAGE_OFFSET : TASK_SIZE,
+ .ptdump = {
+ .note_page = note_page,
+ .range = (struct ptdump_range[]){
+ {TASK_SIZE, ~0UL},
+ {0, 0}
+ }
+ }
};
#ifdef CONFIG_PPC64
if (!radix_enabled())
- st.start_address = KERN_VIRT_START;
+ st.ptdump.range.start = KERN_VIRT_START;
+ else
+ st.ptdump.range.start = PAGE_OFFSET;
#endif
- walk_pagetables(&st);
+ ptdump_walk_pgd(&st.ptdump, &init_mm, NULL);
if (st.wx_pages)
pr_warn("Checked W+X mappings: FAILED, %lu W+X pages found\n",
@@ -465,8 +392,10 @@ static int ptdump_init(void)
{
populate_markers();
build_pgtable_complete_mask();
- debugfs_create_file("kernel_page_tables", 0400, NULL, NULL,
- &ptdump_fops);
+
+ if (IS_ENABLED(CONFIG_PTDUMP_DEBUGFS))
+ debugfs_create_file("kernel_page_tables", 0400, NULL, NULL, &ptdump_fops);
+
return 0;
}
device_initcall(ptdump_init);
diff --git a/arch/powerpc/mm/ptdump/shared.c b/arch/powerpc/mm/ptdump/shared.c
index c005fe041c18..03607ab90c66 100644
--- a/arch/powerpc/mm/ptdump/shared.c
+++ b/arch/powerpc/mm/ptdump/shared.c
@@ -68,8 +68,10 @@ static const struct flag_info flag_array[] = {
};
struct pgtable_level pg_level[5] = {
- {
- }, { /* pgd */
+ { /* pgd */
+ .flag = flag_array,
+ .num = ARRAY_SIZE(flag_array),
+ }, { /* p4d */
.flag = flag_array,
.num = ARRAY_SIZE(flag_array),
}, { /* pud */
--
2.25.0
^ permalink raw reply related
* Re: [PATCH v1 1/1] powerpc/papr_scm: Properly handle UUID types and API
From: Andy Shevchenko @ 2021-04-15 17:18 UTC (permalink / raw)
To: Vaibhav Jain
Cc: Linux Kernel Mailing List, Paul Mackerras, Aneesh Kumar K . V,
Oliver O'Halloran, Andy Shevchenko,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT
In-Reply-To: <87eefblbji.fsf@vajain21.in.ibm.com>
On Thu, Apr 15, 2021 at 8:10 PM Vaibhav Jain <vaibhav@linux.ibm.com> wrote:
>
>
> Thanks for the patch Andy,
>
> Unfortunately ran into a compilation issue due to missing "#include
> <asm/unaligned.h>" that provides definition for
> get_unaligned_le64(). Gcc reported following error:
>
> error: implicit declaration of function ‘get_unaligned_le64’
Right, I have not tested it (as mentioned in the comments to the patch)
> After including the necessary header file, kernel compiled fine and I
> was able to test & verify the patch.
Thank you very much for the testing.
I'm not sure what the coverage of your test is. That's why I have an
additional question below. Is the byte ordering kept the same in BE
(32- and 64-bit) cases? Because I'm worrying that I might have missed
something.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 3/3] powerpc/smp: Cache CPU to chip lookup
From: Gautham R Shenoy @ 2021-04-15 17:19 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Nathan Lynch, Gautham R Shenoy, Peter Zijlstra,
Daniel Henrique Barboza, Valentin Schneider, qemu-ppc,
Cedric Le Goater, linuxppc-dev, Ingo Molnar, David Gibson
In-Reply-To: <20210415120934.232271-4-srikar@linux.vnet.ibm.com>
On Thu, Apr 15, 2021 at 05:39:34PM +0530, Srikar Dronamraju wrote:
> On systems with large CPUs per node, even with the filtered matching of
> related CPUs, there can be large number of calls to cpu_to_chip_id for
> the same CPU. For example with 4096 vCPU, 1 node QEMU configuration,
> with 4 threads per core, system could be see upto 1024 calls to
> cpu_to_chip_id() for the same CPU. On a given system, cpu_to_chip_id()
> for a given CPU would always return the same. Hence cache the result in
> a lookup table for use in subsequent calls.
>
> Since all CPUs sharing the same core will belong to the same chip, the
> lookup_table has an entry for one CPU per core. chip_id_lookup_table is
> not being freed and would be used on subsequent CPU online post CPU
> offline.
>
> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: qemu-ppc@nongnu.org
> Cc: Cedric Le Goater <clg@kaod.org>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Nathan Lynch <nathanl@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Valentin Schneider <valentin.schneider@arm.com>
> Cc: Gautham R Shenoy <ego@linux.vnet.ibm.com>
> Reported-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/smp.h | 1 +
> arch/powerpc/kernel/prom.c | 19 +++++++++++++++----
> arch/powerpc/kernel/smp.c | 21 +++++++++++++++++++--
> 3 files changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> index 47081a9e13ca..03b3d010cbab 100644
> --- a/arch/powerpc/include/asm/smp.h
> +++ b/arch/powerpc/include/asm/smp.h
> @@ -31,6 +31,7 @@ extern u32 *cpu_to_phys_id;
> extern bool coregroup_enabled;
>
> extern int cpu_to_chip_id(int cpu);
> +extern int *chip_id_lookup_table;
>
> #ifdef CONFIG_SMP
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 9a4797d1d40d..6d2e4a5bc471 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -65,6 +65,8 @@
> #define DBG(fmt...)
> #endif
>
> +int *chip_id_lookup_table;
> +
> #ifdef CONFIG_PPC64
> int __initdata iommu_is_off;
> int __initdata iommu_force_on;
> @@ -914,13 +916,22 @@ EXPORT_SYMBOL(of_get_ibm_chip_id);
> int cpu_to_chip_id(int cpu)
> {
> struct device_node *np;
> + int ret = -1, idx;
> +
> + idx = cpu / threads_per_core;
> + if (chip_id_lookup_table && chip_id_lookup_table[idx] != -1)
The value -1 is ambiguous since we won't be able to determine if
it is because we haven't yet made a of_get_ibm_chip_id() call
or if of_get_ibm_chip_id() call was made and it returned a -1.
Thus, perhaps we can initialize chip_id_lookup_table[idx] with a
different unique negative value. How about S32_MIN ? and check
chip_id_lookup_table[idx] is different here ?
> + return chip_id_lookup_table[idx];
>
> np = of_get_cpu_node(cpu, NULL);
> - if (!np)
> - return -1;
> + if (np) {
> + ret = of_get_ibm_chip_id(np);
> + of_node_put(np);
> +
> + if (chip_id_lookup_table)
> + chip_id_lookup_table[idx] = ret;
> + }
>
> - of_node_put(np);
> - return of_get_ibm_chip_id(np);
> + return ret;
> }
> EXPORT_SYMBOL(cpu_to_chip_id);
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 5c7ce1d50631..50520fbea424 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1073,6 +1073,20 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
> cpu_smallcore_mask(boot_cpuid));
> }
>
> + if (cpu_to_chip_id(boot_cpuid) != -1) {
> + int idx = num_possible_cpus() / threads_per_core;
> +
> + /*
> + * All threads of a core will all belong to the same core,
> + * chip_id_lookup_table will have one entry per core.
> + * Assumption: if boot_cpuid doesn't have a chip-id, then no
> + * other CPUs, will also not have chip-id.
> + */
> + chip_id_lookup_table = kcalloc(idx, sizeof(int), GFP_KERNEL);
> + if (chip_id_lookup_table)
> + memset(chip_id_lookup_table, -1, sizeof(int) * idx);
> + }
> +
> if (smp_ops && smp_ops->probe)
> smp_ops->probe();
> }
> @@ -1468,8 +1482,8 @@ static void add_cpu_to_masks(int cpu)
> {
> struct cpumask *(*submask_fn)(int) = cpu_sibling_mask;
> int first_thread = cpu_first_thread_sibling(cpu);
> - int chip_id = cpu_to_chip_id(cpu);
> cpumask_var_t mask;
> + int chip_id = -1;
> bool ret;
> int i;
>
> @@ -1492,7 +1506,10 @@ static void add_cpu_to_masks(int cpu)
> if (has_coregroup_support())
> update_coregroup_mask(cpu, &mask);
>
> - if (chip_id == -1 || !ret) {
> + if (chip_id_lookup_table && ret)
> + chip_id = cpu_to_chip_id(cpu);
> +
> + if (chip_id == -1) {
> cpumask_copy(per_cpu(cpu_core_map, cpu), cpu_cpu_mask(cpu));
> goto out;
> }
> --
> 2.25.1
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox