* [PATCH V2] ocxl: Fix access to the AFU Descriptor Data
From: Christophe Lombard @ 2018-08-13 14:09 UTC (permalink / raw)
To: linuxppc-dev, fbarrat, vaibhav, andrew.donnellan
The AFU Information DVSEC capability is a means to extract common,
general information about all of the AFUs associated with a Function
independent of the specific functionality that each AFU provides.
This patch fixes the access to the AFU Descriptor Data indexed by the
AFU Info Index field.
Fixes: 5ef3166e8a32 ("ocxl: Driver code for 'generic' opencapi devices")
Cc: stable <stable@vger.kernel.org> # 4.16
Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
---
Changelog[v2]
- Rebase to latest upstream.
- Use pci_write_config_byte instead of pci_write_config_word
---
drivers/misc/ocxl/config.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
index 2e30de9..57a6bb1 100644
--- a/drivers/misc/ocxl/config.c
+++ b/drivers/misc/ocxl/config.c
@@ -280,7 +280,9 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
u32 val;
int rc, templ_major, templ_minor, len;
- pci_write_config_word(dev, fn->dvsec_afu_info_pos, afu_idx);
+ pci_write_config_byte(dev,
+ fn->dvsec_afu_info_pos + OCXL_DVSEC_AFU_INFO_AFU_IDX,
+ afu_idx);
rc = read_afu_info(dev, fn, OCXL_DVSEC_TEMPL_VERSION, &val);
if (rc)
return rc;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v6 1/2] powerpc: Detect the presence of big-cores via "ibm,thread-groups"
From: Benjamin Herrenschmidt @ 2018-08-13 13:51 UTC (permalink / raw)
To: ego, Srikar Dronamraju
Cc: Michael Ellerman, Michael Neuling, Vaidyanathan Srinivasan,
Akshay Adiga, Shilpasri G Bhat, Oliver O'Halloran,
Nicholas Piggin, Murilo Opsfelder Araujo, Anton Blanchard,
linuxppc-dev, linux-kernel
In-Reply-To: <20180813113633.GA19859@in.ibm.com>
On Mon, 2018-08-13 at 17:06 +0530, Gautham R Shenoy wrote:
> Hi Srikar,
>
> Thanks for reviewing the patch.
>
> On Thu, Aug 09, 2018 at 06:27:43AM -0700, Srikar Dronamraju wrote:
> > * Gautham R. Shenoy <ego@linux.vnet.ibm.com> [2018-08-09 11:02:07]:
> >
> > >
> > > int threads_per_core, threads_per_subcore, threads_shift;
> > > +bool has_big_cores;
> > > cpumask_t threads_core_mask;
> > > EXPORT_SYMBOL_GPL(threads_per_core);
> > > EXPORT_SYMBOL_GPL(threads_per_subcore);
> > > EXPORT_SYMBOL_GPL(threads_shift);
> > > +EXPORT_SYMBOL_GPL(has_big_cores);
> >
> > Why do we need EXPORT_SYMBOL_GPL?
>
> As Christoph pointed out, I was blindly following the suit.
>
> You are right, we don't need to export it at the moment. The remaining
> EXPORT_SYMBOL_GPL are required by KVM. However, as of now, there is no
> need for "has_big_cores" in the KVM.
There is actually. KVM needs to refuse to start on big cores, at least
HV KVM. And when KVM grows support for big cores (may or may not
happen), it will need to know. So keep the GPL export.
> Will remove this in the next version.
> >
> > > EXPORT_SYMBOL_GPL(threads_core_mask);
> > >
> > > + *
> > > + * Returns 0 on success, -EINVAL if the property does not exist,
> > > + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> > > + * property data isn't large enough.
> > > + */
> > > +int parse_thread_groups(struct device_node *dn,
> > > + struct thread_groups *tg)
> > > +{
> > > + unsigned int nr_groups, threads_per_group, property;
> > > + int i;
> > > + u32 thread_group_array[3 + MAX_THREAD_LIST_SIZE];
> > > + u32 *thread_list;
> > > + size_t total_threads;
> > > + int ret;
> > > +
> > > + ret = of_property_read_u32_array(dn, "ibm,thread-groups",
> > > + thread_group_array, 3);
> > > +
> > > + if (ret)
> > > + goto out_err;
> > > +
> > > + property = thread_group_array[0];
> > > + nr_groups = thread_group_array[1];
> > > + threads_per_group = thread_group_array[2];
> > > + total_threads = nr_groups * threads_per_group;
>
>
> > > +
> >
> > Shouldnt we check for property and nr_groups
> > If the property is not 1 and nr_groups < 1, we should error out
> > No point in calling a of_property read if property is not right.
>
> Yes, the nr_groups < 1 check can be moved into this function.
>
> However, this function merely parses the the thread group structure
> exposed by the device tree. So it should error out only if there is a
> failure in parsing, or as you said the parsed values are incorrect
> (nr_groups < 1, threads_per_group < 1, etc). Whether the thread group
> is relevant or not (in this case we are interested in thread groups
> that share L1 cache, translation etc) is something for the caller to
> decide.
>
> However, I see what you mean. We can avoid parsing the remainder of
> the array if the property in the device-tree isn't the property that
> the caller is interested in.
>
> This can be solved by passing the interested property value as a
> parameter and so that the code errors out if this property doesn't
> match the property in the device-tree. Will add this in the next
> version.
>
> >
> >
> > Nit:
> > Cant we directly assign to tg->property, and hence avoid local
> > variables, property, nr_groups and threads_per_group?
>
> Will clean this up. This was from an older version where I added the
> local variables so that the statements referencing them don't need to
> be split across multiple lines. However, the code has been optimized
> since then. So, the local variables are not needed.
>
> >
> > > + ret = of_property_read_u32_array(dn, "ibm,thread-groups",
> > > + thread_group_array,
> > > + 3 + total_threads);
> > > +
> > > +static inline bool dt_has_big_core(struct device_node *dn,
> > > + struct thread_groups *tg)
> > > +{
> > > + if (parse_thread_groups(dn, tg))
> > > + return false;
> > > +
> > > + if (tg->property != 1)
> > > + return false;
> > > +
> > > + if (tg->nr_groups < 1)
> > > + return false;
> >
> > Can avoid these check if we can check in parse_thread_groups.
> >
> > > /**
> > > * setup_cpu_maps - initialize the following cpu maps:
> > > * cpu_possible_mask
> > > @@ -457,6 +605,7 @@ void __init smp_setup_cpu_maps(void)
> > > int cpu = 0;
> > > int nthreads = 1;
> > >
> > > diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> > > index 755dc98..f5717de 100644
> > > --- a/arch/powerpc/kernel/sysfs.c
> > > +++ b/arch/powerpc/kernel/sysfs.c
> > > @@ -18,6 +18,7 @@
> > > #include <asm/smp.h>
> > > #include <asm/pmc.h>
> > > #include <asm/firmware.h>
> > > +#include <asm/cputhreads.h>
> > >
> > > #include "cacheinfo.h"
> > > #include "setup.h"
> > > @@ -1025,6 +1026,33 @@ static ssize_t show_physical_id(struct device *dev,
> > > }
> > > static DEVICE_ATTR(physical_id, 0444, show_physical_id, NULL);
> > >
> > > +static ssize_t show_small_core_siblings(struct device *dev,
> > > + struct device_attribute *attr,
> > > + char *buf)
> > > +{
> > > + struct cpu *cpu = container_of(dev, struct cpu, dev);
> > > + struct device_node *dn = of_get_cpu_node(cpu->dev.id, NULL);
> > > + struct thread_groups tg;
> > > + int i, j;
> > > + ssize_t ret = 0;
> > > +
> >
> > Here we need to check for validity of dn and error out accordingly.
>
> Will add this check.
>
> >
> >
> > > + if (parse_thread_groups(dn, &tg))
> > > + return -ENODATA;
> >
> > Did we miss a of_node_put(dn)?
>
> Yes we did. Will fix this.
> >
> > > +
> > > + i = get_cpu_thread_group_start(cpu->dev.id, &tg);
> > > +
> > > + if (i == -1)
> > > + return -ENODATA;
> > > +
> > > + for (j = 0; j < tg.threads_per_group - 1; j++)
> > > + ret += sprintf(buf + ret, "%d,", tg.thread_list[i + j]);
> >
> > Here, we are making the assumption that group_start will always be the
> > first thread in the thread_group. However we didnt make the same
> > assumption in get_cpu_thread_group_start.
>
> Above the get_cpu_thread_group_start function , we have the following
> comment which indicates that group_start will point to the start of
> the thread group. Is this what you are referring to?
>
> /*
> * Returns the index to tg->thread_list that points to the the start
> * of the thread_group that @cpu belongs to.
> *
> * Returns -1 if cpu doesn't belong to any of the groups pointed to by
> * tg->thread_list.
> */
^ permalink raw reply
* Re: [PATCH 1/3] powerpc/mm: fix a warning when a cache is common to PGD and hugepages
From: Aneesh Kumar K.V @ 2018-08-13 13:44 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, aneesh.kumar
Cc: linux-kernel, linuxppc-dev
In-Reply-To: <1cbc8dba537393edbb6384b67cbb123431ba6a32.1534166756.git.christophe.leroy@c-s.fr>
On 08/13/2018 06:57 PM, Christophe Leroy wrote:
> While implementing TLB miss HW assistance on the 8xx, the following
> warning was encountered:
>
> [ 423.732965] WARNING: CPU: 0 PID: 345 at mm/slub.c:2412 ___slab_alloc.constprop.30+0x26c/0x46c
> [ 423.733033] CPU: 0 PID: 345 Comm: mmap Not tainted 4.18.0-rc8-00664-g2dfff9121c55 #671
> [ 423.733075] NIP: c0108f90 LR: c0109ad0 CTR: 00000004
> [ 423.733121] REGS: c455bba0 TRAP: 0700 Not tainted (4.18.0-rc8-00664-g2dfff9121c55)
> [ 423.733147] MSR: 00021032 <ME,IR,DR,RI> CR: 24224848 XER: 20000000
> [ 423.733319]
> [ 423.733319] GPR00: c0109ad0 c455bc50 c4521910 c60053c0 007080c0 c0011b34 c7fa41e0 c455be30
> [ 423.733319] GPR08: 00000001 c00103a0 c7fa41e0 c49afcc4 24282842 10018840 c079b37c 00000040
> [ 423.733319] GPR16: 73f00000 00210d00 00000000 00000001 c455a000 00000100 00000200 c455a000
> [ 423.733319] GPR24: c60053c0 c0011b34 007080c0 c455a000 c455a000 c7fa41e0 00000000 00009032
> [ 423.734190] NIP [c0108f90] ___slab_alloc.constprop.30+0x26c/0x46c
> [ 423.734257] LR [c0109ad0] kmem_cache_alloc+0x210/0x23c
> [ 423.734283] Call Trace:
> [ 423.734326] [c455bc50] [00000100] 0x100 (unreliable)
> [ 423.734430] [c455bcc0] [c0109ad0] kmem_cache_alloc+0x210/0x23c
> [ 423.734543] [c455bcf0] [c0011b34] huge_pte_alloc+0xc0/0x1dc
> [ 423.734633] [c455bd20] [c01044dc] hugetlb_fault+0x408/0x48c
> [ 423.734720] [c455bdb0] [c0104b20] follow_hugetlb_page+0x14c/0x44c
> [ 423.734826] [c455be10] [c00e8e54] __get_user_pages+0x1c4/0x3dc
> [ 423.734919] [c455be80] [c00e9924] __mm_populate+0xac/0x140
> [ 423.735020] [c455bec0] [c00db14c] vm_mmap_pgoff+0xb4/0xb8
> [ 423.735127] [c455bf00] [c00f27c0] ksys_mmap_pgoff+0xcc/0x1fc
> [ 423.735222] [c455bf40] [c000e0f8] ret_from_syscall+0x0/0x38
> [ 423.735271] Instruction dump:
> [ 423.735321] 7cbf482e 38fd0008 7fa6eb78 7fc4f378 4bfff5dd 7fe3fb78 4bfffe24 81370010
> [ 423.735536] 71280004 41a2ff88 4840c571 4bffff80 <0fe00000> 4bfffeb8 81340010 712a0004
> [ 423.735757] ---[ end trace e9b222919a470790 ]---
>
> This warning occurs when calling kmem_cache_zalloc() on a
> cache having a constructor.
>
> In this case it happens because PGD cache and 512k hugepte cache are
> the same size (4k). While a cache with constructor is created for
> the PGD, hugepages create cache without constructor and uses
> kmem_cache_zalloc(). As both expect a cache with the same size,
> the hugepages reuse the cache created for PGD, hence the conflict.
>
> As the constructors only aim at zeroing the allocated memory, this
> patch fixes this issue by removing the constructors and using
> kmem_cache_zalloc() instead.
>
But that means we zero out on each alloc from the slab right? Earlier we
allocated we we added memory to the slab. Also we have code that
carefully zero things out when we free the page table back to slab.
The idea there was, it is better take the cost of zeroing out during
free rather than fault.
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/include/asm/book3s/32/pgalloc.h | 2 +-
> arch/powerpc/include/asm/book3s/64/pgalloc.h | 4 ++--
> arch/powerpc/include/asm/nohash/32/pgalloc.h | 2 +-
> arch/powerpc/include/asm/nohash/64/pgalloc.h | 6 +++---
> arch/powerpc/mm/init-common.c | 21 +++------------------
> 5 files changed, 10 insertions(+), 25 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/32/pgalloc.h b/arch/powerpc/include/asm/book3s/32/pgalloc.h
> index 82e44b1a00ae..4c23cc1ae7a1 100644
> --- a/arch/powerpc/include/asm/book3s/32/pgalloc.h
> +++ b/arch/powerpc/include/asm/book3s/32/pgalloc.h
> @@ -32,7 +32,7 @@ extern struct kmem_cache *pgtable_cache[];
>
> static inline pgd_t *pgd_alloc(struct mm_struct *mm)
> {
> - return kmem_cache_alloc(PGT_CACHE(PGD_INDEX_SIZE),
> + return kmem_cache_zalloc(PGT_CACHE(PGD_INDEX_SIZE),
> pgtable_gfp_flags(mm, GFP_KERNEL));
> }
>
> diff --git a/arch/powerpc/include/asm/book3s/64/pgalloc.h b/arch/powerpc/include/asm/book3s/64/pgalloc.h
> index 76234a14b97d..074359cd632a 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgalloc.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgalloc.h
> @@ -81,7 +81,7 @@ static inline pgd_t *pgd_alloc(struct mm_struct *mm)
> if (radix_enabled())
> return radix__pgd_alloc(mm);
>
> - pgd = kmem_cache_alloc(PGT_CACHE(PGD_INDEX_SIZE),
> + pgd = kmem_cache_zalloc(PGT_CACHE(PGD_INDEX_SIZE),
> pgtable_gfp_flags(mm, GFP_KERNEL));
> /*
> * Don't scan the PGD for pointers, it contains references to PUDs but
> @@ -120,7 +120,7 @@ static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long addr)
> {
> pud_t *pud;
>
> - pud = kmem_cache_alloc(PGT_CACHE(PUD_CACHE_INDEX),
> + pud = kmem_cache_zalloc(PGT_CACHE(PUD_CACHE_INDEX),
> pgtable_gfp_flags(mm, GFP_KERNEL));
> /*
> * Tell kmemleak to ignore the PUD, that means don't scan it for
> diff --git a/arch/powerpc/include/asm/nohash/32/pgalloc.h b/arch/powerpc/include/asm/nohash/32/pgalloc.h
> index 8825953c225b..766cf0c90d19 100644
> --- a/arch/powerpc/include/asm/nohash/32/pgalloc.h
> +++ b/arch/powerpc/include/asm/nohash/32/pgalloc.h
> @@ -32,7 +32,7 @@ extern struct kmem_cache *pgtable_cache[];
>
> static inline pgd_t *pgd_alloc(struct mm_struct *mm)
> {
> - return kmem_cache_alloc(PGT_CACHE(PGD_INDEX_SIZE),
> + return kmem_cache_zalloc(PGT_CACHE(PGD_INDEX_SIZE),
> pgtable_gfp_flags(mm, GFP_KERNEL));
> }
>
> diff --git a/arch/powerpc/include/asm/nohash/64/pgalloc.h b/arch/powerpc/include/asm/nohash/64/pgalloc.h
> index e2d62d033708..54ee5ac02d81 100644
> --- a/arch/powerpc/include/asm/nohash/64/pgalloc.h
> +++ b/arch/powerpc/include/asm/nohash/64/pgalloc.h
> @@ -43,7 +43,7 @@ extern struct kmem_cache *pgtable_cache[];
>
> static inline pgd_t *pgd_alloc(struct mm_struct *mm)
> {
> - return kmem_cache_alloc(PGT_CACHE(PGD_INDEX_SIZE),
> + return kmem_cache_zalloc(PGT_CACHE(PGD_INDEX_SIZE),
> pgtable_gfp_flags(mm, GFP_KERNEL));
> }
>
> @@ -56,7 +56,7 @@ static inline void pgd_free(struct mm_struct *mm, pgd_t *pgd)
>
> static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long addr)
> {
> - return kmem_cache_alloc(PGT_CACHE(PUD_INDEX_SIZE),
> + return kmem_cache_zalloc(PGT_CACHE(PUD_INDEX_SIZE),
> pgtable_gfp_flags(mm, GFP_KERNEL));
> }
>
> @@ -86,7 +86,7 @@ static inline void pmd_populate(struct mm_struct *mm, pmd_t *pmd,
>
> static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long addr)
> {
> - return kmem_cache_alloc(PGT_CACHE(PMD_CACHE_INDEX),
> + return kmem_cache_zalloc(PGT_CACHE(PMD_CACHE_INDEX),
> pgtable_gfp_flags(mm, GFP_KERNEL));
> }
>
> diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
> index 2b656e67f2ea..2ae15ff8f76f 100644
> --- a/arch/powerpc/mm/init-common.c
> +++ b/arch/powerpc/mm/init-common.c
> @@ -25,21 +25,6 @@
> #include <asm/pgalloc.h>
> #include <asm/pgtable.h>
>
> -static void pgd_ctor(void *addr)
> -{
> - memset(addr, 0, PGD_TABLE_SIZE);
> -}
> -
> -static void pud_ctor(void *addr)
> -{
> - memset(addr, 0, PUD_TABLE_SIZE);
> -}
> -
> -static void pmd_ctor(void *addr)
> -{
> - memset(addr, 0, PMD_TABLE_SIZE);
> -}
> -
> struct kmem_cache *pgtable_cache[MAX_PGTABLE_INDEX_SIZE];
> EXPORT_SYMBOL_GPL(pgtable_cache); /* used by kvm_hv module */
>
> @@ -91,15 +76,15 @@ EXPORT_SYMBOL_GPL(pgtable_cache_add); /* used by kvm_hv module */
>
> void pgtable_cache_init(void)
> {
> - pgtable_cache_add(PGD_INDEX_SIZE, pgd_ctor);
> + pgtable_cache_add(PGD_INDEX_SIZE, NULL);
>
> if (PMD_CACHE_INDEX && !PGT_CACHE(PMD_CACHE_INDEX))
> - pgtable_cache_add(PMD_CACHE_INDEX, pmd_ctor);
> + pgtable_cache_add(PMD_CACHE_INDEX, NULL);
> /*
> * In all current configs, when the PUD index exists it's the
> * same size as either the pgd or pmd index except with THP enabled
> * on book3s 64
> */
> if (PUD_CACHE_INDEX && !PGT_CACHE(PUD_CACHE_INDEX))
> - pgtable_cache_add(PUD_CACHE_INDEX, pud_ctor);
> + pgtable_cache_add(PUD_CACHE_INDEX, NULL);
> }
>
^ permalink raw reply
* Re: [PATCH] ocxl: Fix access to the AFU Descriptor Data
From: christophe lombard @ 2018-08-13 13:42 UTC (permalink / raw)
To: Andrew Donnellan, linuxppc-dev, fbarrat, vaibhav
In-Reply-To: <85eab07a-5ef8-0d0a-fe82-9860ae84015e@au1.ibm.com>
Le 13/08/2018 à 11:48, Andrew Donnellan a écrit :
> On 13/08/18 19:01, Christophe Lombard wrote:
>> From: Christophe Lombard <christophe_lombard@fr.ibm.cm>
>
> Your git committer email should probably match your sign-off email.
>
>>
>> The AFU Information DVSEC capability is a means to extract common,
>> general information about all of the AFUs associated with a Function
>> independent of the specific functionality that each AFU provides.
>>
>> This patch fixes the access to the AFU Descriptor Data indexed by the
>> AFU Info Index field.
>>
>> Fixes: 5ef3166e8a32 ("ocxl: Driver code for 'generic' opencapi devices")
>>
>> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
>
> This looks like it should go to stable? I assume the reason we haven't
> noticed this previously is because we have not been testing with
> multi-AFU cards.
>
>> ---
>> drivers/misc/ocxl/config.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
>> index 2e30de9..de01623 100644
>> --- a/drivers/misc/ocxl/config.c
>> +++ b/drivers/misc/ocxl/config.c
>> @@ -280,7 +280,9 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
>> u32 val;
>> int rc, templ_major, templ_minor, len;
>> - pci_write_config_word(dev, fn->dvsec_afu_info_pos, afu_idx);
>> + pci_write_config_word(dev,
>> + fn->dvsec_afu_info_pos + OCXL_DVSEC_AFU_INFO_AFU_IDX,
>> + afu_idx);
>
> pci_write_config_byte() would be more appropriate here (see
> ocxl_config_read_afu() at line 454)
>
>> rc = read_afu_info(dev, fn, OCXL_DVSEC_TEMPL_VERSION, &val);
>> if (rc)
>> return rc;
>>
>
right. Thanks for the review.
^ permalink raw reply
* [PATCH 3/3] powerpc/mm: remove unneccessary test in pgtable_cache_init()
From: Christophe Leroy @ 2018-08-13 13:27 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
aneesh.kumar
Cc: linux-kernel, linuxppc-dev
In-Reply-To: <1cbc8dba537393edbb6384b67cbb123431ba6a32.1534166756.git.christophe.leroy@c-s.fr>
pgtable_cache_add() gracefully handles the case when a cache that
size already exists by returning early with the following test:
if (PGT_CACHE(shift))
return; /* Already have a cache of this size */
It is then not needed to test the existance of the cache before.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/mm/init-common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
index 8fb182aaf87d..ebdb76cbb468 100644
--- a/arch/powerpc/mm/init-common.c
+++ b/arch/powerpc/mm/init-common.c
@@ -78,13 +78,13 @@ void pgtable_cache_init(void)
{
pgtable_cache_add(PGD_INDEX_SIZE);
- if (PMD_CACHE_INDEX && !PGT_CACHE(PMD_CACHE_INDEX))
+ if (PMD_CACHE_INDEX)
pgtable_cache_add(PMD_CACHE_INDEX);
/*
* In all current configs, when the PUD index exists it's the
* same size as either the pgd or pmd index except with THP enabled
* on book3s 64
*/
- if (PUD_CACHE_INDEX && !PGT_CACHE(PUD_CACHE_INDEX))
+ if (PUD_CACHE_INDEX)
pgtable_cache_add(PUD_CACHE_INDEX);
}
--
2.13.3
^ permalink raw reply related
* [PATCH 2/3] powerpc/mm: remove ctor argument to pgtable_cache_add()
From: Christophe Leroy @ 2018-08-13 13:27 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
aneesh.kumar
Cc: linux-kernel, linuxppc-dev
In-Reply-To: <1cbc8dba537393edbb6384b67cbb123431ba6a32.1534166756.git.christophe.leroy@c-s.fr>
As previous patch has removed all pgtable cache constructors,
lets remove it completely.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/pgtable.h | 2 +-
arch/powerpc/mm/hugetlbpage.c | 2 +-
arch/powerpc/mm/init-common.c | 10 +++++-----
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
index 14c79a7dc855..d3195ac00a0b 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -72,7 +72,7 @@ extern int gup_hugepte(pte_t *ptep, unsigned long sz, unsigned long addr,
/* can we use this in kvm */
unsigned long vmalloc_to_phys(void *vmalloc_addr);
-void pgtable_cache_add(unsigned shift, void (*ctor)(void *));
+void pgtable_cache_add(unsigned shift);
void pgtable_cache_init(void);
#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_PPC32)
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 7296a42eb62e..72f31fc70b8e 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -701,7 +701,7 @@ static int __init hugetlbpage_init(void)
* use pgt cache for hugepd.
*/
if (pdshift > shift)
- pgtable_cache_add(pdshift - shift, NULL);
+ pgtable_cache_add(pdshift - shift);
#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_8xx)
else if (!hugepte_cache) {
/*
diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
index 2ae15ff8f76f..8fb182aaf87d 100644
--- a/arch/powerpc/mm/init-common.c
+++ b/arch/powerpc/mm/init-common.c
@@ -35,7 +35,7 @@ EXPORT_SYMBOL_GPL(pgtable_cache); /* used by kvm_hv module */
* everything else. Caches created by this function are used for all
* the higher level pagetables, and for hugepage pagetables.
*/
-void pgtable_cache_add(unsigned shift, void (*ctor)(void *))
+void pgtable_cache_add(unsigned shift)
{
char *name;
unsigned long table_size = sizeof(void *) << shift;
@@ -63,7 +63,7 @@ void pgtable_cache_add(unsigned shift, void (*ctor)(void *))
align = max_t(unsigned long, align, minalign);
name = kasprintf(GFP_KERNEL, "pgtable-2^%d", shift);
- new = kmem_cache_create(name, table_size, align, 0, ctor);
+ new = kmem_cache_create(name, table_size, align, 0, NULL);
if (!new)
panic("Could not allocate pgtable cache for order %d", shift);
@@ -76,15 +76,15 @@ EXPORT_SYMBOL_GPL(pgtable_cache_add); /* used by kvm_hv module */
void pgtable_cache_init(void)
{
- pgtable_cache_add(PGD_INDEX_SIZE, NULL);
+ pgtable_cache_add(PGD_INDEX_SIZE);
if (PMD_CACHE_INDEX && !PGT_CACHE(PMD_CACHE_INDEX))
- pgtable_cache_add(PMD_CACHE_INDEX, NULL);
+ pgtable_cache_add(PMD_CACHE_INDEX);
/*
* In all current configs, when the PUD index exists it's the
* same size as either the pgd or pmd index except with THP enabled
* on book3s 64
*/
if (PUD_CACHE_INDEX && !PGT_CACHE(PUD_CACHE_INDEX))
- pgtable_cache_add(PUD_CACHE_INDEX, NULL);
+ pgtable_cache_add(PUD_CACHE_INDEX);
}
--
2.13.3
^ permalink raw reply related
* [PATCH 1/3] powerpc/mm: fix a warning when a cache is common to PGD and hugepages
From: Christophe Leroy @ 2018-08-13 13:27 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
aneesh.kumar
Cc: linux-kernel, linuxppc-dev
While implementing TLB miss HW assistance on the 8xx, the following
warning was encountered:
[ 423.732965] WARNING: CPU: 0 PID: 345 at mm/slub.c:2412 ___slab_alloc.constprop.30+0x26c/0x46c
[ 423.733033] CPU: 0 PID: 345 Comm: mmap Not tainted 4.18.0-rc8-00664-g2dfff9121c55 #671
[ 423.733075] NIP: c0108f90 LR: c0109ad0 CTR: 00000004
[ 423.733121] REGS: c455bba0 TRAP: 0700 Not tainted (4.18.0-rc8-00664-g2dfff9121c55)
[ 423.733147] MSR: 00021032 <ME,IR,DR,RI> CR: 24224848 XER: 20000000
[ 423.733319]
[ 423.733319] GPR00: c0109ad0 c455bc50 c4521910 c60053c0 007080c0 c0011b34 c7fa41e0 c455be30
[ 423.733319] GPR08: 00000001 c00103a0 c7fa41e0 c49afcc4 24282842 10018840 c079b37c 00000040
[ 423.733319] GPR16: 73f00000 00210d00 00000000 00000001 c455a000 00000100 00000200 c455a000
[ 423.733319] GPR24: c60053c0 c0011b34 007080c0 c455a000 c455a000 c7fa41e0 00000000 00009032
[ 423.734190] NIP [c0108f90] ___slab_alloc.constprop.30+0x26c/0x46c
[ 423.734257] LR [c0109ad0] kmem_cache_alloc+0x210/0x23c
[ 423.734283] Call Trace:
[ 423.734326] [c455bc50] [00000100] 0x100 (unreliable)
[ 423.734430] [c455bcc0] [c0109ad0] kmem_cache_alloc+0x210/0x23c
[ 423.734543] [c455bcf0] [c0011b34] huge_pte_alloc+0xc0/0x1dc
[ 423.734633] [c455bd20] [c01044dc] hugetlb_fault+0x408/0x48c
[ 423.734720] [c455bdb0] [c0104b20] follow_hugetlb_page+0x14c/0x44c
[ 423.734826] [c455be10] [c00e8e54] __get_user_pages+0x1c4/0x3dc
[ 423.734919] [c455be80] [c00e9924] __mm_populate+0xac/0x140
[ 423.735020] [c455bec0] [c00db14c] vm_mmap_pgoff+0xb4/0xb8
[ 423.735127] [c455bf00] [c00f27c0] ksys_mmap_pgoff+0xcc/0x1fc
[ 423.735222] [c455bf40] [c000e0f8] ret_from_syscall+0x0/0x38
[ 423.735271] Instruction dump:
[ 423.735321] 7cbf482e 38fd0008 7fa6eb78 7fc4f378 4bfff5dd 7fe3fb78 4bfffe24 81370010
[ 423.735536] 71280004 41a2ff88 4840c571 4bffff80 <0fe00000> 4bfffeb8 81340010 712a0004
[ 423.735757] ---[ end trace e9b222919a470790 ]---
This warning occurs when calling kmem_cache_zalloc() on a
cache having a constructor.
In this case it happens because PGD cache and 512k hugepte cache are
the same size (4k). While a cache with constructor is created for
the PGD, hugepages create cache without constructor and uses
kmem_cache_zalloc(). As both expect a cache with the same size,
the hugepages reuse the cache created for PGD, hence the conflict.
As the constructors only aim at zeroing the allocated memory, this
patch fixes this issue by removing the constructors and using
kmem_cache_zalloc() instead.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/book3s/32/pgalloc.h | 2 +-
arch/powerpc/include/asm/book3s/64/pgalloc.h | 4 ++--
arch/powerpc/include/asm/nohash/32/pgalloc.h | 2 +-
arch/powerpc/include/asm/nohash/64/pgalloc.h | 6 +++---
arch/powerpc/mm/init-common.c | 21 +++------------------
5 files changed, 10 insertions(+), 25 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/pgalloc.h b/arch/powerpc/include/asm/book3s/32/pgalloc.h
index 82e44b1a00ae..4c23cc1ae7a1 100644
--- a/arch/powerpc/include/asm/book3s/32/pgalloc.h
+++ b/arch/powerpc/include/asm/book3s/32/pgalloc.h
@@ -32,7 +32,7 @@ extern struct kmem_cache *pgtable_cache[];
static inline pgd_t *pgd_alloc(struct mm_struct *mm)
{
- return kmem_cache_alloc(PGT_CACHE(PGD_INDEX_SIZE),
+ return kmem_cache_zalloc(PGT_CACHE(PGD_INDEX_SIZE),
pgtable_gfp_flags(mm, GFP_KERNEL));
}
diff --git a/arch/powerpc/include/asm/book3s/64/pgalloc.h b/arch/powerpc/include/asm/book3s/64/pgalloc.h
index 76234a14b97d..074359cd632a 100644
--- a/arch/powerpc/include/asm/book3s/64/pgalloc.h
+++ b/arch/powerpc/include/asm/book3s/64/pgalloc.h
@@ -81,7 +81,7 @@ static inline pgd_t *pgd_alloc(struct mm_struct *mm)
if (radix_enabled())
return radix__pgd_alloc(mm);
- pgd = kmem_cache_alloc(PGT_CACHE(PGD_INDEX_SIZE),
+ pgd = kmem_cache_zalloc(PGT_CACHE(PGD_INDEX_SIZE),
pgtable_gfp_flags(mm, GFP_KERNEL));
/*
* Don't scan the PGD for pointers, it contains references to PUDs but
@@ -120,7 +120,7 @@ static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long addr)
{
pud_t *pud;
- pud = kmem_cache_alloc(PGT_CACHE(PUD_CACHE_INDEX),
+ pud = kmem_cache_zalloc(PGT_CACHE(PUD_CACHE_INDEX),
pgtable_gfp_flags(mm, GFP_KERNEL));
/*
* Tell kmemleak to ignore the PUD, that means don't scan it for
diff --git a/arch/powerpc/include/asm/nohash/32/pgalloc.h b/arch/powerpc/include/asm/nohash/32/pgalloc.h
index 8825953c225b..766cf0c90d19 100644
--- a/arch/powerpc/include/asm/nohash/32/pgalloc.h
+++ b/arch/powerpc/include/asm/nohash/32/pgalloc.h
@@ -32,7 +32,7 @@ extern struct kmem_cache *pgtable_cache[];
static inline pgd_t *pgd_alloc(struct mm_struct *mm)
{
- return kmem_cache_alloc(PGT_CACHE(PGD_INDEX_SIZE),
+ return kmem_cache_zalloc(PGT_CACHE(PGD_INDEX_SIZE),
pgtable_gfp_flags(mm, GFP_KERNEL));
}
diff --git a/arch/powerpc/include/asm/nohash/64/pgalloc.h b/arch/powerpc/include/asm/nohash/64/pgalloc.h
index e2d62d033708..54ee5ac02d81 100644
--- a/arch/powerpc/include/asm/nohash/64/pgalloc.h
+++ b/arch/powerpc/include/asm/nohash/64/pgalloc.h
@@ -43,7 +43,7 @@ extern struct kmem_cache *pgtable_cache[];
static inline pgd_t *pgd_alloc(struct mm_struct *mm)
{
- return kmem_cache_alloc(PGT_CACHE(PGD_INDEX_SIZE),
+ return kmem_cache_zalloc(PGT_CACHE(PGD_INDEX_SIZE),
pgtable_gfp_flags(mm, GFP_KERNEL));
}
@@ -56,7 +56,7 @@ static inline void pgd_free(struct mm_struct *mm, pgd_t *pgd)
static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long addr)
{
- return kmem_cache_alloc(PGT_CACHE(PUD_INDEX_SIZE),
+ return kmem_cache_zalloc(PGT_CACHE(PUD_INDEX_SIZE),
pgtable_gfp_flags(mm, GFP_KERNEL));
}
@@ -86,7 +86,7 @@ static inline void pmd_populate(struct mm_struct *mm, pmd_t *pmd,
static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long addr)
{
- return kmem_cache_alloc(PGT_CACHE(PMD_CACHE_INDEX),
+ return kmem_cache_zalloc(PGT_CACHE(PMD_CACHE_INDEX),
pgtable_gfp_flags(mm, GFP_KERNEL));
}
diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
index 2b656e67f2ea..2ae15ff8f76f 100644
--- a/arch/powerpc/mm/init-common.c
+++ b/arch/powerpc/mm/init-common.c
@@ -25,21 +25,6 @@
#include <asm/pgalloc.h>
#include <asm/pgtable.h>
-static void pgd_ctor(void *addr)
-{
- memset(addr, 0, PGD_TABLE_SIZE);
-}
-
-static void pud_ctor(void *addr)
-{
- memset(addr, 0, PUD_TABLE_SIZE);
-}
-
-static void pmd_ctor(void *addr)
-{
- memset(addr, 0, PMD_TABLE_SIZE);
-}
-
struct kmem_cache *pgtable_cache[MAX_PGTABLE_INDEX_SIZE];
EXPORT_SYMBOL_GPL(pgtable_cache); /* used by kvm_hv module */
@@ -91,15 +76,15 @@ EXPORT_SYMBOL_GPL(pgtable_cache_add); /* used by kvm_hv module */
void pgtable_cache_init(void)
{
- pgtable_cache_add(PGD_INDEX_SIZE, pgd_ctor);
+ pgtable_cache_add(PGD_INDEX_SIZE, NULL);
if (PMD_CACHE_INDEX && !PGT_CACHE(PMD_CACHE_INDEX))
- pgtable_cache_add(PMD_CACHE_INDEX, pmd_ctor);
+ pgtable_cache_add(PMD_CACHE_INDEX, NULL);
/*
* In all current configs, when the PUD index exists it's the
* same size as either the pgd or pmd index except with THP enabled
* on book3s 64
*/
if (PUD_CACHE_INDEX && !PGT_CACHE(PUD_CACHE_INDEX))
- pgtable_cache_add(PUD_CACHE_INDEX, pud_ctor);
+ pgtable_cache_add(PUD_CACHE_INDEX, NULL);
}
--
2.13.3
^ permalink raw reply related
* [PATCH] powerpc/mm: Don't report hugepage tables as memory leaks when using kmemleak
From: Christophe Leroy @ 2018-08-13 13:19 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linux-kernel, linuxppc-dev
When a process allocates a hugepage, the following leak is
reported by kmemleak. This is a false positive which is
due to the pointer to the table being stored in the PGD
as physical memory address and not virtual memory pointer.
unreferenced object 0xc30f8200 (size 512):
comm "mmap", pid 374, jiffies 4872494 (age 627.630s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<e32b68da>] huge_pte_alloc+0xdc/0x1f8
[<9e0df1e1>] hugetlb_fault+0x560/0x8f8
[<7938ec6c>] follow_hugetlb_page+0x14c/0x44c
[<afbdb405>] __get_user_pages+0x1c4/0x3dc
[<b8fd7cd9>] __mm_populate+0xac/0x140
[<3215421e>] vm_mmap_pgoff+0xb4/0xb8
[<c148db69>] ksys_mmap_pgoff+0xcc/0x1fc
[<4fcd760f>] ret_from_syscall+0x0/0x38
See commit a984506c542e2 ("powerpc/mm: Don't report PUDs as
memory leaks when using kmemleak") for detailed explanation.
To fix that, this patch tells kmemleak to ignore the allocated
hugepage table.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/mm/hugetlbpage.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index e87f9ef9115b..7296a42eb62e 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -19,6 +19,7 @@
#include <linux/moduleparam.h>
#include <linux/swap.h>
#include <linux/swapops.h>
+#include <linux/kmemleak.h>
#include <asm/pgtable.h>
#include <asm/pgalloc.h>
#include <asm/tlb.h>
@@ -112,6 +113,8 @@ static int __hugepte_alloc(struct mm_struct *mm, hugepd_t *hpdp,
for (i = i - 1 ; i >= 0; i--, hpdp--)
*hpdp = __hugepd(0);
kmem_cache_free(cachep, new);
+ } else {
+ kmemleak_ignore(new);
}
spin_unlock(ptl);
return 0;
--
2.13.3
^ permalink raw reply related
* Re: [PATCH v6 2/2] powerpc: Use cpu_smallcore_sibling_mask at SMT level on bigcores
From: Gautham R Shenoy @ 2018-08-13 12:07 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Gautham R. Shenoy, Michael Ellerman, Benjamin Herrenschmidt,
Michael Neuling, Vaidyanathan Srinivasan, Akshay Adiga,
Shilpasri G Bhat, Oliver O'Halloran, Nicholas Piggin,
Murilo Opsfelder Araujo, Anton Blanchard, linuxppc-dev,
linux-kernel
In-Reply-To: <20180809132657.GA42474@linux.vnet.ibm.com>
On Thu, Aug 09, 2018 at 06:26:57AM -0700, Srikar Dronamraju wrote:
> * Gautham R. Shenoy <ego@linux.vnet.ibm.com> [2018-08-09 11:02:08]:
>
> >
> > 3) ppc64_cpu --smt=2
> > SMT domain ceases to exist as each domain consists of just one
> > group.
> >
>
> When seen in isolation, the above looks as if ppc64_cpu --smt=2 o/p says
> " SMT domain ceases to exist...."
Ok. The intent was to say that one of the sched-domain level
collapses, thereby leaving only CACHE, DIE and NUMA. Will word it
better.
>
> > @@ -999,7 +1012,17 @@ static void add_cpu_to_masks(int cpu)
> > {
> > int first_thread = cpu_first_thread_sibling(cpu);
> > int chipid = cpu_to_chip_id(cpu);
> > - int i;
> > +
> > + struct thread_groups tg;
> > + int i, cpu_group_start = -1;
> > +
> > + if (has_big_cores) {
> > + struct device_node *dn = of_get_cpu_node(cpu, NULL);
> > +
>
> Not checking for validity of dn and no of_node_puts?
Will fix this. Thanks for catching this.
>
> > + parse_thread_groups(dn, &tg);
> > + cpu_group_start = get_cpu_thread_group_start(cpu, &tg);
> > + cpumask_set_cpu(cpu, cpu_smallcore_sibling_mask(cpu));
> > + }
> >
> > /*
> > * This CPU will not be in the online mask yet so we need to manually
>
> The rest looks good
Thanks for the review.
>
^ permalink raw reply
* Re: [PATCH v6 1/2] powerpc: Detect the presence of big-cores via "ibm,thread-groups"
From: Gautham R Shenoy @ 2018-08-13 11:36 UTC (permalink / raw)
To: Srikar Dronamraju
Cc: Gautham R. Shenoy, Michael Ellerman, Benjamin Herrenschmidt,
Michael Neuling, Vaidyanathan Srinivasan, Akshay Adiga,
Shilpasri G Bhat, Oliver O'Halloran, Nicholas Piggin,
Murilo Opsfelder Araujo, Anton Blanchard, linuxppc-dev,
linux-kernel
In-Reply-To: <20180809132743.GB42474@linux.vnet.ibm.com>
Hi Srikar,
Thanks for reviewing the patch.
On Thu, Aug 09, 2018 at 06:27:43AM -0700, Srikar Dronamraju wrote:
> * Gautham R. Shenoy <ego@linux.vnet.ibm.com> [2018-08-09 11:02:07]:
>
> >
> > int threads_per_core, threads_per_subcore, threads_shift;
> > +bool has_big_cores;
> > cpumask_t threads_core_mask;
> > EXPORT_SYMBOL_GPL(threads_per_core);
> > EXPORT_SYMBOL_GPL(threads_per_subcore);
> > EXPORT_SYMBOL_GPL(threads_shift);
> > +EXPORT_SYMBOL_GPL(has_big_cores);
>
> Why do we need EXPORT_SYMBOL_GPL?
As Christoph pointed out, I was blindly following the suit.
You are right, we don't need to export it at the moment. The remaining
EXPORT_SYMBOL_GPL are required by KVM. However, as of now, there is no
need for "has_big_cores" in the KVM.
Will remove this in the next version.
>
> > EXPORT_SYMBOL_GPL(threads_core_mask);
> >
> > + *
> > + * Returns 0 on success, -EINVAL if the property does not exist,
> > + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> > + * property data isn't large enough.
> > + */
> > +int parse_thread_groups(struct device_node *dn,
> > + struct thread_groups *tg)
> > +{
> > + unsigned int nr_groups, threads_per_group, property;
> > + int i;
> > + u32 thread_group_array[3 + MAX_THREAD_LIST_SIZE];
> > + u32 *thread_list;
> > + size_t total_threads;
> > + int ret;
> > +
> > + ret = of_property_read_u32_array(dn, "ibm,thread-groups",
> > + thread_group_array, 3);
> > +
> > + if (ret)
> > + goto out_err;
> > +
> > + property = thread_group_array[0];
> > + nr_groups = thread_group_array[1];
> > + threads_per_group = thread_group_array[2];
> > + total_threads = nr_groups * threads_per_group;
> > +
>
> Shouldnt we check for property and nr_groups
> If the property is not 1 and nr_groups < 1, we should error out
> No point in calling a of_property read if property is not right.
Yes, the nr_groups < 1 check can be moved into this function.
However, this function merely parses the the thread group structure
exposed by the device tree. So it should error out only if there is a
failure in parsing, or as you said the parsed values are incorrect
(nr_groups < 1, threads_per_group < 1, etc). Whether the thread group
is relevant or not (in this case we are interested in thread groups
that share L1 cache, translation etc) is something for the caller to
decide.
However, I see what you mean. We can avoid parsing the remainder of
the array if the property in the device-tree isn't the property that
the caller is interested in.
This can be solved by passing the interested property value as a
parameter and so that the code errors out if this property doesn't
match the property in the device-tree. Will add this in the next
version.
>
>
> Nit:
> Cant we directly assign to tg->property, and hence avoid local
> variables, property, nr_groups and threads_per_group?
Will clean this up. This was from an older version where I added the
local variables so that the statements referencing them don't need to
be split across multiple lines. However, the code has been optimized
since then. So, the local variables are not needed.
>
> > + ret = of_property_read_u32_array(dn, "ibm,thread-groups",
> > + thread_group_array,
> > + 3 + total_threads);
> > +
> > +static inline bool dt_has_big_core(struct device_node *dn,
> > + struct thread_groups *tg)
> > +{
> > + if (parse_thread_groups(dn, tg))
> > + return false;
> > +
> > + if (tg->property != 1)
> > + return false;
> > +
> > + if (tg->nr_groups < 1)
> > + return false;
>
> Can avoid these check if we can check in parse_thread_groups.
>
> > /**
> > * setup_cpu_maps - initialize the following cpu maps:
> > * cpu_possible_mask
> > @@ -457,6 +605,7 @@ void __init smp_setup_cpu_maps(void)
> > int cpu = 0;
> > int nthreads = 1;
> >
> > diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> > index 755dc98..f5717de 100644
> > --- a/arch/powerpc/kernel/sysfs.c
> > +++ b/arch/powerpc/kernel/sysfs.c
> > @@ -18,6 +18,7 @@
> > #include <asm/smp.h>
> > #include <asm/pmc.h>
> > #include <asm/firmware.h>
> > +#include <asm/cputhreads.h>
> >
> > #include "cacheinfo.h"
> > #include "setup.h"
> > @@ -1025,6 +1026,33 @@ static ssize_t show_physical_id(struct device *dev,
> > }
> > static DEVICE_ATTR(physical_id, 0444, show_physical_id, NULL);
> >
> > +static ssize_t show_small_core_siblings(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct cpu *cpu = container_of(dev, struct cpu, dev);
> > + struct device_node *dn = of_get_cpu_node(cpu->dev.id, NULL);
> > + struct thread_groups tg;
> > + int i, j;
> > + ssize_t ret = 0;
> > +
>
> Here we need to check for validity of dn and error out accordingly.
Will add this check.
>
>
> > + if (parse_thread_groups(dn, &tg))
> > + return -ENODATA;
>
> Did we miss a of_node_put(dn)?
Yes we did. Will fix this.
>
> > +
> > + i = get_cpu_thread_group_start(cpu->dev.id, &tg);
> > +
> > + if (i == -1)
> > + return -ENODATA;
> > +
> > + for (j = 0; j < tg.threads_per_group - 1; j++)
> > + ret += sprintf(buf + ret, "%d,", tg.thread_list[i + j]);
>
> Here, we are making the assumption that group_start will always be the
> first thread in the thread_group. However we didnt make the same
> assumption in get_cpu_thread_group_start.
Above the get_cpu_thread_group_start function , we have the following
comment which indicates that group_start will point to the start of
the thread group. Is this what you are referring to?
/*
* Returns the index to tg->thread_list that points to the the start
* of the thread_group that @cpu belongs to.
*
* Returns -1 if cpu doesn't belong to any of the groups pointed to by
* tg->thread_list.
*/
^ permalink raw reply
* Re: cxl: remove a dead branch
From: Mathieu Malaterre @ 2018-08-13 11:25 UTC (permalink / raw)
To: Frederic Barrat
Cc: linuxppc-dev, Christophe Lombard, Andrew Donnellan,
Michael Ellerman
In-Reply-To: <41ptcv2xyYz9sC7@ozlabs.org>
Frederic,
Could you double check with Michael what is now best to do.
Thanks
On Mon, Aug 13, 2018 at 1:23 PM Michael Ellerman
<patch-notifications@ellerman.id.au> wrote:
>
> On Thu, 2018-03-22 at 21:05:28 UTC, Mathieu Malaterre wrote:
> > In commit 14baf4d9c739 ("cxl: Add guest-specific code") the following code
> > was added:
> >
> > if (afu->crs_len < 0) {
> > dev_err(&afu->dev, "Unexpected configuration record size value\n");
> > return -EINVAL;
> > }
> >
> > However the variable `crs_len` is of type u64 and cannot be compared < 0.
> > Remove the dead code section. Fix the following warning treated as error
> > with W=1:
> >
> > ../drivers/misc/cxl/guest.c:919:19: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
> >
> > Signed-off-by: Mathieu Malaterre <malat@debian.org>
>
> Applied to powerpc next, thanks.
>
> https://git.kernel.org/powerpc/c/e4ecafb14fd9cd77d8f4320af1922e
>
> cheers
^ permalink raw reply
* Re: [V2] powerpc/mm/book3s/radix: Add mapping statistics
From: Michael Ellerman @ 2018-08-13 11:23 UTC (permalink / raw)
To: Aneesh Kumar K.V, npiggin, benh, paulus; +Cc: Aneesh Kumar K.V, linuxppc-dev
In-Reply-To: <20180813054457.13241-1-aneesh.kumar@linux.ibm.com>
On Mon, 2018-08-13 at 05:44:57 UTC, "Aneesh Kumar K.V" wrote:
> Add statistics that show how memory is mapped within the kernel linear mapping.
> This is similar to commit 37cd944c8d8f ("s390/pgtable: add mapping statistics")
>
> We don't do this with Hash translation mode. Hash uses one size (mmu_linear_psize)
> to map the kernel linear mapping and we print the linear psize during boot as
> below.
>
> "Page orders: linear mapping = 24, virtual = 16, io = 16, vmemmap = 24"
>
> A sample output looks like:
>
> DirectMap4k: 0 kB
> DirectMap64k: 18432 kB
> DirectMap2M: 1030144 kB
> DirectMap1G: 11534336 kB
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/a2dc009afa9ae8b92305be77286765
cheers
^ permalink raw reply
* Re: powerpc/uaccess: Enable get_user(u64, *p) on 32-bit
From: Michael Ellerman @ 2018-08-13 11:23 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
In-Reply-To: <20180810122535.11710-1-mpe@ellerman.id.au>
On Fri, 2018-08-10 at 12:25:35 UTC, Michael Ellerman wrote:
> Currently if you build a 32-bit powerpc kernel and use get_user() to
> load a u64 value it will fail to build with eg:
>
> kernel/rseq.o: In function `rseq_get_rseq_cs':
> kernel/rseq.c:123: undefined reference to `__get_user_bad'
>
> This is hitting the check in __get_user_size() that makes sure the
> size we're copying doesn't exceed the size of the destination:
>
> #define __get_user_size(x, ptr, size, retval)
> do {
> retval = 0;
> __chk_user_ptr(ptr);
> if (size > sizeof(x))
> (x) = __get_user_bad();
>
> Which doesn't immediately make sense because the size of the
> destination is u64, but it's not really, because __get_user_check()
> etc. internally create an unsigned long and copy into that:
>
> #define __get_user_check(x, ptr, size)
> ({
> long __gu_err = -EFAULT;
> unsigned long __gu_val = 0;
>
> The problem being that on 32-bit unsigned long is not big enough to
> hold a u64. We can fix this with a trick from hpa in the x86 code, we
> statically check the type of x and set the type of __gu_val to either
> unsigned long or unsigned long long.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Applied to powerpc next.
https://git.kernel.org/powerpc/c/f7a6947cd49b7ff4e03f1b4f7e7b22
cheers
^ permalink raw reply
* Re: [v2, 1/2] powerpc/64s: move machine check SLB flushing to mm/slb.c
From: Michael Ellerman @ 2018-08-13 11:23 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: Gautham R . Shenoy, Mahesh Jagannath Salgaonkar, Nicholas Piggin,
kvm-ppc, Aneesh Kumar K.V, Akshay Adiga
In-Reply-To: <20180810064249.13724-1-npiggin@gmail.com>
On Fri, 2018-08-10 at 06:42:48 UTC, Nicholas Piggin wrote:
> The machine check code that flushes and restores bolted segments in
> real mode belongs in mm/slb.c. This will also be used by pseries
> machine check and idle code in future changes.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/e7e81847478b37a3958a3163171bf6
cheers
^ permalink raw reply
* Re: powerpc/powernv/idle: Fix build error
From: Michael Ellerman @ 2018-08-13 11:23 UTC (permalink / raw)
To: Aneesh Kumar K.V, npiggin, benh, paulus; +Cc: Aneesh Kumar K.V, linuxppc-dev
In-Reply-To: <20180809133720.16406-1-aneesh.kumar@linux.ibm.com>
On Thu, 2018-08-09 at 13:37:20 UTC, "Aneesh Kumar K.V" wrote:
> Fix the below build error using strlcpy instead of strncpy
>
> In function 'pnv_parse_cpuidle_dt',
> inlined from 'pnv_init_idle_states' at arch/powerpc/platforms/powernv/idle.c:840:7,
> inlined from '__machine_initcall_powernv_pnv_init_idle_states' at arch/powerpc/platforms/powernv/idle.c:870:1:
> arch/powerpc/platforms/powernv/idle.c:820:3: error: 'strncpy' specified bound 16 equals destination size [-Werror=stringop-truncation]
> strncpy(pnv_idle_states[i].name, temp_string[i],
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> PNV_IDLE_NAME_LEN);
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/ae24ce5e12127eeef6bf946c3ee0e9
cheers
^ permalink raw reply
* Re: powerpc/mm/tlbflush: update the mmu_gather page size while iterating address range
From: Michael Ellerman @ 2018-08-13 11:23 UTC (permalink / raw)
To: Aneesh Kumar K.V, npiggin, benh, paulus; +Cc: Aneesh Kumar K.V, linuxppc-dev
In-Reply-To: <20180809133659.16230-1-aneesh.kumar@linux.ibm.com>
On Thu, 2018-08-09 at 13:36:59 UTC, "Aneesh Kumar K.V" wrote:
> This patch makes sure we update the mmu_gather page size even if we are
> requesting for a fullmm flush. This avoids triggering VM_WARN_ON in code
> paths like __tlb_remove_page_size that explicitly check for removing range page
> size to be same as mmu gather page size.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Acked-by: Nicholas Piggin <npiggin@gmail.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/0b6aa1a20add96437c46db77c9bae2
cheers
^ permalink raw reply
* Re: powerpc/mm/hash: Remove unnecessary do { }while(0) loop
From: Michael Ellerman @ 2018-08-13 11:23 UTC (permalink / raw)
To: Aneesh Kumar K.V, npiggin, benh, paulus; +Cc: Aneesh Kumar K.V, linuxppc-dev
In-Reply-To: <20180809133642.16007-1-aneesh.kumar@linux.ibm.com>
On Thu, 2018-08-09 at 13:36:42 UTC, "Aneesh Kumar K.V" wrote:
> Avoid coverity false warnings like
>
> *** CID 187347: Control flow issues (UNREACHABLE)
> /arch/powerpc/mm/hash_native_64.c: 819 in native_flush_hash_range()
> 813 slot += hidx & _PTEIDX_GROUP_IX;
> 814 hptep = htab_address + slot;
> 815 want_v = hpte_encode_avpn(vpn, psize, ssize);
> 816 hpte_v = hpte_get_old_v(hptep);
> 817
> 818 if (!HPTE_V_COMPARE(hpte_v, want_v) || !(hpte_v & HPTE_V_VALID))
> >>> CID 187347: Control flow issues (UNREACHABLE)
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/f405b510c93eeb7390d0e2c6ef8d12
cheers
^ permalink raw reply
* Re: powerpc/lib: Use patch_site to patch copy_32 functions once cache is enabled
From: Michael Ellerman @ 2018-08-13 11:23 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <da0c182b76c27988b88ebd35ee284f5be5882dd1.1533802042.git.christophe.leroy@c-s.fr>
On Thu, 2018-08-09 at 08:14:41 UTC, Christophe Leroy wrote:
> The symbol memcpy_nocache_branch defined in order to allow patching
> of memset function once cache is enabled leads to confusing reports
> by perf tool.
>
> Using the new patch_site functionality solves this issue.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/fa54a981ea7a852c145b05c95abba1
cheers
^ permalink raw reply
* Re: powerpc/mm: remove huge_pte_offset_and_shift() prototype
From: Michael Ellerman @ 2018-08-13 11:23 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <ea622e15bc0c7e72ee94beaa609fa0df60f3a051.1533742526.git.christophe.leroy@c-s.fr>
On Wed, 2018-08-08 at 15:36:34 UTC, Christophe Leroy wrote:
> huge_pte_offset_and_shift() has never existed
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/646dbe40fa2a54118975792fa9b98c
cheers
^ permalink raw reply
* Re: powerpc: fix size calculation using resource_size()
From: Michael Ellerman @ 2018-08-13 11:23 UTC (permalink / raw)
To: Dan Carpenter, Benjamin Herrenschmidt, Jia Hongtao
Cc: Rob Herring, kernel-janitors, Paul Mackerras, Tyrel Datwyler,
linuxppc-dev
In-Reply-To: <20180808115724.gh5mbbi6twzqbqao@kili.mountain>
On Wed, 2018-08-08 at 11:57:24 UTC, Dan Carpenter wrote:
> The problem is the the calculation should be "end - start + 1" but the
> plus one is missing in this calculation.
>
> Fixes: 8626816e905e ("powerpc: add support for MPIC message register API")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/c42d3be0c06f0c1c416054022aa535
cheers
^ permalink raw reply
* Re: [v7, 3/9] powerpc/pseries: Fix endainness while restoring of r3 in MCE handler.
From: Michael Ellerman @ 2018-08-13 11:23 UTC (permalink / raw)
To: Mahesh J Salgaonkar, linuxppc-dev
Cc: Michal Suchanek, Ananth Narayan, Nicholas Piggin, stable,
Laurent Dufour, Aneesh Kumar K.V
In-Reply-To: <153365139399.14256.13555279013971561179.stgit@jupiter.in.ibm.com>
On Tue, 2018-08-07 at 14:16:46 UTC, Mahesh J Salgaonkar wrote:
> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>
> During Machine Check interrupt on pseries platform, register r3 points
> RTAS extended event log passed by hypervisor. Since hypervisor uses r3
> to pass pointer to rtas log, it stores the original r3 value at the
> start of the memory (first 8 bytes) pointed by r3. Since hypervisor
> stores this info and rtas log is in BE format, linux should make
> sure to restore r3 value in correct endian format.
>
> Without this patch when MCE handler, after recovery, returns to code that
> that caused the MCE may end up with Data SLB access interrupt for invalid
> address followed by kernel panic or hang.
>
> [ 62.878965] Severe Machine check interrupt [Recovered]
> [ 62.878968] NIP [d00000000ca301b8]: init_module+0x1b8/0x338 [bork_kernel]
> [ 62.878969] Initiator: CPU
> [ 62.878970] Error type: SLB [Multihit]
> [ 62.878971] Effective address: d00000000ca70000
> cpu 0xa: Vector: 380 (Data SLB Access) at [c0000000fc7775b0]
> pc: c0000000009694c0: vsnprintf+0x80/0x480
> lr: c0000000009698e0: vscnprintf+0x20/0x60
> sp: c0000000fc777830
> msr: 8000000002009033
> dar: a803a30c000000d0
> current = 0xc00000000bc9ef00
> paca = 0xc00000001eca5c00 softe: 3 irq_happened: 0x01
> pid = 8860, comm = insmod
> [c0000000fc7778b0] c0000000009698e0 vscnprintf+0x20/0x60
> [c0000000fc7778e0] c00000000016b6c4 vprintk_emit+0xb4/0x4b0
> [c0000000fc777960] c00000000016d40c vprintk_func+0x5c/0xd0
> [c0000000fc777980] c00000000016cbb4 printk+0x38/0x4c
> [c0000000fc7779a0] d00000000ca301c0 init_module+0x1c0/0x338 [bork_kernel]
> [c0000000fc777a40] c00000000000d9c4 do_one_initcall+0x54/0x230
> [c0000000fc777b00] c0000000001b3b74 do_init_module+0x8c/0x248
> [c0000000fc777b90] c0000000001b2478 load_module+0x12b8/0x15b0
> [c0000000fc777d30] c0000000001b29e8 sys_finit_module+0xa8/0x110
> [c0000000fc777e30] c00000000000b204 system_call+0x58/0x6c
> --- Exception: c00 (System Call) at 00007fff8bda0644
> SP (7fffdfbfe980) is in userspace
>
> This patch fixes this issue.
>
> Fixes: a08a53ea4c97 ("powerpc/le: Enable RTAS events support")
> Cc: stable@vger.kernel.org
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/cd813e1cd7122f2c261dce5b54d1e0
cheers
^ permalink raw reply
* Re: [v3] selftests/powerpc: Kill child processes on SIGINT
From: Michael Ellerman @ 2018-08-13 11:23 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev; +Cc: Breno Leitao, Gustavo Romero
In-Reply-To: <1533651339-12816-1-git-send-email-leitao@debian.org>
On Tue, 2018-08-07 at 14:15:39 UTC, Breno Leitao wrote:
> There are some powerpc selftests, as tm/tm-unavailable, that run for a long
> period (>120 seconds), and if it is interrupted, as pressing CRTL-C
> (SIGINT), the foreground process (harness) dies but the child process and
> threads continue to execute (with PPID = 1 now) in background.
>
> In this case, you'd think the whole test exited, but there are remaining
> threads and processes being executed in background. Sometimes these
> zombies processes are doing annoying things, as consuming the whole CPU or
> dumping things to STDOUT.
>
> This patch fixes this problem by attaching an empty signal handler to
> SIGINT in the harness process. This handler will interrupt (EINTR) the
> parent process waitpid() call, letting the code to follow through the
> normal flow, which will kill all the processes in the child process group.
>
> This patch also fixes a typo.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> Signed-off-by: Gustavo Romero <gromero@linux.vnet.ibm.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/7c27a26e1ed5a7dd709aa19685d2c9
cheers
^ permalink raw reply
* Re: powerpc/cpm1: fix compilation error with CONFIG_PPC_EARLY_DEBUG_CPM
From: Michael Ellerman @ 2018-08-13 11:23 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <975595591e096f4ad3253b5a11713beaf62912c4.1533561636.git.christophe.leroy@c-s.fr>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3381 bytes --]
On Mon, 2018-08-06 at 15:09:11 UTC, Christophe Leroy wrote:
> commit e8cb7a55eb8dc ("powerpc: remove superflous inclusions of
> asm/fixmap.h") removed inclusion of asm/fixmap.h from files not
> including objects from that file.
>
> However, asm/mmu-8xx.h includes call to __fix_to_virt(). The proper
> way would be to include asm/fixmap.h in asm/mmu-8xx.h but it creates
> an inclusion loop.
>
> So we have to leave asm/fixmap.h in sysdep/cpm_common.c for
> CONFIG_PPC_EARLY_DEBUG_CPM
>
> CC arch/powerpc/sysdev/cpm_common.o
> In file included from ./arch/powerpc/include/asm/mmu.h:340:0,
> from ./arch/powerpc/include/asm/reg_8xx.h:8,
> from ./arch/powerpc/include/asm/reg.h:29,
> from ./arch/powerpc/include/asm/processor.h:13,
> from ./arch/powerpc/include/asm/thread_info.h:28,
> from ./include/linux/thread_info.h:38,
> from ./arch/powerpc/include/asm/ptrace.h:159,
> from ./arch/powerpc/include/asm/hw_irq.h:12,
> from ./arch/powerpc/include/asm/irqflags.h:12,
> from ./include/linux/irqflags.h:16,
> from ./include/asm-generic/cmpxchg-local.h:6,
> from ./arch/powerpc/include/asm/cmpxchg.h:537,
> from ./arch/powerpc/include/asm/atomic.h:11,
> from ./include/linux/atomic.h:5,
> from ./include/linux/mutex.h:18,
> from ./include/linux/kernfs.h:13,
> from ./include/linux/sysfs.h:16,
> from ./include/linux/kobject.h:20,
> from ./include/linux/device.h:16,
> from ./include/linux/node.h:18,
> from ./include/linux/cpu.h:17,
> from ./include/linux/of_device.h:5,
> from arch/powerpc/sysdev/cpm_common.c:21:
> arch/powerpc/sysdev/cpm_common.c: In function ‘udbg_init_cpm’:
> ./arch/powerpc/include/asm/mmu-8xx.h:218:25: error: implicit declaration of function ‘__fix_to_virt’ [-Werror=implicit-function-declaration]
> #define VIRT_IMMR_BASE (__fix_to_virt(FIX_IMMR_BASE))
> ^
> arch/powerpc/sysdev/cpm_common.c:75:7: note: in expansion of macro ‘VIRT_IMMR_BASE’
> VIRT_IMMR_BASE);
> ^
> ./arch/powerpc/include/asm/mmu-8xx.h:218:39: error: ‘FIX_IMMR_BASE’ undeclared (first use in this function)
> #define VIRT_IMMR_BASE (__fix_to_virt(FIX_IMMR_BASE))
> ^
> arch/powerpc/sysdev/cpm_common.c:75:7: note: in expansion of macro ‘VIRT_IMMR_BASE’
> VIRT_IMMR_BASE);
> ^
> ./arch/powerpc/include/asm/mmu-8xx.h:218:39: note: each undeclared identifier is reported only once for each function it appears in
> #define VIRT_IMMR_BASE (__fix_to_virt(FIX_IMMR_BASE))
> ^
> arch/powerpc/sysdev/cpm_common.c:75:7: note: in expansion of macro ‘VIRT_IMMR_BASE’
> VIRT_IMMR_BASE);
> ^
> cc1: all warnings being treated as errors
> make[1]: *** [arch/powerpc/sysdev/cpm_common.o] Error 1
>
> Fixes: e8cb7a55eb8dc ("powerpc: remove superflous inclusions of asm/fixmap.h")
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/6bd6d8672208e8dc0c18588d6eb458
cheers
^ permalink raw reply
* Re: [v2, 1/2] powerpc/fadump: handle crash memory ranges array index overflow
From: Michael Ellerman @ 2018-08-13 11:23 UTC (permalink / raw)
To: Hari Bathini; +Cc: Mahesh Salgaonkar, Mahesh J Salgaonkar, stable, linuxppc-dev
In-Reply-To: <153358813908.15150.18359359970445648733.stgit@hbathini.in.ibm.com>
On Mon, 2018-08-06 at 20:42:45 UTC, Hari Bathini wrote:
> Crash memory ranges is an array of memory ranges of the crashing kernel
> to be exported as a dump via /proc/vmcore file. The size of the array
> is set based on INIT_MEMBLOCK_REGIONS, which works alright in most cases
> where memblock memory regions count is less than INIT_MEMBLOCK_REGIONS
> value. But this count can grow beyond INIT_MEMBLOCK_REGIONS value since
> commit 142b45a72e22 ("memblock: Add array resizing support").
>
> On large memory systems with a few DLPAR operations, the memblock memory
> regions count could be larger than INIT_MEMBLOCK_REGIONS value. On such
> systems, registering fadump results in crash or other system failures
> like below:
>
> task: c00007f39a290010 ti: c00000000b738000 task.ti: c00000000b738000
> NIP: c000000000047df4 LR: c0000000000f9e58 CTR: c00000000010f180
> REGS: c00000000b73b570 TRAP: 0300 Tainted: G L X (4.4.140+)
> MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 22004484 XER: 20000000
> CFAR: c000000000008500 DAR: 000007a450000000 DSISR: 40000000 SOFTE: 0
> GPR00: c0000000000f9e58 c00000000b73b7f0 c000000000f09a00 000000000000001a
> GPR04: c00007f3bf774c90 0000000000000004 c000000000eb9a00 0000000000000800
> GPR08: 0000000000000804 000007a450000000 c000000000fa9a00 c00007ffb169ca20
> GPR12: 0000000022004482 c00000000fa12c00 c00007f3a0ea97a8 0000000000000000
> GPR16: c00007f3a0ea9a50 c00000000b73bd60 0000000000000118 000000000001fe80
> GPR20: 0000000000000118 0000000000000000 c000000000b8c980 00000000000000d0
> GPR24: 000007ffb0b10000 c00007ffb169c980 0000000000000000 c000000000b8c980
> GPR28: 0000000000000004 c00007ffb169c980 000000000000001a c00007ffb169c980
> NIP [c000000000047df4] smp_send_reschedule+0x24/0x80
> LR [c0000000000f9e58] resched_curr+0x138/0x160
> Call Trace:
> [c00000000b73b7f0] [c0000000000f9e58] resched_curr+0x138/0x160 (unreliable)
> [c00000000b73b820] [c0000000000fb538] check_preempt_curr+0xc8/0xf0
> [c00000000b73b850] [c0000000000fb598] ttwu_do_wakeup+0x38/0x150
> [c00000000b73b890] [c0000000000fc9c4] try_to_wake_up+0x224/0x4d0
> [c00000000b73b900] [c00000000011ef34] __wake_up_common+0x94/0x100
> [c00000000b73b960] [c00000000034a78c] ep_poll_callback+0xac/0x1c0
> [c00000000b73b9b0] [c00000000011ef34] __wake_up_common+0x94/0x100
> [c00000000b73ba10] [c00000000011f810] __wake_up_sync_key+0x70/0xa0
> [c00000000b73ba60] [c00000000067c3e8] sock_def_readable+0x58/0xa0
> [c00000000b73ba90] [c0000000007848ac] unix_stream_sendmsg+0x2dc/0x4c0
> [c00000000b73bb70] [c000000000675a38] sock_sendmsg+0x68/0xa0
> [c00000000b73bba0] [c00000000067673c] ___sys_sendmsg+0x2cc/0x2e0
> [c00000000b73bd30] [c000000000677dbc] __sys_sendmsg+0x5c/0xc0
> [c00000000b73bdd0] [c0000000006789bc] SyS_socketcall+0x36c/0x3f0
> [c00000000b73be30] [c000000000009488] system_call+0x3c/0x100
> Instruction dump:
> 4e800020 60000000 60420000 3c4c00ec 38421c30 7c0802a6 f8010010 60000000
> 3d42000a e92ab420 2fa90000 4dde0020 <e9290000> 2fa90000 419e0044 7c0802a6
> ---[ end trace a6d1dd4bab5f8253 ]---
>
> as array index overflow is not checked for while setting up crash memory
> ranges causing memory corruption. To resolve this issue, dynamically
> allocate memory for crash memory ranges and resize it incrementally,
> in units of pagesize, on hitting array size limit.
>
> Fixes: 2df173d9e85d ("fadump: Initialize elfcore header and add PT_LOAD program headers.")
> Cc: stable@vger.kernel.org
> Cc: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> Reviewed-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Series applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/1bd6a1c4b80a28d975287630644e6b
cheers
^ permalink raw reply
* Re: [1/2] powerpc: Allow memory that has been hot-removed to be hot-added
From: Michael Ellerman @ 2018-08-13 11:23 UTC (permalink / raw)
To: Rashmica Gupta, linuxppc-dev, mikey, benh, paulus, bsingharora
Cc: Rashmica Gupta
In-Reply-To: <20180803060601.724-1-rashmica.g@gmail.com>
On Fri, 2018-08-03 at 06:06:00 UTC, Rashmica Gupta wrote:
> This patch allows the memory removed by memtrace to be readded to the
> kernel. So now you don't have to reboot your system to add the memory
> back to the kernel or to have a different amount of memory removed.
>
> Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
> Tested-by: Michael Neuling <mikey@neuling.org>
Series applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/d3da701d3308ce1fa457f32c6c9e21
cheers
^ 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