* Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again
From: Mauricio Faria de Oliveira @ 2018-03-12 23:08 UTC (permalink / raw)
To: Michal Suchánek, Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <20180306135522.040707de@kitsune.suse.cz>
Hi Michael and Michal,
Got back to this; sorry for the delay.
On 03/06/2018 09:55 AM, Michal Suchánek wrote:
> Michael Ellerman<mpe@ellerman.id.au> wrote:
>> I*think* the patch below is all we need, as well as some tweaking of
>> patch 2, are you able to test and repost?
> Enabling the fallback flush always looks a bit dodgy but
> do_rfi_flush_fixups will overwrite the jump so long any other fixup is
> enabled.
I agree; the 'Using fallback displacement flush' message is misleading
(is the system slower/fallback or not? Ô_o)
The suggested patch is definitely more contained within pseries, but
the informational part seemed dodgy IMHO too.
So I wrote something with a new function parameter to force the init of
the fallback flush area (true in pseries, false in powernv). Not that
contained, but it seemed to convey the intent here in a clear way.
That's v2, just sent.
cheers,
Mauricio
^ permalink raw reply
* Re: [PATCH 2/2] misc: ocxl: use put_device() instead of device_unregister()
From: Andrew Donnellan @ 2018-03-13 0:26 UTC (permalink / raw)
To: Arvind Yadav, sudeep.dutt, ashutosh.dixit, arnd, gregkh, fbarrat
Cc: linux-kernel, linuxppc-dev
In-Reply-To: <8c4d28703e408b71284af68d07c117e7b8c8a75a.1520849284.git.arvind.yadav.cs@gmail.com>
On 12/03/18 22:36, Arvind Yadav wrote:
> if device_register() returned an error! Always use put_device()
> to give up the reference initialized.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
This looks right, thanks for picking it up.
Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCHv4 1/3] powerpc, cpu: partially unbind the mapping between cpu logical id and its seq in dt
From: Benjamin Herrenschmidt @ 2018-03-13 2:58 UTC (permalink / raw)
To: Pingfan Liu, linuxppc-dev; +Cc: kexec, mahesh, cascardo, gpiccoli, paulus, mpe
In-Reply-To: <1520829790-14029-2-git-send-email-kernelfans@gmail.com>
On Mon, 2018-03-12 at 12:43 +0800, Pingfan Liu wrote:
> For kexec -p, the boot cpu can be not the cpu0, this causes the problem
> to alloc paca[]. In theory, there is no requirement to assign cpu's logical
> id as its present seq by device tree. But we have something like
> cpu_first_thread_sibling(), which makes assumption on the mapping inside
> a core. Hence partially changing the mapping, i.e. unbind the mapping of
> core while keep the mapping inside a core. After this patch, boot-cpu
> will always be mapped into the range [0,threads_per_core).
I'm ok with the idea but not fan of the implementation:
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> ---
> arch/powerpc/include/asm/smp.h | 1 +
> arch/powerpc/kernel/prom.c | 25 ++++++++++++++-----------
> arch/powerpc/kernel/setup-common.c | 21 +++++++++++++++++++++
> 3 files changed, 36 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> index fac963e..1299100 100644
> --- a/arch/powerpc/include/asm/smp.h
> +++ b/arch/powerpc/include/asm/smp.h
> @@ -30,6 +30,7 @@
> #include <asm/percpu.h>
>
> extern int boot_cpuid;
> +extern int boot_cpuhwid;
> extern int spinning_secondaries;
>
> extern void cpu_die(void);
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index da67606..d0ebb25 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -315,8 +315,7 @@ static int __init early_init_dt_scan_cpus(unsigned long node,
> const __be32 *intserv;
> int i, nthreads;
> int len;
> - int found = -1;
> - int found_thread = 0;
> + bool found = false;
>
> /* We are scanning "cpu" nodes only */
> if (type == NULL || strcmp(type, "cpu") != 0)
> @@ -341,8 +340,11 @@ static int __init early_init_dt_scan_cpus(unsigned long node,
> if (fdt_version(initial_boot_params) >= 2) {
> if (be32_to_cpu(intserv[i]) ==
> fdt_boot_cpuid_phys(initial_boot_params)) {
> - found = boot_cpu_count;
> - found_thread = i;
> + /* always map the boot-cpu logical id into the
> + * the range of [0, thread_per_core)
> + */
> + boot_cpuid = i;
> + found = true;
> }
Call it boot_thread_id
> } else {
> /*
> @@ -351,8 +353,10 @@ static int __init early_init_dt_scan_cpus(unsigned long node,
> * off secondary threads.
> */
> if (of_get_flat_dt_prop(node,
> - "linux,boot-cpu", NULL) != NULL)
> - found = boot_cpu_count;
> + "linux,boot-cpu", NULL) != NULL) {
> + boot_cpuid = i;
> + found = true;
> + }
> }
> #ifdef CONFIG_SMP
> /* logical cpu id is always 0 on UP kernels */
> @@ -361,13 +365,12 @@ static int __init early_init_dt_scan_cpus(unsigned long node,
> }
>
> /* Not the boot CPU */
> - if (found < 0)
> + if (!found)
> return 0;
>
> - DBG("boot cpu: logical %d physical %d\n", found,
> - be32_to_cpu(intserv[found_thread]));
> - boot_cpuid = found;
> - set_hard_smp_processor_id(found, be32_to_cpu(intserv[found_thread]));
> + boot_cpuhwid = be32_to_cpu(intserv[boot_cpuid]);
> + DBG("boot cpu: logical %d physical %d\n", boot_cpuid, boot_cpuhwid);
> + set_hard_smp_processor_id(boot_cpuid, boot_cpuhwid);
>
> /*
> * PAPR defines "logical" PVR values for cpus that
> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> index 66f7cc6..1a67344 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setup-common.c
> @@ -86,6 +86,7 @@ struct machdep_calls *machine_id;
> EXPORT_SYMBOL(machine_id);
>
> int boot_cpuid = -1;
> +int boot_cpuhwid = -1;
> EXPORT_SYMBOL_GPL(boot_cpuid);
>
> /*
> @@ -459,11 +460,17 @@ static void __init cpu_init_thread_core_maps(int tpc)
> void __init smp_setup_cpu_maps(void)
> {
> struct device_node *dn;
> + struct device_node *boot_dn = NULL;
> + bool handling_bootdn = true;
> int cpu = 0;
> int nthreads = 1;
>
> DBG("smp_setup_cpu_maps()\n");
>
> +again:
> + /* E.g. kexec will not boot from the 1st core. So firstly loop to find out
> + * the dn of boot-cpu, and map them onto [0, nthreads)
> + */
> for_each_node_by_type(dn, "cpu") {
> const __be32 *intserv;
> __be32 cpu_be;
> @@ -488,6 +495,16 @@ void __init smp_setup_cpu_maps(void)
>
> nthreads = len / sizeof(int);
>
> + if (handling_bootdn) {
> + if (boot_cpuid < nthreads &&
> + be32_to_cpu(intserv[boot_cpuid]) == boot_cpuhwid) {
> + boot_dn = dn;
> + }
> + if (boot_dn == NULL)
> + continue;
> + } else if (dn == boot_dn)
> + continue;
> +
> for (j = 0; j < nthreads && cpu < nr_cpu_ids; j++) {
> bool avail;
>
> @@ -509,6 +526,10 @@ void __init smp_setup_cpu_maps(void)
> of_node_put(dn);
> break;
> }
> + if (handling_bootdn) {
> + handling_bootdn = false;
> + goto again;
> + }
> }
You don't need that "again" loop and "handling_bootdn" weird boolean.
Instead, start with cpu = 1 instead of cpu = 0, and rename it to
"next_cpu".
Then, before the thread loop, check if we are on the same core
as boot_cpuhwid:
if (same_core_as_boot_cpu(intserv)) {
cpu = 0;
} else if (next_cpu < nr_cpus_ids) {
cpu = next_cpu++;
} else {
of_node_put(dn);
break;
}
>
> /* If no SMT supported, nthreads is forced to 1 */
^ permalink raw reply
* Re: [PATCHv4 1/3] powerpc, cpu: partially unbind the mapping between cpu logical id and its seq in dt
From: kbuild test robot @ 2018-03-13 5:06 UTC (permalink / raw)
To: Pingfan Liu; +Cc: kbuild-all, linuxppc-dev, cascardo, gpiccoli, mahesh, kexec
In-Reply-To: <1520829790-14029-2-git-send-email-kernelfans@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2210 bytes --]
Hi Pingfan,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180309]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Pingfan-Liu/powerpc-cpu-partially-unbind-the-mapping-between-cpu-logical-id-and-its-seq-in-dt/20180313-034420
config: powerpc-pq2fads_defconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
>> arch/powerpc/kernel/prom.c:79:23: error: 'boot_cpu_count' defined but not used [-Werror=unused-variable]
static int __initdata boot_cpu_count;
^~~~~~~~~~~~~~
cc1: all warnings being treated as errors
vim +/boot_cpu_count +79 arch/powerpc/kernel/prom.c
9b6b563c Paul Mackerras 2005-10-06 71
9b6b563c Paul Mackerras 2005-10-06 72 #ifdef CONFIG_PPC64
28897731 Olof Johansson 2006-04-12 73 int __initdata iommu_is_off;
9b6b563c Paul Mackerras 2005-10-06 74 int __initdata iommu_force_on;
cf00a8d1 Paul Mackerras 2005-10-31 75 unsigned long tce_alloc_start, tce_alloc_end;
cd3db0c4 Benjamin Herrenschmidt 2010-07-06 76 u64 ppc64_rma_size;
9b6b563c Paul Mackerras 2005-10-06 77 #endif
03bf469a Benjamin Herrenschmidt 2011-05-11 78 static phys_addr_t first_memblock_size;
7ac87abb Matt Evans 2011-05-25 @79 static int __initdata boot_cpu_count;
9b6b563c Paul Mackerras 2005-10-06 80
:::::: The code at line 79 was first introduced by commit
:::::: 7ac87abb8166b99584149fcfb2efef5773a078e9 powerpc: Fix early boot accounting of CPUs
:::::: TO: Matt Evans <matt@ozlabs.org>
:::::: CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 14183 bytes --]
^ permalink raw reply
* Re: [PATCH v3] powerpc/kernel/sysfs: Export ldbar spr to sysfs
From: Anju T Sudhakar @ 2018-03-13 5:43 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, linux-kernel, maddy
In-Reply-To: <878tb5foqk.fsf@concordia.ellerman.id.au>
Hi,
On Tuesday 06 March 2018 04:35 PM, Michael Ellerman wrote:
> Anju T Sudhakar <anju@linux.vnet.ibm.com> writes:
>
>> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
>> index 4437c70..caefb64 100644
>> --- a/arch/powerpc/kernel/sysfs.c
>> +++ b/arch/powerpc/kernel/sysfs.c
>> @@ -757,6 +759,9 @@ static int register_cpu_online(unsigned int cpu)
>> device_create_file(s, &pmc_attrs[i]);
>>
>> #ifdef CONFIG_PPC64
>> + if (cpu_has_feature(CPU_FTR_ARCH_300))
>> + device_create_file(s, &dev_attr_ldbar);
> Is this register readable in supervisor state?
This is a nice catch, thanks. :)
The guest kernel can not access the register, it is only readable in the
hypervisor state.
I will resend the patch with a condition check so that this spr will not
get registered for guest kernel.
Regards,
Anju
>
> cheers
>
^ permalink raw reply
* [PATCH] cxl: Add new kernel traces
From: Christophe Lombard @ 2018-03-13 9:30 UTC (permalink / raw)
To: linuxppc-dev, fbarrat, vaibhav, andrew.donnellan
This patch adds new kernel traces in the current in-kernel 'library'
which can be called by other drivers to help interacting with an
IBM XSL on a POWER9 system.
If some kernel traces exist in the 'normal path' to handle a page or a
segment fault, some others are missing when a page fault is handle
through cxllib.
Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
---
drivers/misc/cxl/cxllib.c | 3 ++
drivers/misc/cxl/fault.c | 2 +
drivers/misc/cxl/irq.c | 2 +-
drivers/misc/cxl/trace.h | 115 ++++++++++++++++++++++++++--------------------
4 files changed, 72 insertions(+), 50 deletions(-)
diff --git a/drivers/misc/cxl/cxllib.c b/drivers/misc/cxl/cxllib.c
index 30ccba4..91cfb69 100644
--- a/drivers/misc/cxl/cxllib.c
+++ b/drivers/misc/cxl/cxllib.c
@@ -13,6 +13,7 @@
#include <misc/cxllib.h>
#include "cxl.h"
+#include "trace.h"
#define CXL_INVALID_DRA ~0ull
#define CXL_DUMMY_READ_SIZE 128
@@ -218,6 +219,8 @@ int cxllib_handle_fault(struct mm_struct *mm, u64 addr, u64 size, u64 flags)
if (mm == NULL)
return -EFAULT;
+ trace_cxl_lib_handle_fault(addr, size, flags);
+
down_read(&mm->mmap_sem);
vma = find_vma(mm, addr);
diff --git a/drivers/misc/cxl/fault.c b/drivers/misc/cxl/fault.c
index 70dbb6d..1c4fd74 100644
--- a/drivers/misc/cxl/fault.c
+++ b/drivers/misc/cxl/fault.c
@@ -138,6 +138,8 @@ int cxl_handle_mm_fault(struct mm_struct *mm, u64 dsisr, u64 dar)
int result;
unsigned long access, flags, inv_flags = 0;
+ trace_cxl_handle_mm_fault(dsisr, dar);
+
/*
* Add the fault handling cpu to task mm cpumask so that we
* can do a safe lockless page table walk when inserting the
diff --git a/drivers/misc/cxl/irq.c b/drivers/misc/cxl/irq.c
index ce08a9f..79b8b49 100644
--- a/drivers/misc/cxl/irq.c
+++ b/drivers/misc/cxl/irq.c
@@ -41,7 +41,7 @@ irqreturn_t cxl_irq_psl9(int irq, struct cxl_context *ctx, struct cxl_irq_info *
dsisr = irq_info->dsisr;
dar = irq_info->dar;
- trace_cxl_psl9_irq(ctx, irq, dsisr, dar);
+ trace_cxl_psl_irq(ctx, irq, dsisr, dar);
pr_devel("CXL interrupt %i for afu pe: %i DSISR: %#llx DAR: %#llx\n", irq, ctx->pe, dsisr, dar);
diff --git a/drivers/misc/cxl/trace.h b/drivers/misc/cxl/trace.h
index b8e300a..8eb2607 100644
--- a/drivers/misc/cxl/trace.h
+++ b/drivers/misc/cxl/trace.h
@@ -26,19 +26,20 @@
{ CXL_PSL9_DSISR_An_OC, "OC" }, \
{ CXL_PSL9_DSISR_An_S, "S" })
-#define DSISR_FLAGS \
- { CXL_PSL_DSISR_An_DS, "DS" }, \
- { CXL_PSL_DSISR_An_DM, "DM" }, \
- { CXL_PSL_DSISR_An_ST, "ST" }, \
- { CXL_PSL_DSISR_An_UR, "UR" }, \
- { CXL_PSL_DSISR_An_PE, "PE" }, \
- { CXL_PSL_DSISR_An_AE, "AE" }, \
- { CXL_PSL_DSISR_An_OC, "OC" }, \
- { CXL_PSL_DSISR_An_M, "M" }, \
- { CXL_PSL_DSISR_An_P, "P" }, \
- { CXL_PSL_DSISR_An_A, "A" }, \
- { CXL_PSL_DSISR_An_S, "S" }, \
- { CXL_PSL_DSISR_An_K, "K" }
+#define dsisr_psl8_flags(flags) \
+ __print_flags(flags, "|", \
+ { CXL_PSL_DSISR_An_DS, "DS" }, \
+ { CXL_PSL_DSISR_An_DM, "DM" }, \
+ { CXL_PSL_DSISR_An_ST, "ST" }, \
+ { CXL_PSL_DSISR_An_UR, "UR" }, \
+ { CXL_PSL_DSISR_An_PE, "PE" }, \
+ { CXL_PSL_DSISR_An_AE, "AE" }, \
+ { CXL_PSL_DSISR_An_OC, "OC" }, \
+ { CXL_PSL_DSISR_An_M, "M" }, \
+ { CXL_PSL_DSISR_An_P, "P" }, \
+ { CXL_PSL_DSISR_An_A, "A" }, \
+ { CXL_PSL_DSISR_An_S, "S" }, \
+ { CXL_PSL_DSISR_An_K, "K" })
#define TFC_FLAGS \
{ CXL_PSL_TFC_An_A, "A" }, \
@@ -163,7 +164,7 @@ TRACE_EVENT(cxl_afu_irq,
)
);
-TRACE_EVENT(cxl_psl9_irq,
+TRACE_EVENT(cxl_psl_irq,
TP_PROTO(struct cxl_context *ctx, int irq, u64 dsisr, u64 dar),
TP_ARGS(ctx, irq, dsisr, dar),
@@ -192,40 +193,8 @@ TRACE_EVENT(cxl_psl9_irq,
__entry->pe,
__entry->irq,
__entry->dsisr,
- dsisr_psl9_flags(__entry->dsisr),
- __entry->dar
- )
-);
-
-TRACE_EVENT(cxl_psl_irq,
- TP_PROTO(struct cxl_context *ctx, int irq, u64 dsisr, u64 dar),
-
- TP_ARGS(ctx, irq, dsisr, dar),
-
- TP_STRUCT__entry(
- __field(u8, card)
- __field(u8, afu)
- __field(u16, pe)
- __field(int, irq)
- __field(u64, dsisr)
- __field(u64, dar)
- ),
-
- TP_fast_assign(
- __entry->card = ctx->afu->adapter->adapter_num;
- __entry->afu = ctx->afu->slice;
- __entry->pe = ctx->pe;
- __entry->irq = irq;
- __entry->dsisr = dsisr;
- __entry->dar = dar;
- ),
-
- TP_printk("afu%i.%i pe=%i irq=%i dsisr=%s dar=0x%016llx",
- __entry->card,
- __entry->afu,
- __entry->pe,
- __entry->irq,
- __print_flags(__entry->dsisr, "|", DSISR_FLAGS),
+ cxl_is_power8() ? dsisr_psl8_flags(__entry->dsisr) :
+ dsisr_psl9_flags(__entry->dsisr),
__entry->dar
)
);
@@ -342,11 +311,59 @@ TRACE_EVENT(cxl_pte_miss,
__entry->card,
__entry->afu,
__entry->pe,
- __print_flags(__entry->dsisr, "|", DSISR_FLAGS),
+ cxl_is_power8() ? dsisr_psl8_flags(__entry->dsisr) :
+ dsisr_psl9_flags(__entry->dsisr),
+ __entry->dar
+ )
+);
+
+TRACE_EVENT(cxl_handle_mm_fault,
+ TP_PROTO(u64 dsisr, u64 dar),
+
+ TP_ARGS(dsisr, dar),
+
+ TP_STRUCT__entry(
+ __field(u64, dsisr)
+ __field(u64, dar)
+ ),
+
+ TP_fast_assign(
+ __entry->dsisr = dsisr;
+ __entry->dar = dar;
+ ),
+
+ TP_printk("dsisr=0x%016llx(%s), dar=0x%016llx",
+ __entry->dsisr,
+ cxl_is_power8() ? dsisr_psl8_flags(__entry->dsisr) :
+ dsisr_psl9_flags(__entry->dsisr),
__entry->dar
)
);
+TRACE_EVENT(cxl_lib_handle_fault,
+ TP_PROTO(u64 addr, u64 size, u64 flags),
+
+ TP_ARGS(addr, size, flags),
+
+ TP_STRUCT__entry(
+ __field(u64, addr)
+ __field(u64, size)
+ __field(u64, flags)
+ ),
+
+ TP_fast_assign(
+ __entry->addr = addr;
+ __entry->size = size;
+ __entry->flags = flags;
+ ),
+
+ TP_printk("addr=0x%016llx, size=0x%016llx, flags=0x%016llx",
+ __entry->addr,
+ __entry->size,
+ __entry->flags
+ )
+);
+
TRACE_EVENT(cxl_llcmd,
TP_PROTO(struct cxl_context *ctx, u64 cmd),
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 2/2] misc: ocxl: use put_device() instead of device_unregister()
From: Frederic Barrat @ 2018-03-13 9:38 UTC (permalink / raw)
To: Arvind Yadav, sudeep.dutt, ashutosh.dixit, arnd, gregkh,
andrew.donnellan
Cc: linux-kernel, linuxppc-dev
In-Reply-To: <8c4d28703e408b71284af68d07c117e7b8c8a75a.1520849284.git.arvind.yadav.cs@gmail.com>
Le 12/03/2018 à 12:36, Arvind Yadav a écrit :
> if device_register() returned an error! Always use put_device()
> to give up the reference initialized.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
OK, device_unregister() calls put_device() but also other actions that
we can skip in this case.
Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> drivers/misc/ocxl/pci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/misc/ocxl/pci.c b/drivers/misc/ocxl/pci.c
> index 0051d9e..21f4254 100644
> --- a/drivers/misc/ocxl/pci.c
> +++ b/drivers/misc/ocxl/pci.c
> @@ -519,7 +519,7 @@ static struct ocxl_fn *init_function(struct pci_dev *dev)
> rc = device_register(&fn->dev);
> if (rc) {
> deconfigure_function(fn);
> - device_unregister(&fn->dev);
> + put_device(&fn->dev);
> return ERR_PTR(rc);
> }
> return fn;
>
^ permalink raw reply
* Re: [PATCH] cxl: Perform NULL check for 'cxl_afu *' at various places in cxl
From: Frederic Barrat @ 2018-03-13 10:20 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <20180308100547.2994-1-vaibhav@linux.vnet.ibm.com>
Le 08/03/2018 à 11:05, Vaibhav Jain a écrit :
> It is possible for a CXL card to have a valid PSL but no valid
> AFUs. When this happens we have a valid instance of 'struct cxl'
> representing the adapter but with its member 'struct cxl_afu *cxl[]'
> as empty. Unfortunately at many placed within cxl code (especially
> during an EEH) the elements of this array are passed on to various
> other cxl functions. Which may result in kernel oops/panic when this
> 'struct cxl_afu *' is dereferenced.
>
> So this patch puts a NULL check at the beginning of various cxl
> functions that accept 'struct cxl_afu *' as a formal argument and are
> called from with a loop of the form:
>
> for (i = 0; i < adapter->slices; i++) {
> afu = adapter->afu[i];
> /* call some function with 'afu' */
> }
So we are calling functions with an invalid afu argument. We can verify
in the callees the value of the afu pointer, like you're doing here, but
why not tackle it at source and avoid calling the function in the first
place? It would have the nice side effect of reminding developers that
the AFU array can be empty.
We already have a few checks in place in the "for (i = 0; i <
adapter->slices; i++)" loops, but it was overlooked when eeh support was
added. I think we should fix that.
Fred
> Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
> ---
> drivers/misc/cxl/api.c | 2 +-
> drivers/misc/cxl/context.c | 3 +++
> drivers/misc/cxl/guest.c | 4 ++++
> drivers/misc/cxl/main.c | 3 +++
> drivers/misc/cxl/native.c | 4 ++++
> drivers/misc/cxl/pci.c | 13 ++++++++++++-
> 6 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
> index 753b1a698fc4..3466ef8b9e86 100644
> --- a/drivers/misc/cxl/api.c
> +++ b/drivers/misc/cxl/api.c
> @@ -128,7 +128,7 @@ struct cxl_context *cxl_dev_context_init(struct pci_dev *dev)
> int rc;
>
> afu = cxl_pci_to_afu(dev);
> - if (IS_ERR(afu))
> + if (IS_ERR_OR_NULL(afu))
> return ERR_CAST(afu);
>
> ctx = cxl_context_alloc();
> diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
> index 7ff315ad3692..3957e6e7d187 100644
> --- a/drivers/misc/cxl/context.c
> +++ b/drivers/misc/cxl/context.c
> @@ -303,6 +303,9 @@ void cxl_context_detach_all(struct cxl_afu *afu)
> struct cxl_context *ctx;
> int tmp;
>
> + if (afu == NULL)
> + return;
> +
> mutex_lock(&afu->contexts_lock);
> idr_for_each_entry(&afu->contexts_idr, ctx, tmp) {
> /*
> diff --git a/drivers/misc/cxl/guest.c b/drivers/misc/cxl/guest.c
> index f58b4b6c79f2..8165f6f26704 100644
> --- a/drivers/misc/cxl/guest.c
> +++ b/drivers/misc/cxl/guest.c
> @@ -760,6 +760,8 @@ static int activate_afu_directed(struct cxl_afu *afu)
>
> static int guest_afu_activate_mode(struct cxl_afu *afu, int mode)
> {
> + if (afu == NULL)
> + return -EINVAL;
> if (!mode)
> return 0;
> if (!(mode & afu->modes_supported))
> @@ -791,6 +793,8 @@ static int deactivate_afu_directed(struct cxl_afu *afu)
>
> static int guest_afu_deactivate_mode(struct cxl_afu *afu, int mode)
> {
> + if (afu == NULL)
> + return -EINVAL;
> if (!mode)
> return 0;
> if (!(mode & afu->modes_supported))
> diff --git a/drivers/misc/cxl/main.c b/drivers/misc/cxl/main.c
> index c1ba0d42cbc8..296a71ca6f2e 100644
> --- a/drivers/misc/cxl/main.c
> +++ b/drivers/misc/cxl/main.c
> @@ -271,6 +271,9 @@ struct cxl_afu *cxl_alloc_afu(struct cxl *adapter, int slice)
>
> int cxl_afu_select_best_mode(struct cxl_afu *afu)
> {
> + if (afu == NULL)
> + return -EINVAL;
> +
> if (afu->modes_supported & CXL_MODE_DIRECTED)
> return cxl_ops->afu_activate_mode(afu, CXL_MODE_DIRECTED);
>
> diff --git a/drivers/misc/cxl/native.c b/drivers/misc/cxl/native.c
> index 1b3d7c65ea3f..d46415b19b71 100644
> --- a/drivers/misc/cxl/native.c
> +++ b/drivers/misc/cxl/native.c
> @@ -971,6 +971,8 @@ static int deactivate_dedicated_process(struct cxl_afu *afu)
>
> static int native_afu_deactivate_mode(struct cxl_afu *afu, int mode)
> {
> + if (afu == NULL)
> + return -EINVAL;
> if (mode == CXL_MODE_DIRECTED)
> return deactivate_afu_directed(afu);
> if (mode == CXL_MODE_DEDICATED)
> @@ -980,6 +982,8 @@ static int native_afu_deactivate_mode(struct cxl_afu *afu, int mode)
>
> static int native_afu_activate_mode(struct cxl_afu *afu, int mode)
> {
> + if (!afu)
> + return -EINVAL;
> if (!mode)
> return 0;
> if (!(mode & afu->modes_supported))
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index 758842f65a1b..8c87d9fdcf5a 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -1295,6 +1295,9 @@ static int pci_configure_afu(struct cxl_afu *afu, struct cxl *adapter, struct pc
> {
> int rc;
>
> + if (afu == NULL)
> + return -EINVAL;
> +
> if ((rc = pci_map_slice_regs(afu, adapter, dev)))
> return rc;
>
> @@ -1341,6 +1344,10 @@ static int pci_configure_afu(struct cxl_afu *afu, struct cxl *adapter, struct pc
>
> static void pci_deconfigure_afu(struct cxl_afu *afu)
> {
> +
> + if (afu == NULL)
> + return;
> +
> /*
> * It's okay to deconfigure when AFU is already locked, otherwise wait
> * until there are no readers
> @@ -2078,6 +2085,10 @@ static pci_ers_result_t cxl_vphb_error_detected(struct cxl_afu *afu,
> pci_ers_result_t result = PCI_ERS_RESULT_NEED_RESET;
> pci_ers_result_t afu_result = PCI_ERS_RESULT_NEED_RESET;
>
> + /* Force a hotplug if an uninitiaized AFU is encountered */
> + if (afu == NULL)
> + return PCI_ERS_RESULT_DISCONNECT;
> +
> /* There should only be one entry, but go through the list
> * anyway
> */
> @@ -2330,7 +2341,7 @@ static void cxl_pci_resume(struct pci_dev *pdev)
> for (i = 0; i < adapter->slices; i++) {
> afu = adapter->afu[i];
>
> - if (afu->phb == NULL)
> + if (afu == NULL || afu->phb == NULL)
> continue;
>
> list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
>
^ permalink raw reply
* [PATCH] powerpc/numa: Correct kernel message severity
From: Vipin K Parashar @ 2018-03-13 10:11 UTC (permalink / raw)
To: linuxppc-dev; +Cc: nfont, Vipin K Parashar
printk in unmap_cpu_from_node() uses KERN_ERR message severity
for a WARNING message. Correct message severity to KERN_WARNING.
Signed-off-by: Vipin K Parashar <vipin@linux.vnet.ibm.com>
---
arch/powerpc/mm/numa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index edd8d0b..79c94cc 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -163,7 +163,7 @@ static void unmap_cpu_from_node(unsigned long cpu)
if (cpumask_test_cpu(cpu, node_to_cpumask_map[node])) {
cpumask_clear_cpu(cpu, node_to_cpumask_map[node]);
} else {
- printk(KERN_ERR "WARNING: cpu %lu not found in node %d\n",
+ printk(KERN_WARNING "WARNING: cpu %lu not found in node %d\n",
cpu, node);
}
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] powerpc/numa: Correct kernel message severity
From: Christophe LEROY @ 2018-03-13 10:28 UTC (permalink / raw)
To: Vipin K Parashar, linuxppc-dev; +Cc: nfont
In-Reply-To: <1520935913-23593-1-git-send-email-vipin@linux.vnet.ibm.com>
Le 13/03/2018 à 11:11, Vipin K Parashar a écrit :
> printk in unmap_cpu_from_node() uses KERN_ERR message severity
> for a WARNING message. Correct message severity to KERN_WARNING.
>
> Signed-off-by: Vipin K Parashar <vipin@linux.vnet.ibm.com>
> ---
> arch/powerpc/mm/numa.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index edd8d0b..79c94cc 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -163,7 +163,7 @@ static void unmap_cpu_from_node(unsigned long cpu)
> if (cpumask_test_cpu(cpu, node_to_cpumask_map[node])) {
> cpumask_clear_cpu(cpu, node_to_cpumask_map[node]);
> } else {
> - printk(KERN_ERR "WARNING: cpu %lu not found in node %d\n",
> + printk(KERN_WARNING "WARNING: cpu %lu not found in node %d\n",
> cpu, node);
Why not take the opportunity to use pr_warn() instead, hence to put back
the cpu and node vars on the same line.
Christophe
> }
> }
>
^ permalink raw reply
* Re: [PATCH] powerpc/powernv : Add support to enable sensor groups
From: Michael Ellerman @ 2018-03-13 11:02 UTC (permalink / raw)
To: Shilpasri G Bhat, Stewart Smith, linuxppc-dev
Cc: linux-kernel, ego, akshay.adiga, svaidy,
linux@roeck-us.net >> Guenter Roeck, Andrew Jeffery
In-Reply-To: <7b64ac19-5463-5f30-83bb-d5ff0611dc94@linux.vnet.ibm.com>
Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com> writes:
> On 12/04/2017 10:11 AM, Stewart Smith wrote:
>> Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com> writes:
>>> On 11/28/2017 05:07 PM, Michael Ellerman wrote:
>>>> Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com> writes:
>>>>
>>>>> Adds support to enable/disable a sensor group. This can be used to
>>>>> select the sensor groups that needs to be copied to main memory by
>>>>> OCC. Sensor groups like power, temperature, current, voltage,
>>>>> frequency, utilization can be enabled/disabled at runtime.
>>>>>
>>>>> Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
>>>>> ---
>>>>> The skiboot patch for the opal call is posted below:
>>>>> https://lists.ozlabs.org/pipermail/skiboot/2017-November/009713.html
>>>>
>>>> Can you remind me why we're doing this with a completely bespoke sysfs
>>>> API, rather than using some generic sensors API?
>>>
>>> Disabling/Enabling sensor groups is not supported in the current generic sensors
>>> API. And also we dont export all type of sensors in HWMON as not all of them are
>>> environment sensors (like performance).
>>
>> Are there barriers to adding such concepts to the generic sensors API?
>
> Yes.
>
> HWMON does not support attributes for a sensor-group. If we are to extend HWMON
> to add new per-sensor attributes to disable/enable, then we need to do either of
> the below:
>
> 1) If any one of the sensor is disabled then all the sensors belonging to that
> group will be disabled. OR
>
> 2) To disable a sensor group we need to disable all the sensors belonging to
> that group.
Either of those sound doable, the first is probably simpler, as long as
there's some way for userspace to understand that it is modifying the
state of the whole group.
> Another problem is hwmon categorizes the sensor-groups based on the type of
> sensors like power, temp. If OCC allows multiple groups of the same type then
> this approach adds some more complexity to the user to identify the sensors
> belonging to correct group.
I don't really understand this one, what do you mean by "If OCC allows"?
Also do we really expect users to be using this API? Or rather tools?
> And lastly HWMON does not allow platform specific non-standard sensor groups
> like CSM, job-scheduler, profiler.
Have we actually made specific proposals to the hwmon maintainers on
adding/changing any of the above? Have they rejected those proposals and
told us to go away?
cheers
^ permalink raw reply
* Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again
From: Michael Ellerman @ 2018-03-13 11:39 UTC (permalink / raw)
To: Mauricio Faria de Oliveira, Michal Suchánek; +Cc: linuxppc-dev
In-Reply-To: <f11fdded-9d5e-d06f-b75e-d0a802b2005d@linux.vnet.ibm.com>
Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> writes:
> Hi Michael and Michal,
>
> Got back to this; sorry for the delay.
>
> On 03/06/2018 09:55 AM, Michal Such=C3=A1nek wrote:
>> Michael Ellerman<mpe@ellerman.id.au> wrote:
>
>>> I*think* the patch below is all we need, as well as some tweaking of
>>> patch 2, are you able to test and repost?
>
>> Enabling the fallback flush always looks a bit dodgy but
>> do_rfi_flush_fixups will overwrite the jump so long any other fixup is
>> enabled.
>
> I agree; the 'Using fallback displacement flush' message is misleading
> (is the system slower/fallback or not? =C3=94_o)
That message is actually just wrong.
It still prints that even if enable=3Dfalse.
So we should change all those messages, perhaps:
pr_info("rfi-flush: fallback displacement flush available\n");
pr_info("rfi-flush: ori type flush available\n");
pr_info("rfi-flush: mttrig type flush available\n");
> So I wrote something with a new function parameter to force the init of
> the fallback flush area (true in pseries, false in powernv). Not that
> contained, but it seemed to convey the intent here in a clear way.
>
> That's v2, just sent.
OK thanks. I don't really like it :D - sorry!
It's a lot of plumbing of that bool just to avoid the message, whereas I
think we could just change the message like above.
cheers
^ permalink raw reply
* OK to merge via powerpc? (was Re: [PATCH 05/14] mm: make memblock_alloc_base_nid non-static)
From: Michael Ellerman @ 2018-03-13 12:06 UTC (permalink / raw)
To: akpm, mhocko, catalin.marinas, pasha.tatashin, takahiro.akashi,
gi-oh.kim, npiggin, baiyaowei, bob.picco, ard.biesheuvel,
linux-mm, linux-kernel
Cc: Nicholas Piggin, linuxppc-dev
In-Reply-To: <20180213150824.27689-6-npiggin@gmail.com>
Anyone object to us merging the following patch via the powerpc tree?
Full series is here if anyone's interested:
http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=28377&state=*
cheers
Nicholas Piggin <npiggin@gmail.com> writes:
> This will be used by powerpc to allocate per-cpu stacks and other
> data structures node-local where possible.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> include/linux/memblock.h | 5 ++++-
> mm/memblock.c | 2 +-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index 8be5077efb5f..8cab51398705 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -316,9 +316,12 @@ static inline bool memblock_bottom_up(void)
> #define MEMBLOCK_ALLOC_ANYWHERE (~(phys_addr_t)0)
> #define MEMBLOCK_ALLOC_ACCESSIBLE 0
>
> -phys_addr_t __init memblock_alloc_range(phys_addr_t size, phys_addr_t align,
> +phys_addr_t memblock_alloc_range(phys_addr_t size, phys_addr_t align,
> phys_addr_t start, phys_addr_t end,
> ulong flags);
> +phys_addr_t memblock_alloc_base_nid(phys_addr_t size,
> + phys_addr_t align, phys_addr_t max_addr,
> + int nid, ulong flags);
> phys_addr_t memblock_alloc_base(phys_addr_t size, phys_addr_t align,
> phys_addr_t max_addr);
> phys_addr_t __memblock_alloc_base(phys_addr_t size, phys_addr_t align,
> diff --git a/mm/memblock.c b/mm/memblock.c
> index 5a9ca2a1751b..cea2af494da0 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -1190,7 +1190,7 @@ phys_addr_t __init memblock_alloc_range(phys_addr_t size, phys_addr_t align,
> flags);
> }
>
> -static phys_addr_t __init memblock_alloc_base_nid(phys_addr_t size,
> +phys_addr_t __init memblock_alloc_base_nid(phys_addr_t size,
> phys_addr_t align, phys_addr_t max_addr,
> int nid, ulong flags)
> {
> --
> 2.16.1
^ permalink raw reply
* Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again
From: Mauricio Faria de Oliveira @ 2018-03-13 12:14 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Michal Suchánek, linuxppc-dev
In-Reply-To: <876060gq5s.fsf@concordia.ellerman.id.au>
Hi Michael,
On 03/13/2018 08:39 AM, Michael Ellerman wrote:
>> I agree; the 'Using fallback displacement flush' message is misleading
>> (is the system slower/fallback or not? Ô_o)
> That message is actually just wrong.
>
> It still prints that even if enable=false.
>
> So we should change all those messages, perhaps:
>
> pr_info("rfi-flush: fallback displacement flush available\n");
> pr_info("rfi-flush: ori type flush available\n");
> pr_info("rfi-flush: mttrig type flush available\n");
Indeed.
>> So I wrote something with a new function parameter to force the init of
>> the fallback flush area (true in pseries, false in powernv). Not that
>> contained, but it seemed to convey the intent here in a clear way.
>>
>> That's v2, just sent.
> OK thanks. I don't really like it :D - sorry!
No worries :) fair enough. Well, I didn't like it much, either, TBH.
> It's a lot of plumbing of that bool just to avoid the message, whereas I
> think we could just change the message like above.
Yup.
And what you think about a more descriptive confirmation of what flush
instructions/methods are _actually_ being used?
Currently and w/ your suggestion aobve, all that is known is what is
_available_, not what has gone in (or out, in the disable case) the
nop slots.
cheers,
mauricio
^ permalink raw reply
* Re: [PATCH 07/14] powerpc/64: move default SPR recording
From: Michael Ellerman @ 2018-03-13 12:25 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20180213150824.27689-8-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> Move this into the early setup code, and don't iterate over CPU masks.
> We don't want to call into sysfs so early from setup, and a future patch
> won't initialize CPU masks by the time this is called.
> ---
> arch/powerpc/kernel/paca.c | 3 +++
> arch/powerpc/kernel/setup.h | 9 +++------
> arch/powerpc/kernel/setup_64.c | 8 ++++++++
> arch/powerpc/kernel/sysfs.c | 18 +++++++-----------
> 4 files changed, 21 insertions(+), 17 deletions(-)
This patch, and 8, 9, 10, aren't signed-off by you.
I'll assume you just forgot and add it.
cheers
^ permalink raw reply
* Re: [PATCH 03/14] powerpc/64s: allocate lppacas individually
From: Michael Ellerman @ 2018-03-13 12:41 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20180213150824.27689-4-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> diff --git a/arch/powerpc/platforms/pseries/kexec.c b/arch/powerpc/platforms/pseries/kexec.c
> index eeb13429d685..3fe126796975 100644
> --- a/arch/powerpc/platforms/pseries/kexec.c
> +++ b/arch/powerpc/platforms/pseries/kexec.c
> @@ -23,7 +23,12 @@
>
> void pseries_kexec_cpu_down(int crash_shutdown, int secondary)
> {
> - /* Don't risk a hypervisor call if we're crashing */
> + /*
> + * Don't risk a hypervisor call if we're crashing
> + * XXX: Why? The hypervisor is not crashing. It might be better
> + * to at least attempt unregister to avoid the hypervisor stepping
> + * on our memory.
> + */
Because every extra line of code we run in the crashed kernel is another
opportunity to screw up and not make it into the kdump kernel.
For example the hcalls we do to unregister the VPA might trigger hcall
tracing which runs a bunch of code and might trip up on something. We
could modify those hcalls to not be traced, but then we can't trace them
in normal operation.
And the hypervisor might continue to write to the VPA, but that's OK
because it's the VPA of the crashing kernel, the kdump kernel runs in a
separate reserved memory region.
Possibly we could fix the hcall tracing issues etc, but this code has
not given us any problems for quite a while (~13 years) - ie. there
seems to be no issue with re-registering the VPAs etc. in the kdump
kernel.
cheers
^ permalink raw reply
* Re: [PATCH 03/14] powerpc/64s: allocate lppacas individually
From: Nicholas Piggin @ 2018-03-13 12:54 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <87woygf8qd.fsf@concordia.ellerman.id.au>
On Tue, 13 Mar 2018 23:41:46 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
> > diff --git a/arch/powerpc/platforms/pseries/kexec.c b/arch/powerpc/platforms/pseries/kexec.c
> > index eeb13429d685..3fe126796975 100644
> > --- a/arch/powerpc/platforms/pseries/kexec.c
> > +++ b/arch/powerpc/platforms/pseries/kexec.c
> > @@ -23,7 +23,12 @@
> >
> > void pseries_kexec_cpu_down(int crash_shutdown, int secondary)
> > {
> > - /* Don't risk a hypervisor call if we're crashing */
> > + /*
> > + * Don't risk a hypervisor call if we're crashing
> > + * XXX: Why? The hypervisor is not crashing. It might be better
> > + * to at least attempt unregister to avoid the hypervisor stepping
> > + * on our memory.
> > + */
>
> Because every extra line of code we run in the crashed kernel is another
> opportunity to screw up and not make it into the kdump kernel.
>
> For example the hcalls we do to unregister the VPA might trigger hcall
> tracing which runs a bunch of code and might trip up on something. We
> could modify those hcalls to not be traced, but then we can't trace them
> in normal operation.
We really make no other hcalls in a crash? I didn't think of that.
>
> And the hypervisor might continue to write to the VPA, but that's OK
> because it's the VPA of the crashing kernel, the kdump kernel runs in a
> separate reserved memory region.
Well that takes care of that concern.
> Possibly we could fix the hcall tracing issues etc, but this code has
> not given us any problems for quite a while (~13 years) - ie. there
> seems to be no issue with re-registering the VPAs etc. in the kdump
> kernel.
No I think it's okay then, if you could drop that hunk...
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 07/14] powerpc/64: move default SPR recording
From: Nicholas Piggin @ 2018-03-13 12:55 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <87zi3cf9i6.fsf@concordia.ellerman.id.au>
On Tue, 13 Mar 2018 23:25:05 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
> > Move this into the early setup code, and don't iterate over CPU masks.
> > We don't want to call into sysfs so early from setup, and a future patch
> > won't initialize CPU masks by the time this is called.
> > ---
> > arch/powerpc/kernel/paca.c | 3 +++
> > arch/powerpc/kernel/setup.h | 9 +++------
> > arch/powerpc/kernel/setup_64.c | 8 ++++++++
> > arch/powerpc/kernel/sysfs.c | 18 +++++++-----------
> > 4 files changed, 21 insertions(+), 17 deletions(-)
>
> This patch, and 8, 9, 10, aren't signed-off by you.
>
> I'll assume you just forgot and add it.
Yes I did.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 4.14 1/4] powerpc/mm/slice: Remove intermediate bitmap copy
From: Michael Ellerman @ 2018-03-13 13:24 UTC (permalink / raw)
To: Greg Kroah-Hartman, christophe leroy; +Cc: linuxppc-dev, linux-kernel, stable
In-Reply-To: <20180310194742.GA12279@kroah.com>
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> On Sat, Mar 10, 2018 at 05:14:22PM +0100, christophe leroy wrote:
>> Le 10/03/2018 =C3=A0 15:52, Greg Kroah-Hartman a =C3=A9crit=C2=A0:
>> > On Sat, Mar 10, 2018 at 08:27:54AM +0100, christophe leroy wrote:
>> > > Le 10/03/2018 =C3=A0 01:10, Greg Kroah-Hartman a =C3=A9crit=C2=A0:
>> > > > On Fri, Mar 09, 2018 at 04:48:59PM +0100, Christophe Leroy wrote:
>> > > > > Upstream 326691ad4f179e6edc7eb1271e618dd673e4736d
>> > > >=20
>> > > > There is no such git commit id in Linus's tree :(
>> > > >=20
>> > > > Please fix up and resend the series.
>> > >=20
>> > > I checked again, it is there
>> > >=20
>> > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/=
commit/arch/powerpc/mm/slice.c?h=3Dnext-20180309&id=3D326691ad4f179e6edc7eb=
1271e618dd673e4736d
>> >=20
>> > That is linux-next, which has everything and the kitchen sink. It is
>> > not Linus's tree. Please wait for these things to be merged into
>> > Linus's tree before asking for the to be merged into the stable tree.
>> > That's a requirement.
>>=20
>> Oops, sorry, I thought everything on kernel.org was official.
>
> That would be a whole lot of "official" :)
>
> Please read:
> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.ht=
ml
> for what the rules are here, if you haven't already.
>
>> Once it is in, do I resend the patches or do I just ping you ?
>
> You would need to resend the patches (if they need backporting
> manually), or just send a list of the git commit ids that are needed to
> be applied (usually easier.)
>
> Also, why were these patches not tagged with the stable tag to start
> with? That way they would be automatically included in the stable tree
> when they hit Linus's tree.
Because they're fairly large and invasive and not well tested on other
platforms, so the maintainer is not comfortable with them going straight
to stable :)
Once they've had some testing in Linus' tree at least, then we'll ask
for a backport if there's no issues.
Sorry for the confusion.
cheers
^ permalink raw reply
* Re: [PATCH v4 02/10] include: Move compat_timespec/ timeval to compat_time.h
From: kbuild test robot @ 2018-03-13 15:22 UTC (permalink / raw)
To: Deepa Dinamani
Cc: kbuild-all, arnd, tglx, john.stultz, mark.rutland, linux-mips,
peterz, benh, heiko.carstens, paulus, hpa, sparclinux, devel,
linux-s390, y2038, mpe, deller, x86, sebott, jejb, will.deacon,
borntraeger, mingo, oprofile-list, catalin.marinas, rric,
cmetcalf, oberpar, acme, jwi, rostedt, ubraun, gerald.schaefer,
linux-parisc, gregkh, cohuck, linux-kernel, ralf, hoeppner, sth,
schwidefsky, linuxppc-dev, davem
In-Reply-To: <20180312175307.11032-3-deepa.kernel@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6059 bytes --]
Hi Deepa,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on ]
url: https://github.com/0day-ci/linux/commits/Deepa-Dinamani/posix_clocks-Prepare-syscalls-for-64-bit-time_t-conversion/20180313-203305
base:
config: arm64-allnoconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
All errors (new ones prefixed by >>):
arch/arm64/kernel/process.c: In function 'copy_thread':
>> arch/arm64/kernel/process.c:342:8: error: implicit declaration of function 'is_compat_thread'; did you mean 'is_compat_task'? [-Werror=implicit-function-declaration]
if (is_compat_thread(task_thread_info(p)))
^~~~~~~~~~~~~~~~
is_compat_task
cc1: some warnings being treated as errors
vim +342 arch/arm64/kernel/process.c
b3901d54d Catalin Marinas 2012-03-05 307
b3901d54d Catalin Marinas 2012-03-05 308 int copy_thread(unsigned long clone_flags, unsigned long stack_start,
afa86fc42 Al Viro 2012-10-22 309 unsigned long stk_sz, struct task_struct *p)
b3901d54d Catalin Marinas 2012-03-05 310 {
b3901d54d Catalin Marinas 2012-03-05 311 struct pt_regs *childregs = task_pt_regs(p);
b3901d54d Catalin Marinas 2012-03-05 312
c34501d21 Catalin Marinas 2012-10-05 313 memset(&p->thread.cpu_context, 0, sizeof(struct cpu_context));
c34501d21 Catalin Marinas 2012-10-05 314
bc0ee4760 Dave Martin 2017-10-31 315 /*
bc0ee4760 Dave Martin 2017-10-31 316 * Unalias p->thread.sve_state (if any) from the parent task
bc0ee4760 Dave Martin 2017-10-31 317 * and disable discard SVE state for p:
bc0ee4760 Dave Martin 2017-10-31 318 */
bc0ee4760 Dave Martin 2017-10-31 319 clear_tsk_thread_flag(p, TIF_SVE);
bc0ee4760 Dave Martin 2017-10-31 320 p->thread.sve_state = NULL;
bc0ee4760 Dave Martin 2017-10-31 321
071b6d4a5 Dave Martin 2017-12-05 322 /*
071b6d4a5 Dave Martin 2017-12-05 323 * In case p was allocated the same task_struct pointer as some
071b6d4a5 Dave Martin 2017-12-05 324 * other recently-exited task, make sure p is disassociated from
071b6d4a5 Dave Martin 2017-12-05 325 * any cpu that may have run that now-exited task recently.
071b6d4a5 Dave Martin 2017-12-05 326 * Otherwise we could erroneously skip reloading the FPSIMD
071b6d4a5 Dave Martin 2017-12-05 327 * registers for p.
071b6d4a5 Dave Martin 2017-12-05 328 */
071b6d4a5 Dave Martin 2017-12-05 329 fpsimd_flush_task_state(p);
071b6d4a5 Dave Martin 2017-12-05 330
9ac080021 Al Viro 2012-10-21 331 if (likely(!(p->flags & PF_KTHREAD))) {
9ac080021 Al Viro 2012-10-21 332 *childregs = *current_pt_regs();
b3901d54d Catalin Marinas 2012-03-05 333 childregs->regs[0] = 0;
d00a3810c Will Deacon 2015-05-27 334
b3901d54d Catalin Marinas 2012-03-05 335 /*
b3901d54d Catalin Marinas 2012-03-05 336 * Read the current TLS pointer from tpidr_el0 as it may be
b3901d54d Catalin Marinas 2012-03-05 337 * out-of-sync with the saved value.
b3901d54d Catalin Marinas 2012-03-05 338 */
adf758999 Mark Rutland 2016-09-08 339 *task_user_tls(p) = read_sysreg(tpidr_el0);
d00a3810c Will Deacon 2015-05-27 340
e0fd18ce1 Al Viro 2012-10-18 341 if (stack_start) {
d00a3810c Will Deacon 2015-05-27 @342 if (is_compat_thread(task_thread_info(p)))
d00a3810c Will Deacon 2015-05-27 343 childregs->compat_sp = stack_start;
d00a3810c Will Deacon 2015-05-27 344 else
b3901d54d Catalin Marinas 2012-03-05 345 childregs->sp = stack_start;
b3901d54d Catalin Marinas 2012-03-05 346 }
d00a3810c Will Deacon 2015-05-27 347
c34501d21 Catalin Marinas 2012-10-05 348 /*
c34501d21 Catalin Marinas 2012-10-05 349 * If a TLS pointer was passed to clone (4th argument), use it
c34501d21 Catalin Marinas 2012-10-05 350 * for the new thread.
c34501d21 Catalin Marinas 2012-10-05 351 */
b3901d54d Catalin Marinas 2012-03-05 352 if (clone_flags & CLONE_SETTLS)
d00a3810c Will Deacon 2015-05-27 353 p->thread.tp_value = childregs->regs[3];
c34501d21 Catalin Marinas 2012-10-05 354 } else {
c34501d21 Catalin Marinas 2012-10-05 355 memset(childregs, 0, sizeof(struct pt_regs));
c34501d21 Catalin Marinas 2012-10-05 356 childregs->pstate = PSR_MODE_EL1h;
57f4959ba James Morse 2016-02-05 357 if (IS_ENABLED(CONFIG_ARM64_UAO) &&
a4023f682 Suzuki K Poulose 2016-11-08 358 cpus_have_const_cap(ARM64_HAS_UAO))
57f4959ba James Morse 2016-02-05 359 childregs->pstate |= PSR_UAO_BIT;
c34501d21 Catalin Marinas 2012-10-05 360 p->thread.cpu_context.x19 = stack_start;
c34501d21 Catalin Marinas 2012-10-05 361 p->thread.cpu_context.x20 = stk_sz;
c34501d21 Catalin Marinas 2012-10-05 362 }
c34501d21 Catalin Marinas 2012-10-05 363 p->thread.cpu_context.pc = (unsigned long)ret_from_fork;
c34501d21 Catalin Marinas 2012-10-05 364 p->thread.cpu_context.sp = (unsigned long)childregs;
b3901d54d Catalin Marinas 2012-03-05 365
b3901d54d Catalin Marinas 2012-03-05 366 ptrace_hw_copy_thread(p);
b3901d54d Catalin Marinas 2012-03-05 367
b3901d54d Catalin Marinas 2012-03-05 368 return 0;
b3901d54d Catalin Marinas 2012-03-05 369 }
b3901d54d Catalin Marinas 2012-03-05 370
:::::: The code at line 342 was first introduced by commit
:::::: d00a3810c16207d2541b7796a73cca5a24ea3742 arm64: context-switch user tls register tpidr_el0 for compat tasks
:::::: TO: Will Deacon <will.deacon@arm.com>
:::::: CC: Catalin Marinas <catalin.marinas@arm.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6538 bytes --]
^ permalink raw reply
* Re: [PATCH v4 02/10] include: Move compat_timespec/ timeval to compat_time.h
From: kbuild test robot @ 2018-03-13 15:30 UTC (permalink / raw)
To: Deepa Dinamani
Cc: kbuild-all, arnd, tglx, john.stultz, mark.rutland, linux-mips,
peterz, benh, heiko.carstens, paulus, hpa, sparclinux, devel,
linux-s390, y2038, mpe, deller, x86, sebott, jejb, will.deacon,
borntraeger, mingo, oprofile-list, catalin.marinas, rric,
cmetcalf, oberpar, acme, jwi, rostedt, ubraun, gerald.schaefer,
linux-parisc, gregkh, cohuck, linux-kernel, ralf, hoeppner, sth,
schwidefsky, linuxppc-dev, davem
In-Reply-To: <20180312175307.11032-3-deepa.kernel@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3259 bytes --]
Hi Deepa,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on ]
url: https://github.com/0day-ci/linux/commits/Deepa-Dinamani/posix_clocks-Prepare-syscalls-for-64-bit-time_t-conversion/20180313-203305
base:
config: powerpc-iss476-smp_defconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
arch/powerpc/oprofile/backtrace.c: In function 'user_getsp32':
>> arch/powerpc/oprofile/backtrace.c:31:19: error: implicit declaration of function 'compat_ptr'; did you mean 'complete'? [-Werror=implicit-function-declaration]
void __user *p = compat_ptr(sp);
^~~~~~~~~~
complete
>> arch/powerpc/oprofile/backtrace.c:31:19: error: initialization makes pointer from integer without a cast [-Werror=int-conversion]
cc1: all warnings being treated as errors
vim +31 arch/powerpc/oprofile/backtrace.c
6c6bd754 Brian Rogan 2006-03-27 27
6c6bd754 Brian Rogan 2006-03-27 28 static unsigned int user_getsp32(unsigned int sp, int is_first)
6c6bd754 Brian Rogan 2006-03-27 29 {
6c6bd754 Brian Rogan 2006-03-27 30 unsigned int stack_frame[2];
62034f03 Al Viro 2006-09-23 @31 void __user *p = compat_ptr(sp);
6c6bd754 Brian Rogan 2006-03-27 32
62034f03 Al Viro 2006-09-23 33 if (!access_ok(VERIFY_READ, p, sizeof(stack_frame)))
6c6bd754 Brian Rogan 2006-03-27 34 return 0;
6c6bd754 Brian Rogan 2006-03-27 35
6c6bd754 Brian Rogan 2006-03-27 36 /*
6c6bd754 Brian Rogan 2006-03-27 37 * The most likely reason for this is that we returned -EFAULT,
6c6bd754 Brian Rogan 2006-03-27 38 * which means that we've done all that we can do from
6c6bd754 Brian Rogan 2006-03-27 39 * interrupt context.
6c6bd754 Brian Rogan 2006-03-27 40 */
62034f03 Al Viro 2006-09-23 41 if (__copy_from_user_inatomic(stack_frame, p, sizeof(stack_frame)))
6c6bd754 Brian Rogan 2006-03-27 42 return 0;
6c6bd754 Brian Rogan 2006-03-27 43
6c6bd754 Brian Rogan 2006-03-27 44 if (!is_first)
6c6bd754 Brian Rogan 2006-03-27 45 oprofile_add_trace(STACK_LR32(stack_frame));
6c6bd754 Brian Rogan 2006-03-27 46
6c6bd754 Brian Rogan 2006-03-27 47 /*
6c6bd754 Brian Rogan 2006-03-27 48 * We do not enforce increasing stack addresses here because
6c6bd754 Brian Rogan 2006-03-27 49 * we may transition to a different stack, eg a signal handler.
6c6bd754 Brian Rogan 2006-03-27 50 */
6c6bd754 Brian Rogan 2006-03-27 51 return STACK_SP(stack_frame);
6c6bd754 Brian Rogan 2006-03-27 52 }
6c6bd754 Brian Rogan 2006-03-27 53
:::::: The code at line 31 was first introduced by commit
:::::: 62034f03380a64c0144b6721f4a2aa55d65346c1 [POWERPC] powerpc oprofile __user annotations
:::::: TO: Al Viro <viro@ftp.linux.org.uk>
:::::: CC: Paul Mackerras <paulus@samba.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 9648 bytes --]
^ permalink raw reply
* Re: [PATCH 07/14] powerpc/64: move default SPR recording
From: Nicholas Piggin @ 2018-03-13 15:47 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <87zi3cf9i6.fsf@concordia.ellerman.id.au>
On Tue, 13 Mar 2018 23:25:05 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
> > Move this into the early setup code, and don't iterate over CPU masks.
> > We don't want to call into sysfs so early from setup, and a future patch
> > won't initialize CPU masks by the time this is called.
> > ---
> > arch/powerpc/kernel/paca.c | 3 +++
> > arch/powerpc/kernel/setup.h | 9 +++------
> > arch/powerpc/kernel/setup_64.c | 8 ++++++++
> > arch/powerpc/kernel/sysfs.c | 18 +++++++-----------
> > 4 files changed, 21 insertions(+), 17 deletions(-)
>
> This patch, and 8, 9, 10, aren't signed-off by you.
>
> I'll assume you just forgot and add it.
Can I give you an incremental fix for this patch? dscr_default
is zero at this point, so set it from spr_default_dscr before
setting pacas.
Remove the assignment from initialise_paca -- this happens too
early and gets overwritten anyway.
---
arch/powerpc/kernel/paca.c | 3 ---
arch/powerpc/kernel/sysfs.c | 2 ++
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index 2f3187501a36..7736188c764f 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -133,9 +133,6 @@ void __init initialise_paca(struct paca_struct *new_paca, int cpu)
new_paca->kexec_state = KEXEC_STATE_NONE;
new_paca->__current = &init_task;
new_paca->data_offset = 0xfeeeeeeeeeeeeeeeULL;
-#ifdef CONFIG_PPC64
- new_paca->dscr_default = spr_default_dscr;
-#endif
#ifdef CONFIG_PPC_BOOK3S_64
new_paca->slb_shadow_ptr = NULL;
#endif
diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index aaab582a640c..755dc98a57ae 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -20,6 +20,7 @@
#include <asm/firmware.h>
#include "cacheinfo.h"
+#include "setup.h"
#ifdef CONFIG_PPC64
#include <asm/paca.h>
@@ -592,6 +593,7 @@ static void sysfs_create_dscr_default(void)
int err = 0;
int cpu;
+ dscr_default = spr_default_dscr;
for_each_possible_cpu(cpu)
paca_ptrs[cpu]->dscr_default = dscr_default;
--
2.16.1
^ permalink raw reply related
* Re: [PATCH] powerpc/powernv : Add support to enable sensor groups
From: Guenter Roeck @ 2018-03-13 16:13 UTC (permalink / raw)
To: Michael Ellerman
Cc: Shilpasri G Bhat, Stewart Smith, linuxppc-dev, linux-kernel, ego,
akshay.adiga, svaidy, Andrew Jeffery
In-Reply-To: <87a7vcgrwu.fsf@concordia.ellerman.id.au>
On Tue, Mar 13, 2018 at 10:02:09PM +1100, Michael Ellerman wrote:
> Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com> writes:
> > On 12/04/2017 10:11 AM, Stewart Smith wrote:
> >> Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com> writes:
> >>> On 11/28/2017 05:07 PM, Michael Ellerman wrote:
> >>>> Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com> writes:
> >>>>
> >>>>> Adds support to enable/disable a sensor group. This can be used to
> >>>>> select the sensor groups that needs to be copied to main memory by
> >>>>> OCC. Sensor groups like power, temperature, current, voltage,
> >>>>> frequency, utilization can be enabled/disabled at runtime.
> >>>>>
> >>>>> Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
> >>>>> ---
> >>>>> The skiboot patch for the opal call is posted below:
> >>>>> https://lists.ozlabs.org/pipermail/skiboot/2017-November/009713.html
> >>>>
> >>>> Can you remind me why we're doing this with a completely bespoke sysfs
> >>>> API, rather than using some generic sensors API?
> >>>
> >>> Disabling/Enabling sensor groups is not supported in the current generic sensors
> >>> API. And also we dont export all type of sensors in HWMON as not all of them are
> >>> environment sensors (like performance).
> >>
> >> Are there barriers to adding such concepts to the generic sensors API?
> >
> > Yes.
> >
> > HWMON does not support attributes for a sensor-group. If we are to extend HWMON
> > to add new per-sensor attributes to disable/enable, then we need to do either of
> > the below:
> >
> > 1) If any one of the sensor is disabled then all the sensors belonging to that
> > group will be disabled. OR
> >
> > 2) To disable a sensor group we need to disable all the sensors belonging to
> > that group.
>
> Either of those sound doable, the first is probably simpler, as long as
> there's some way for userspace to understand that it is modifying the
> state of the whole group.
>
> > Another problem is hwmon categorizes the sensor-groups based on the type of
> > sensors like power, temp. If OCC allows multiple groups of the same type then
> > this approach adds some more complexity to the user to identify the sensors
> > belonging to correct group.
>
> I don't really understand this one, what do you mean by "If OCC allows"?
>
> Also do we really expect users to be using this API? Or rather tools?
>
> > And lastly HWMON does not allow platform specific non-standard sensor groups
> > like CSM, job-scheduler, profiler.
>
> Have we actually made specific proposals to the hwmon maintainers on
> adding/changing any of the above? Have they rejected those proposals and
> told us to go away?
>
Those don't really sound like sensor groups at all. What does "job-scheduler"
or "profiler" have to do with hardware monitoring ? We do allow additional
attributes if it makes sense, but those should be hardware monitoring related.
We also allow registration with other subsystems (such as gpio) if a hardware
monitoring device also has gpio pins and it seems to cumbersome to request
that an mfd driver is written. However, I am not convinced that completely
unrelated attributes should be handled through the hwmon subsystem; if this
is deemed necessary, it rather seems that hardware monitoring is one of many
functionalities of a given chip, and such functionality should be handled
elsewhere.
For the rest (enabling or disabling sensors dynamically), I am not specifically
opposed to improving the hwmon core to add such support, but someone would have
to make a specific proposal. One key problem is that the hwmon API assumes
'static' sensor allocation. The behavior of sensors appearng or disappearing
at runtime (even though it happens) is not well defined. Any proposal along
that line will need to ensure that userspace behavior is well documented.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again
From: Michal Suchánek @ 2018-03-13 17:59 UTC (permalink / raw)
To: Mauricio Faria de Oliveira; +Cc: Michael Ellerman, linuxppc-dev
In-Reply-To: <c7cadceb-1957-046e-8934-592baccca5b7@linux.vnet.ibm.com>
On Tue, 13 Mar 2018 09:14:39 -0300
Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> wrote:
> Hi Michael,
>=20
> On 03/13/2018 08:39 AM, Michael Ellerman wrote:
> >> I agree; the 'Using fallback displacement flush' message is
> >> misleading (is the system slower/fallback or not? =C3=94_o) =20
>=20
> > That message is actually just wrong.
> >=20
> > It still prints that even if enable=3Dfalse.
> >=20
> > So we should change all those messages, perhaps:
> >=20
> > pr_info("rfi-flush: fallback displacement flush
> > available\n"); pr_info("rfi-flush: ori type flush available\n");
> > pr_info("rfi-flush: mttrig type flush available\n"); =20
>=20
> Indeed.
Maybe it would make more sense to move the messages to the function
that actually patches in the instructions?
Thanks
Michal
^ permalink raw reply
* [PATCH v9 00/24] Speculative page faults
From: Laurent Dufour @ 2018-03-13 17:59 UTC (permalink / raw)
To: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
hpa, Will Deacon, Sergey Senozhatsky, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work,
Daniel Jordan
Cc: linux-kernel, linux-mm, haren, khandual, npiggin, bsingharora,
Tim Chen, linuxppc-dev, x86
This is a port on kernel 4.16 of the work done by Peter Zijlstra to
handle page fault without holding the mm semaphore [1].
The idea is to try to handle user space page faults without holding the
mmap_sem. This should allow better concurrency for massively threaded
process since the page fault handler will not wait for other threads memory
layout change to be done, assuming that this change is done in another part
of the process's memory space. This type page fault is named speculative
page fault. If the speculative page fault fails because of a concurrency is
detected or because underlying PMD or PTE tables are not yet allocating, it
is failing its processing and a classic page fault is then tried.
The speculative page fault (SPF) has to look for the VMA matching the fault
address without holding the mmap_sem, this is done by introducing a rwlock
which protects the access to the mm_rb tree. Previously this was done using
SRCU but it was introducing a lot of scheduling to process the VMA's
freeing
operation which was hitting the performance by 20% as reported by Kemi Wang
[2].Using a rwlock to protect access to the mm_rb tree is limiting the
locking contention to these operations which are expected to be in a O(log
n)
order. In addition to ensure that the VMA is not freed in our back a
reference count is added and 2 services (get_vma() and put_vma()) are
introduced to handle the reference count. When a VMA is fetch from the RB
tree using get_vma() is must be later freeed using put_vma(). Furthermore,
to allow the VMA to be used again by the classic page fault handler a
service is introduced can_reuse_spf_vma(). This service is expected to be
called with the mmap_sem hold. It checked that the VMA is still matching
the specified address and is releasing its reference count as the mmap_sem
is hold it is ensure that it will not be freed in our back. In general, the
VMA's reference count could be decremented when holding the mmap_sem but it
should not be increased as holding the mmap_sem is ensuring that the VMA is
stable. I can't see anymore the overhead I got while will-it-scale
benchmark anymore.
The VMA's attributes checked during the speculative page fault processing
have to be protected against parallel changes. This is done by using a per
VMA sequence lock. This sequence lock allows the speculative page fault
handler to fast check for parallel changes in progress and to abort the
speculative page fault in that case.
Once the VMA is found, the speculative page fault handler would check for
the VMA's attributes to verify that the page fault has to be handled
correctly or not. Thus the VMA is protected through a sequence lock which
allows fast detection of concurrent VMA changes. If such a change is
detected, the speculative page fault is aborted and a *classic* page fault
is tried. VMA sequence lockings are added when VMA attributes which are
checked during the page fault are modified.
When the PTE is fetched, the VMA is checked to see if it has been changed,
so once the page table is locked, the VMA is valid, so any other changes
leading to touching this PTE will need to lock the page table, so no
parallel change is possible at this time.
The locking of the PTE is done with interrupts disabled, this allows to
check for the PMD to ensure that there is not an ongoing collapsing
operation. Since khugepaged is firstly set the PMD to pmd_none and then is
waiting for the other CPU to have catch the IPI interrupt, if the pmd is
valid at the time the PTE is locked, we have the guarantee that the
collapsing opertion will have to wait on the PTE lock to move foward. This
allows the SPF handler to map the PTE safely. If the PMD value is different
than the one recorded at the beginning of the SPF operation, the classic
page fault handler will be called to handle the operation while holding the
mmap_sem. As the PTE lock is done with the interrupts disabled, the lock is
done using spin_trylock() to avoid dead lock when handling a page fault
while a TLB invalidate is requested by an other CPU holding the PTE.
Support for THP is not done because when checking for the PMD, we can be
confused by an in progress collapsing operation done by khugepaged. The
issue is that pmd_none() could be true either if the PMD is not already
populated or if the underlying PTE are in the way to be collapsed. So we
cannot safely allocate a PMD if pmd_none() is true.
This series a new software performance event named 'speculative-faults' or
'spf'. It counts the number of successful page fault event handled in a
speculative way. When recording 'faults,spf' events, the faults one is
counting the total number of page fault events while 'spf' is only counting
the part of the faults processed in a speculative way.
There are some trace events introduced by this series. They allow to
identify why the page faults where not processed in a speculative way. This
doesn't take in account the faults generated by a monothreaded process
which directly processed while holding the mmap_sem. This trace events are
grouped in a system named 'pagefault', they are:
- pagefault:spf_pte_lock : if the pte was already locked by another thread
- pagefault:spf_vma_changed : if the VMA has been changed in our back
- pagefault:spf_vma_noanon : the vma->anon_vma field was not yet set.
- pagefault:spf_vma_notsup : the VMA's type is not supported
- pagefault:spf_vma_access : the VMA's access right are not respected
- pagefault:spf_pmd_changed : the upper PMD pointer has changed in our
back.
To record all the related events, the easier is to run perf with the
following arguments :
$ perf stat -e 'faults,spf,pagefault:*' <command>
This series builds on top of v4.16-rc2-mmotm-2018-02-21-14-48 and is
functional on x86 and PowerPC.
---------------------
Real Workload results
As mentioned in previous email, we did non official runs using a "popular
in memory multithreaded database product" on 176 cores SMT8 Power system
which showed a 30% improvements in the number of transaction processed per
second. This run has been done on the v6 series, but changes introduced in
this new verion should not impact the performance boost seen.
Here are the perf data captured during 2 of these runs on top of the v8
series:
vanilla spf
faults 89.418 101.364
spf n/a 97.989
With the SPF kernel, most of the page fault were processed in a speculative
way.
------------------
Benchmarks results
Base kernel is v4.16-rc4-mmotm-2018-03-09-16-34
SPF is BASE + this series
Kernbench:
----------
Here are the results on a 16 CPUs X86 guest using kernbench on a 4.13-rc4
kernel (kernel is build 5 times):
Average Half load -j 8
Run (std deviation)
BASE SPF
Elapsed Time 151.36 (1.40139) 151.748 (1.09716) 0.26%
User Time 1023.19 (3.58972) 1027.35 (2.30396) 0.41%
System Time 125.026 (1.8547) 124.504 (0.980015) -0.42%
Percent CPU 758.2 (5.54076) 758.6 (3.97492) 0.05%
Context Switches 54924 (453.634) 54851 (382.293) -0.13%
Sleeps 105589 (704.581) 105282 (435.502) -0.29%
Average Optimal load -j 16
Run (std deviation)
BASE SPF
Elapsed Time 74.804 (1.25139) 74.368 (0.406288) -0.58%
User Time 962.033 (64.5125) 963.93 (66.8797) 0.20%
System Time 110.771 (15.0817) 110.387 (14.8989) -0.35%
Percent CPU 1045.7 (303.387) 1049.1 (306.255) 0.33%
Context Switches 76201.8 (22433.1) 76170.4 (22482.9) -0.04%
Sleeps 110289 (5024.05) 110220 (5248.58) -0.06%
During a run on the SPF, perf events were captured:
Performance counter stats for '../kernbench -M':
510334017 faults
200 spf
0 pagefault:spf_pte_lock
0 pagefault:spf_vma_changed
0 pagefault:spf_vma_noanon
2174 pagefault:spf_vma_notsup
0 pagefault:spf_vma_access
0 pagefault:spf_pmd_changed
Very few speculative page fault were recorded as most of the processes
involved are monothreaded (sounds that on this architecture some threads
were created during the kernel build processing).
Here are the kerbench results on a 80 CPUs Power8 system:
Average Half load -j 40
Run (std deviation)
BASE SPF
Elapsed Time 116.958 (0.73401) 117.43 (0.927497) 0.40%
User Time 4472.35 (7.85792) 4480.16 (19.4909) 0.17%
System Time 136.248 (0.587639) 136.922 (1.09058) 0.49%
Percent CPU 3939.8 (20.6567) 3931.2 (17.2829) -0.22%
Context Switches 92445.8 (236.672) 92720.8 (270.118) 0.30%
Sleeps 318475 (1412.6) 317996 (1819.07) -0.15%
Average Optimal load -j 80
Run (std deviation)
BASE SPF
Elapsed Time 106.976 (0.406731) 107.72 (0.329014) 0.70%
User Time 5863.47 (1466.45) 5865.38 (1460.27) 0.03%
System Time 159.995 (25.0393) 160.329 (24.6921) 0.21%
Percent CPU 5446.2 (1588.23) 5416 (1565.34) -0.55%
Context Switches 223018 (137637) 224867 (139305) 0.83%
Sleeps 330846 (13127.3) 332348 (15556.9) 0.45%
During a run on the SPF, perf events were captured:
Performance counter stats for '../kernbench -M':
116612488 faults
0 spf
0 pagefault:spf_pte_lock
0 pagefault:spf_vma_changed
0 pagefault:spf_vma_noanon
473 pagefault:spf_vma_notsup
0 pagefault:spf_vma_access
0 pagefault:spf_pmd_changed
Most of the processes involved are monothreaded so SPF is not activated but
there is no impact on the performance.
Ebizzy:
-------
The test is counting the number of records per second it can manage, the
higher is the best. I run it like this 'ebizzy -mTRp'. To get consistent
result I repeated the test 100 times and measure the average result. The
number is the record processes per second, the higher is the best.
BASE SPF delta
16 CPUs x86 VM 14902.6 95905.16 543.55%
80 CPUs P8 node 37240.24 78185.67 109.95%
Here are the performance counter read during a run on a 16 CPUs x86 VM:
Performance counter stats for './ebizzy -mRTp':
888157 faults
884773 spf
92 pagefault:spf_pte_lock
2379 pagefault:spf_vma_changed
0 pagefault:spf_vma_noanon
80 pagefault:spf_vma_notsup
0 pagefault:spf_vma_access
0 pagefault:spf_pmd_changed
And the ones captured during a run on a 80 CPUs Power node:
Performance counter stats for './ebizzy -mRTp':
762134 faults
728663 spf
19101 pagefault:spf_pte_lock
13969 pagefault:spf_vma_changed
0 pagefault:spf_vma_noanon
272 pagefault:spf_vma_notsup
0 pagefault:spf_vma_access
0 pagefault:spf_pmd_changed
In ebizzy's case most of the page fault were handled in a speculative way,
leading the ebizzy performance boost.
------------------
Changes since v8:
- Don't check PMD when locking the pte when THP is disabled
Thanks to Daniel Jordan for reporting this.
- Rebase on 4.16
Changes since v7:
- move pte_map_lock() and pte_spinlock() upper in mm/memory.c (patch 4 &
5)
- make pte_unmap_same() compatible with the speculative page fault (patch
6)
Changes since v6:
- Rename config variable to CONFIG_SPECULATIVE_PAGE_FAULT (patch 1)
- Review the way the config variable is set (patch 1 to 3)
- Introduce mm_rb_write_*lock() in mm/mmap.c (patch 18)
- Merge patch introducing pte try locking in the patch 18.
Changes since v5:
- use rwlock agains the mm RB tree in place of SRCU
- add a VMA's reference count to protect VMA while using it without
holding the mmap_sem.
- check PMD value to detect collapsing operation
- don't try speculative page fault for mono threaded processes
- try to reuse the fetched VMA if VM_RETRY is returned
- go directly to the error path if an error is detected during the SPF
path
- fix race window when moving VMA in move_vma()
Changes since v4:
- As requested by Andrew Morton, use CONFIG_SPF and define it earlier in
the series to ease bisection.
Changes since v3:
- Don't build when CONFIG_SMP is not set
- Fixed a lock dependency warning in __vma_adjust()
- Use READ_ONCE to access p*d values in handle_speculative_fault()
- Call memcp_oom() service in handle_speculative_fault()
Changes since v2:
- Perf event is renamed in PERF_COUNT_SW_SPF
- On Power handle do_page_fault()'s cleaning
- On Power if the VM_FAULT_ERROR is returned by
handle_speculative_fault(), do not retry but jump to the error path
- If VMA's flags are not matching the fault, directly returns
VM_FAULT_SIGSEGV and not VM_FAULT_RETRY
- Check for pud_trans_huge() to avoid speculative path
- Handles _vm_normal_page()'s introduced by 6f16211df3bf
("mm/device-public-memory: device memory cache coherent with CPU")
- add and review few comments in the code
Changes since v1:
- Remove PERF_COUNT_SW_SPF_FAILED perf event.
- Add tracing events to details speculative page fault failures.
- Cache VMA fields values which are used once the PTE is unlocked at the
end of the page fault events.
- Ensure that fields read during the speculative path are written and read
using WRITE_ONCE and READ_ONCE.
- Add checks at the beginning of the speculative path to abort it if the
VMA is known to not be supported.
Changes since RFC V5 [5]
- Port to 4.13 kernel
- Merging patch fixing lock dependency into the original patch
- Replace the 2 parameters of vma_has_changed() with the vmf pointer
- In patch 7, don't call __do_fault() in the speculative path as it may
want to unlock the mmap_sem.
- In patch 11-12, don't check for vma boundaries when
page_add_new_anon_rmap() is called during the spf path and protect against
anon_vma pointer's update.
- In patch 13-16, add performance events to report number of successful
and failed speculative events.
[1]
http://linux-kernel.2935.n7.nabble.com/RFC-PATCH-0-6-Another-go-at-speculative-page-faults-tt965642.html#none
[2] https://patchwork.kernel.org/patch/9999687/
Laurent Dufour (20):
mm: Introduce CONFIG_SPECULATIVE_PAGE_FAULT
x86/mm: Define CONFIG_SPECULATIVE_PAGE_FAULT
powerpc/mm: Define CONFIG_SPECULATIVE_PAGE_FAULT
mm: Introduce pte_spinlock for FAULT_FLAG_SPECULATIVE
mm: make pte_unmap_same compatible with SPF
mm: Protect VMA modifications using VMA sequence count
mm: protect mremap() against SPF hanlder
mm: Protect SPF handler against anon_vma changes
mm: Cache some VMA fields in the vm_fault structure
mm/migrate: Pass vm_fault pointer to migrate_misplaced_page()
mm: Introduce __lru_cache_add_active_or_unevictable
mm: Introduce __maybe_mkwrite()
mm: Introduce __vm_normal_page()
mm: Introduce __page_add_new_anon_rmap()
mm: Protect mm_rb tree with a rwlock
mm: Adding speculative page fault failure trace events
perf: Add a speculative page fault sw event
perf tools: Add support for the SPF perf event
mm: Speculative page fault handler return VMA
powerpc/mm: Add speculative page fault
Peter Zijlstra (4):
mm: Prepare for FAULT_FLAG_SPECULATIVE
mm: VMA sequence count
mm: Provide speculative fault infrastructure
x86/mm: Add speculative pagefault handling
arch/powerpc/Kconfig | 1 +
arch/powerpc/mm/fault.c | 31 +-
arch/x86/Kconfig | 1 +
arch/x86/mm/fault.c | 38 ++-
fs/proc/task_mmu.c | 5 +-
fs/userfaultfd.c | 17 +-
include/linux/hugetlb_inline.h | 2 +-
include/linux/migrate.h | 4 +-
include/linux/mm.h | 92 +++++-
include/linux/mm_types.h | 7 +
include/linux/pagemap.h | 4 +-
include/linux/rmap.h | 12 +-
include/linux/swap.h | 10 +-
include/trace/events/pagefault.h | 87 +++++
include/uapi/linux/perf_event.h | 1 +
kernel/fork.c | 3 +
mm/Kconfig | 3 +
mm/hugetlb.c | 2 +
mm/init-mm.c | 3 +
mm/internal.h | 20 ++
mm/khugepaged.c | 5 +
mm/madvise.c | 6 +-
mm/memory.c | 594 ++++++++++++++++++++++++++++++----
mm/mempolicy.c | 51 ++-
mm/migrate.c | 4 +-
mm/mlock.c | 13 +-
mm/mmap.c | 211 +++++++++---
mm/mprotect.c | 4 +-
mm/mremap.c | 13 +
mm/rmap.c | 5 +-
mm/swap.c | 6 +-
mm/swap_state.c | 8 +-
tools/include/uapi/linux/perf_event.h | 1 +
tools/perf/util/evsel.c | 1 +
tools/perf/util/parse-events.c | 4 +
tools/perf/util/parse-events.l | 1 +
tools/perf/util/python.c | 1 +
37 files changed, 1097 insertions(+), 174 deletions(-)
create mode 100644 include/trace/events/pagefault.h
--
2.7.4
^ 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