* [PATCH v2 4/8] powernv/memtrace: always online added memory blocks
From: David Hildenbrand @ 2020-03-17 10:49 UTC (permalink / raw)
To: linux-kernel
Cc: linux-hyperv, Baoquan He, David Hildenbrand, Rafael J. Wysocki,
Greg Kroah-Hartman, Michal Hocko, linux-mm, Paul Mackerras,
Andrew Morton, Wei Yang, linuxppc-dev, Oscar Salvador
In-Reply-To: <20200317104942.11178-1-david@redhat.com>
Let's always try to online the re-added memory blocks. In case add_memory()
already onlined the added memory blocks, the first device_online() call
will fail and stop processing the remaining memory blocks.
This avoids manually having to check memhp_auto_online.
Note: PPC always onlines all hotplugged memory directly from the kernel
as well - something that is handled by user space on other
architectures.
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/powerpc/platforms/powernv/memtrace.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
index d6d64f8718e6..13b369d2cc45 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -231,16 +231,10 @@ static int memtrace_online(void)
continue;
}
- /*
- * If kernel isn't compiled with the auto online option
- * we need to online the memory ourselves.
- */
- if (!memhp_auto_online) {
- lock_device_hotplug();
- walk_memory_blocks(ent->start, ent->size, NULL,
- online_mem_block);
- unlock_device_hotplug();
- }
+ lock_device_hotplug();
+ walk_memory_blocks(ent->start, ent->size, NULL,
+ online_mem_block);
+ unlock_device_hotplug();
/*
* Memory was added successfully so clean up references to it
--
2.24.1
^ permalink raw reply related
* [PATCH v2 3/8] drivers/base/memory: store mapping between MMOP_* and string in an array
From: David Hildenbrand @ 2020-03-17 10:49 UTC (permalink / raw)
To: linux-kernel
Cc: linux-hyperv, Michal Hocko, Baoquan He, David Hildenbrand,
Greg Kroah-Hartman, Rafael J. Wysocki, Wei Yang, linux-mm,
Andrew Morton, Michal Hocko, linuxppc-dev, Oscar Salvador
In-Reply-To: <20200317104942.11178-1-david@redhat.com>
Let's use a simple array which we can reuse soon. While at it, move the
string->mmop conversion out of the device hotplug lock.
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
drivers/base/memory.c | 38 +++++++++++++++++++++++---------------
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index e7e77cafef80..8a7f29c0bf97 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -28,6 +28,24 @@
#define MEMORY_CLASS_NAME "memory"
+static const char *const online_type_to_str[] = {
+ [MMOP_OFFLINE] = "offline",
+ [MMOP_ONLINE] = "online",
+ [MMOP_ONLINE_KERNEL] = "online_kernel",
+ [MMOP_ONLINE_MOVABLE] = "online_movable",
+};
+
+static int memhp_online_type_from_str(const char *str)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(online_type_to_str); i++) {
+ if (sysfs_streq(str, online_type_to_str[i]))
+ return i;
+ }
+ return -EINVAL;
+}
+
#define to_memory_block(dev) container_of(dev, struct memory_block, dev)
static int sections_per_block;
@@ -236,26 +254,17 @@ static int memory_subsys_offline(struct device *dev)
static ssize_t state_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
+ const int online_type = memhp_online_type_from_str(buf);
struct memory_block *mem = to_memory_block(dev);
- int ret, online_type;
+ int ret;
+
+ if (online_type < 0)
+ return -EINVAL;
ret = lock_device_hotplug_sysfs();
if (ret)
return ret;
- if (sysfs_streq(buf, "online_kernel"))
- online_type = MMOP_ONLINE_KERNEL;
- else if (sysfs_streq(buf, "online_movable"))
- online_type = MMOP_ONLINE_MOVABLE;
- else if (sysfs_streq(buf, "online"))
- online_type = MMOP_ONLINE;
- else if (sysfs_streq(buf, "offline"))
- online_type = MMOP_OFFLINE;
- else {
- ret = -EINVAL;
- goto err;
- }
-
switch (online_type) {
case MMOP_ONLINE_KERNEL:
case MMOP_ONLINE_MOVABLE:
@@ -271,7 +280,6 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr,
ret = -EINVAL; /* should never happen */
}
-err:
unlock_device_hotplug();
if (ret < 0)
--
2.24.1
^ permalink raw reply related
* [PATCH v2 2/8] drivers/base/memory: map MMOP_OFFLINE to 0
From: David Hildenbrand @ 2020-03-17 10:49 UTC (permalink / raw)
To: linux-kernel
Cc: linux-hyperv, Michal Hocko, Baoquan He, David Hildenbrand,
Greg Kroah-Hartman, Rafael J. Wysocki, Wei Yang, linux-mm,
Andrew Morton, Michal Hocko, linuxppc-dev, Oscar Salvador
In-Reply-To: <20200317104942.11178-1-david@redhat.com>
Historically, we used the value -1. Just treat 0 as the special
case now. Clarify a comment (which was wrong, when we come via
device_online() the first time, the online_type would have been 0 /
MEM_ONLINE). The default is now always MMOP_OFFLINE. This removes the
last user of the manual "-1", which didn't use the enum value.
This is a preparation to use the online_type as an array index.
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
drivers/base/memory.c | 11 ++++-------
include/linux/memory_hotplug.h | 2 +-
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 8c5ce42c0fc3..e7e77cafef80 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -211,17 +211,14 @@ static int memory_subsys_online(struct device *dev)
return 0;
/*
- * If we are called from state_store(), online_type will be
- * set >= 0 Otherwise we were called from the device online
- * attribute and need to set the online_type.
+ * When called via device_online() without configuring the online_type,
+ * we want to default to MMOP_ONLINE.
*/
- if (mem->online_type < 0)
+ if (mem->online_type == MMOP_OFFLINE)
mem->online_type = MMOP_ONLINE;
ret = memory_block_change_state(mem, MEM_ONLINE, MEM_OFFLINE);
-
- /* clear online_type */
- mem->online_type = -1;
+ mem->online_type = MMOP_OFFLINE;
return ret;
}
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 261dbf010d5d..c2e06ed5e0e9 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -48,7 +48,7 @@ enum {
/* Types for control the zone type of onlined and offlined memory */
enum {
/* Offline the memory. */
- MMOP_OFFLINE = -1,
+ MMOP_OFFLINE = 0,
/* Online the memory. Zone depends, see default_zone_for_pfn(). */
MMOP_ONLINE,
/* Online the memory to ZONE_NORMAL. */
--
2.24.1
^ permalink raw reply related
* [PATCH v2 1/8] drivers/base/memory: rename MMOP_ONLINE_KEEP to MMOP_ONLINE
From: David Hildenbrand @ 2020-03-17 10:49 UTC (permalink / raw)
To: linux-kernel
Cc: linux-hyperv, Baoquan He, David Hildenbrand, Greg Kroah-Hartman,
Rafael J. Wysocki, Wei Yang, linux-mm, Andrew Morton,
Michal Hocko, linuxppc-dev, Oscar Salvador
In-Reply-To: <20200317104942.11178-1-david@redhat.com>
The name is misleading and it's not really clear what is "kept". Let's just
name it like the online_type name we expose to user space ("online").
Add some documentation to the types.
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
drivers/base/memory.c | 9 +++++----
include/linux/memory_hotplug.h | 6 +++++-
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 6448c9ece2cb..8c5ce42c0fc3 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -216,7 +216,7 @@ static int memory_subsys_online(struct device *dev)
* attribute and need to set the online_type.
*/
if (mem->online_type < 0)
- mem->online_type = MMOP_ONLINE_KEEP;
+ mem->online_type = MMOP_ONLINE;
ret = memory_block_change_state(mem, MEM_ONLINE, MEM_OFFLINE);
@@ -251,7 +251,7 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr,
else if (sysfs_streq(buf, "online_movable"))
online_type = MMOP_ONLINE_MOVABLE;
else if (sysfs_streq(buf, "online"))
- online_type = MMOP_ONLINE_KEEP;
+ online_type = MMOP_ONLINE;
else if (sysfs_streq(buf, "offline"))
online_type = MMOP_OFFLINE;
else {
@@ -262,7 +262,7 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr,
switch (online_type) {
case MMOP_ONLINE_KERNEL:
case MMOP_ONLINE_MOVABLE:
- case MMOP_ONLINE_KEEP:
+ case MMOP_ONLINE:
/* mem->online_type is protected by device_hotplug_lock */
mem->online_type = online_type;
ret = device_online(&mem->dev);
@@ -342,7 +342,8 @@ static ssize_t valid_zones_show(struct device *dev,
}
nid = mem->nid;
- default_zone = zone_for_pfn_range(MMOP_ONLINE_KEEP, nid, start_pfn, nr_pages);
+ default_zone = zone_for_pfn_range(MMOP_ONLINE, nid, start_pfn,
+ nr_pages);
strcat(buf, default_zone->name);
print_allowed_zone(buf, nid, start_pfn, nr_pages, MMOP_ONLINE_KERNEL,
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index f4d59155f3d4..261dbf010d5d 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -47,9 +47,13 @@ enum {
/* Types for control the zone type of onlined and offlined memory */
enum {
+ /* Offline the memory. */
MMOP_OFFLINE = -1,
- MMOP_ONLINE_KEEP,
+ /* Online the memory. Zone depends, see default_zone_for_pfn(). */
+ MMOP_ONLINE,
+ /* Online the memory to ZONE_NORMAL. */
MMOP_ONLINE_KERNEL,
+ /* Online the memory to ZONE_MOVABLE. */
MMOP_ONLINE_MOVABLE,
};
--
2.24.1
^ permalink raw reply related
* [PATCH v2 0/8] mm/memory_hotplug: allow to specify a default online_type
From: David Hildenbrand @ 2020-03-17 10:49 UTC (permalink / raw)
To: linux-kernel
Cc: Yumei Huang, linux-hyperv, Michal Hocko, David Hildenbrand,
Michal Hocko, linux-mm, Paul Mackerras, K. Y. Srinivasan, Wei Liu,
Stephen Hemminger, Baoquan He, Rafael J. Wysocki, Eduardo Habkost,
Haiyang Zhang, Wei Yang, Andrew Morton, Oscar Salvador,
Milan Zamazal, Greg Kroah-Hartman, Igor Mammedov,
Vitaly Kuznetsov, linuxppc-dev
Distributions nowadays use udev rules ([1] [2]) to specify if and
how to online hotplugged memory. The rules seem to get more complex with
many special cases. Due to the various special cases,
CONFIG_MEMORY_HOTPLUG_DEFAULT_ONLINE cannot be used. All memory hotplug
is handled via udev rules.
Everytime we hotplug memory, the udev rule will come to the same
conclusion. Especially Hyper-V (but also soon virtio-mem) add a lot of
memory in separate memory blocks and wait for memory to get onlined by user
space before continuing to add more memory blocks (to not add memory faster
than it is getting onlined). This of course slows down the whole memory
hotplug process.
To make the job of distributions easier and to avoid udev rules that get
more and more complicated, let's extend the mechanism provided by
- /sys/devices/system/memory/auto_online_blocks
- "memhp_default_state=" on the kernel cmdline
to be able to specify also "online_movable" as well as "online_kernel"
v1 -> v2:
- Tweaked some patch descriptions
- Added
-- "powernv/memtrace: always online added memory blocks"
-- "hv_balloon: don't check for memhp_auto_online manually"
-- "mm/memory_hotplug: unexport memhp_auto_online"
- "mm/memory_hotplug: convert memhp_auto_online to store an online_type"
-- No longer touches hv/memtrace code
=== Example /usr/libexec/config-memhotplug ===
#!/bin/bash
VIRT=`systemd-detect-virt --vm`
ARCH=`uname -p`
sense_virtio_mem() {
if [ -d "/sys/bus/virtio/drivers/virtio_mem/" ]; then
DEVICES=`find /sys/bus/virtio/drivers/virtio_mem/ -maxdepth 1 -type l | wc -l`
if [ $DEVICES != "0" ]; then
return 0
fi
fi
return 1
}
if [ ! -e "/sys/devices/system/memory/auto_online_blocks" ]; then
echo "Memory hotplug configuration support missing in the kernel"
exit 1
fi
if grep "memhp_default_state=" /proc/cmdline > /dev/null; then
echo "Memory hotplug configuration overridden in kernel cmdline (memhp_default_state=)"
exit 1
fi
if [ $VIRT == "microsoft" ]; then
echo "Detected Hyper-V on $ARCH"
# Hyper-V wants all memory in ZONE_NORMAL
ONLINE_TYPE="online_kernel"
elif sense_virtio_mem; then
echo "Detected virtio-mem on $ARCH"
# virtio-mem wants all memory in ZONE_NORMAL
ONLINE_TYPE="online_kernel"
elif [ $ARCH == "s390x" ] || [ $ARCH == "s390" ]; then
echo "Detected $ARCH"
# standby memory should not be onlined automatically
ONLINE_TYPE="offline"
elif [ $ARCH == "ppc64" ] || [ $ARCH == "ppc64le" ]; then
echo "Detected" $ARCH
# PPC64 onlines all hotplugged memory right from the kernel
ONLINE_TYPE="offline"
elif [ $VIRT == "none" ]; then
echo "Detected bare-metal on $ARCH"
# Bare metal users expect hotplugged memory to be unpluggable. We assume
# that ZONE imbalances on such enterpise servers cannot happen and is
# properly documented
ONLINE_TYPE="online_movable"
else
# TODO: Hypervisors that want to unplug DIMMs and can guarantee that ZONE
# imbalances won't happen
echo "Detected $VIRT on $ARCH"
# Usually, ballooning is used in virtual environments, so memory should go to
# ZONE_NORMAL. However, sometimes "movable_node" is relevant.
ONLINE_TYPE="online"
fi
echo "Selected online_type:" $ONLINE_TYPE
# Configure what to do with memory that will be hotplugged in the future
echo $ONLINE_TYPE 2>/dev/null > /sys/devices/system/memory/auto_online_blocks
if [ $? != "0" ]; then
echo "Memory hotplug cannot be configured (e.g., old kernel or missing permissions)"
# A backup udev rule should handle old kernels if necessary
exit 1
fi
# Process all already pluggedd blocks (e.g., DIMMs, but also Hyper-V or virtio-mem)
if [ $ONLINE_TYPE != "offline" ]; then
for MEMORY in /sys/devices/system/memory/memory*; do
STATE=`cat $MEMORY/state`
if [ $STATE == "offline" ]; then
echo $ONLINE_TYPE > $MEMORY/state
fi
done
fi
=== Example /usr/lib/systemd/system/config-memhotplug.service ===
[Unit]
Description=Configure memory hotplug behavior
DefaultDependencies=no
Conflicts=shutdown.target
Before=sysinit.target shutdown.target
After=systemd-modules-load.service
ConditionPathExists=|/sys/devices/system/memory/auto_online_blocks
[Service]
ExecStart=/usr/libexec/config-memhotplug
Type=oneshot
TimeoutSec=0
RemainAfterExit=yes
[Install]
WantedBy=sysinit.target
=== Example modification to the 40-redhat.rules [2] ===
diff --git a/40-redhat.rules b/40-redhat.rules-new
index 2c690e5..168fd03 100644
--- a/40-redhat.rules
+++ b/40-redhat.rules-new
@@ -6,6 +6,9 @@ SUBSYSTEM=="cpu", ACTION=="add", TEST=="online", ATTR{online}=="0", ATTR{online}
# Memory hotadd request
SUBSYSTEM!="memory", GOTO="memory_hotplug_end"
ACTION!="add", GOTO="memory_hotplug_end"
+# memory hotplug behavior configured
+PROGRAM=="grep online /sys/devices/system/memory/auto_online_blocks", GOTO="memory_hotplug_end"
+
PROGRAM="/bin/uname -p", RESULT=="s390*", GOTO="memory_hotplug_end"
ENV{.state}="online"
===
[1] https://github.com/lnykryn/systemd-rhel/pull/281
[2] https://github.com/lnykryn/systemd-rhel/blob/staging/rules/40-redhat.rules
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Yumei Huang <yuhuang@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Baoquan He <bhe@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Milan Zamazal <mzamazal@redhat.com>
David Hildenbrand (8):
drivers/base/memory: rename MMOP_ONLINE_KEEP to MMOP_ONLINE
drivers/base/memory: map MMOP_OFFLINE to 0
drivers/base/memory: store mapping between MMOP_* and string in an
array
powernv/memtrace: always online added memory blocks
hv_balloon: don't check for memhp_auto_online manually
mm/memory_hotplug: unexport memhp_auto_online
mm/memory_hotplug: convert memhp_auto_online to store an online_type
mm/memory_hotplug: allow to specify a default online_type
arch/powerpc/platforms/powernv/memtrace.c | 14 ++---
drivers/base/memory.c | 71 ++++++++++++-----------
drivers/hv/hv_balloon.c | 25 ++++----
include/linux/memory_hotplug.h | 13 ++++-
mm/memory_hotplug.c | 16 ++---
5 files changed, 69 insertions(+), 70 deletions(-)
--
2.24.1
^ permalink raw reply related
* Re: [PATCH 11/15] powerpc/watchpoint: Introduce is_ptrace_bp() function
From: Christophe Leroy @ 2020-03-17 10:49 UTC (permalink / raw)
To: Ravi Bangoria, mpe, mikey
Cc: apopple, peterz, fweisbec, oleg, npiggin, linux-kernel, paulus,
jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200309085806.155823-12-ravi.bangoria@linux.ibm.com>
Le 09/03/2020 à 09:58, Ravi Bangoria a écrit :
> Introduce is_ptrace_bp() function and move the check inside the
> function. We will utilize it more in later set of patches.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> arch/powerpc/kernel/hw_breakpoint.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
> index b27aca623267..0e35ff372d8e 100644
> --- a/arch/powerpc/kernel/hw_breakpoint.c
> +++ b/arch/powerpc/kernel/hw_breakpoint.c
> @@ -90,6 +90,11 @@ void arch_uninstall_hw_breakpoint(struct perf_event *bp)
> hw_breakpoint_disable();
> }
>
> +static bool is_ptrace_bp(struct perf_event *bp)
> +{
> + return (bp->overflow_handler == ptrace_triggered);
You don't need parenthesis here.
> +}
> +
> /*
> * Perform cleanup of arch-specific counters during unregistration
> * of the perf-event
> @@ -324,7 +329,7 @@ int hw_breakpoint_handler(struct die_args *args)
> * one-shot mode. The ptrace-ed process will receive the SIGTRAP signal
> * generated in do_dabr().
> */
> - if (bp->overflow_handler == ptrace_triggered) {
> + if (is_ptrace_bp(bp)) {
> perf_bp_event(bp, regs);
> rc = NOTIFY_DONE;
> goto out;
>
Christophe
^ permalink raw reply
* Re: [PATCH 10/15] powerpc/watchpoint: Use loop for thread_struct->ptrace_bps
From: Christophe Leroy @ 2020-03-17 10:48 UTC (permalink / raw)
To: Ravi Bangoria, mpe, mikey
Cc: apopple, peterz, fweisbec, oleg, npiggin, linux-kernel, paulus,
jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200309085806.155823-11-ravi.bangoria@linux.ibm.com>
Le 09/03/2020 à 09:58, Ravi Bangoria a écrit :
> ptrace_bps is already an array of size HBP_NUM_MAX. But we use
> hardcoded index 0 while fetching/updating it. Convert such code
> to loop over array.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> arch/powerpc/kernel/hw_breakpoint.c | 7 +++++--
> arch/powerpc/kernel/process.c | 6 +++++-
> arch/powerpc/kernel/ptrace.c | 28 +++++++++++++++++++++-------
> 3 files changed, 31 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
> index f4d48f87dcb8..b27aca623267 100644
> --- a/arch/powerpc/kernel/hw_breakpoint.c
> +++ b/arch/powerpc/kernel/hw_breakpoint.c
> @@ -419,10 +419,13 @@ NOKPROBE_SYMBOL(hw_breakpoint_exceptions_notify);
> */
> void flush_ptrace_hw_breakpoint(struct task_struct *tsk)
> {
> + int i;
> struct thread_struct *t = &tsk->thread;
>
> - unregister_hw_breakpoint(t->ptrace_bps[0]);
> - t->ptrace_bps[0] = NULL;
> + for (i = 0; i < nr_wp_slots(); i++) {
> + unregister_hw_breakpoint(t->ptrace_bps[i]);
> + t->ptrace_bps[i] = NULL;
> + }
> }
>
> void hw_breakpoint_pmu_read(struct perf_event *bp)
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 42ff62ef749c..b9ab740fcacf 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1628,6 +1628,9 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
> void (*f)(void);
> unsigned long sp = (unsigned long)task_stack_page(p) + THREAD_SIZE;
> struct thread_info *ti = task_thread_info(p);
> +#ifdef CONFIG_HAVE_HW_BREAKPOINT
> + int i;
> +#endif
Could we avoid all those #ifdefs ?
I think if we make p->thread.ptrace_bps[] exist all the time, with a
size of 0 when CONFIG_HAVE_HW_BREAKPOINT is not set, then we can drop a
lot of #ifdefs.
>
> klp_init_thread_info(p);
>
> @@ -1687,7 +1690,8 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
> p->thread.ksp_limit = (unsigned long)end_of_stack(p);
> #endif
> #ifdef CONFIG_HAVE_HW_BREAKPOINT
> - p->thread.ptrace_bps[0] = NULL;
> + for (i = 0; i < nr_wp_slots(); i++)
> + p->thread.ptrace_bps[i] = NULL;
> #endif
>
> p->thread.fp_save_area = NULL;
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index f6d7955fc61e..e2651f86d56f 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
You'll have to rebase all this on the series
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=161356
which is about to go into powerpc-next
> @@ -2829,6 +2829,19 @@ static int set_dac_range(struct task_struct *child,
> }
> #endif /* CONFIG_PPC_ADV_DEBUG_DAC_RANGE */
>
> +#ifdef CONFIG_HAVE_HW_BREAKPOINT
> +static int empty_ptrace_bp(struct thread_struct *thread)
> +{
> + int i;
> +
> + for (i = 0; i < nr_wp_slots(); i++) {
> + if (!thread->ptrace_bps[i])
> + return i;
> + }
> + return -1;
> +}
> +#endif
What does this function do exactly ? I seems to do more than what its
name suggests.
> +
> #ifndef CONFIG_PPC_ADV_DEBUG_REGS
> static int empty_hw_brk(struct thread_struct *thread)
> {
> @@ -2915,8 +2928,9 @@ static long ppc_set_hwdebug(struct task_struct *child,
> len = 1;
> else
> return -EINVAL;
> - bp = thread->ptrace_bps[0];
> - if (bp)
> +
> + i = empty_ptrace_bp(thread);
> + if (i < 0)
> return -ENOSPC;
>
> /* Create a new breakpoint request if one doesn't exist already */
> @@ -2925,14 +2939,14 @@ static long ppc_set_hwdebug(struct task_struct *child,
> attr.bp_len = len;
> arch_bp_generic_fields(brk.type, &attr.bp_type);
>
> - thread->ptrace_bps[0] = bp = register_user_hw_breakpoint(&attr,
> + thread->ptrace_bps[i] = bp = register_user_hw_breakpoint(&attr,
> ptrace_triggered, NULL, child);
> if (IS_ERR(bp)) {
> - thread->ptrace_bps[0] = NULL;
> + thread->ptrace_bps[i] = NULL;
> return PTR_ERR(bp);
> }
>
> - return 1;
> + return i + 1;
> #endif /* CONFIG_HAVE_HW_BREAKPOINT */
>
> if (bp_info->addr_mode != PPC_BREAKPOINT_MODE_EXACT)
> @@ -2979,10 +2993,10 @@ static long ppc_del_hwdebug(struct task_struct *child, long data)
> return -EINVAL;
>
> #ifdef CONFIG_HAVE_HW_BREAKPOINT
> - bp = thread->ptrace_bps[0];
> + bp = thread->ptrace_bps[data - 1];
Is data checked somewhere to ensure it is not out of boundaries ? Or are
we sure it is always within ?
> if (bp) {
> unregister_hw_breakpoint(bp);
> - thread->ptrace_bps[0] = NULL;
> + thread->ptrace_bps[data - 1] = NULL;
> } else
> ret = -ENOENT;
> return ret;
>
Christophe
^ permalink raw reply
* Re: [PATCH 09/15] powerpc/watchpoint: Convert thread_struct->hw_brk to an array
From: Christophe Leroy @ 2020-03-17 10:37 UTC (permalink / raw)
To: Ravi Bangoria, mpe, mikey
Cc: apopple, peterz, fweisbec, oleg, npiggin, linux-kernel, paulus,
jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200309085806.155823-10-ravi.bangoria@linux.ibm.com>
Le 09/03/2020 à 09:58, Ravi Bangoria a écrit :
> So far powerpc hw supported only one watchpoint. But Future Power
> architecture is introducing 2nd DAWR. Convert thread_struct->hw_brk
> into an array.
Looks like you are doing a lot more than that in this patch.
Should this patch be splitted in two parts ?
Christophe
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> arch/powerpc/include/asm/processor.h | 2 +-
> arch/powerpc/kernel/process.c | 43 ++++++++++++++++++++--------
> arch/powerpc/kernel/ptrace.c | 42 ++++++++++++++++++++-------
> arch/powerpc/kernel/ptrace32.c | 4 +--
> arch/powerpc/kernel/signal.c | 9 ++++--
> 5 files changed, 72 insertions(+), 28 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index 666b2825278c..57a8fac2e72b 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -183,7 +183,7 @@ struct thread_struct {
> */
> struct perf_event *last_hit_ubp;
> #endif /* CONFIG_HAVE_HW_BREAKPOINT */
> - struct arch_hw_breakpoint hw_brk; /* info on the hardware breakpoint */
> + struct arch_hw_breakpoint hw_brk[HBP_NUM_MAX]; /* hardware breakpoint info */
> unsigned long trap_nr; /* last trap # on this thread */
> u8 load_slb; /* Ages out SLB preload cache entries */
> u8 load_fp;
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index f6bb2586fa5d..42ff62ef749c 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -704,21 +704,25 @@ void switch_booke_debug_regs(struct debug_reg *new_debug)
> EXPORT_SYMBOL_GPL(switch_booke_debug_regs);
> #else /* !CONFIG_PPC_ADV_DEBUG_REGS */
> #ifndef CONFIG_HAVE_HW_BREAKPOINT
> -static void set_breakpoint(struct arch_hw_breakpoint *brk)
> +static void set_breakpoint(struct arch_hw_breakpoint *brk, int i)
> {
> preempt_disable();
> - __set_breakpoint(brk, 0);
> + __set_breakpoint(brk, i);
> preempt_enable();
> }
>
> static void set_debug_reg_defaults(struct thread_struct *thread)
> {
> - thread->hw_brk.address = 0;
> - thread->hw_brk.type = 0;
> - thread->hw_brk.len = 0;
> - thread->hw_brk.hw_len = 0;
> - if (ppc_breakpoint_available())
> - set_breakpoint(&thread->hw_brk);
> + int i;
> +
> + for (i = 0; i < nr_wp_slots(); i++) {
> + thread->hw_brk[i].address = 0;
> + thread->hw_brk[i].type = 0;
> + thread->hw_brk[i].len = 0;
> + thread->hw_brk[i].hw_len = 0;
> + if (ppc_breakpoint_available())
> + set_breakpoint(&thread->hw_brk[i], i);
> + }
> }
> #endif /* !CONFIG_HAVE_HW_BREAKPOINT */
> #endif /* CONFIG_PPC_ADV_DEBUG_REGS */
> @@ -1141,6 +1145,24 @@ static inline void restore_sprs(struct thread_struct *old_thread,
> thread_pkey_regs_restore(new_thread, old_thread);
> }
>
> +#ifndef CONFIG_HAVE_HW_BREAKPOINT
> +static void switch_hw_breakpoint(struct task_struct *new)
> +{
> + int i;
> +
> + for (i = 0; i < nr_wp_slots(); i++) {
> + if (unlikely(!hw_brk_match(this_cpu_ptr(¤t_brk[i]),
> + &new->thread.hw_brk[i]))) {
> + __set_breakpoint(&new->thread.hw_brk[i], i);
> + }
> + }
> +}
> +#else
> +static void switch_hw_breakpoint(struct task_struct *new)
> +{
> +}
> +#endif
> +
> struct task_struct *__switch_to(struct task_struct *prev,
> struct task_struct *new)
> {
> @@ -1172,10 +1194,7 @@ struct task_struct *__switch_to(struct task_struct *prev,
> * For PPC_BOOK3S_64, we use the hw-breakpoint interfaces that would
> * schedule DABR
> */
> -#ifndef CONFIG_HAVE_HW_BREAKPOINT
> - if (unlikely(!hw_brk_match(this_cpu_ptr(¤t_brk[0]), &new->thread.hw_brk)))
> - __set_breakpoint(&new->thread.hw_brk, 0);
> -#endif /* CONFIG_HAVE_HW_BREAKPOINT */
> + switch_hw_breakpoint(new);
> #endif
>
> /*
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index dd46e174dbe7..f6d7955fc61e 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -2382,6 +2382,11 @@ void ptrace_triggered(struct perf_event *bp,
> }
> #endif /* CONFIG_HAVE_HW_BREAKPOINT */
>
> +/*
> + * ptrace_set_debugreg() fakes DABR and DABR is only one. So even if
> + * internal hw supports more than one watchpoint, we support only one
> + * watchpoint with this interface.
> + */
> static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
> unsigned long data)
> {
> @@ -2451,7 +2456,7 @@ static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
> return ret;
> }
> thread->ptrace_bps[0] = bp;
> - thread->hw_brk = hw_brk;
> + thread->hw_brk[0] = hw_brk;
> return 0;
> }
>
> @@ -2473,7 +2478,7 @@ static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
> if (set_bp && (!ppc_breakpoint_available()))
> return -ENODEV;
> #endif /* CONFIG_HAVE_HW_BREAKPOINT */
> - task->thread.hw_brk = hw_brk;
> + task->thread.hw_brk[0] = hw_brk;
> #else /* CONFIG_PPC_ADV_DEBUG_REGS */
> /* As described above, it was assumed 3 bits were passed with the data
> * address, but we will assume only the mode bits will be passed
> @@ -2824,9 +2829,23 @@ static int set_dac_range(struct task_struct *child,
> }
> #endif /* CONFIG_PPC_ADV_DEBUG_DAC_RANGE */
>
> +#ifndef CONFIG_PPC_ADV_DEBUG_REGS
> +static int empty_hw_brk(struct thread_struct *thread)
> +{
> + int i;
> +
> + for (i = 0; i < nr_wp_slots(); i++) {
> + if (!thread->hw_brk[i].address)
> + return i;
> + }
> + return -1;
> +}
> +#endif
> +
> static long ppc_set_hwdebug(struct task_struct *child,
> struct ppc_hw_breakpoint *bp_info)
> {
> + int i;
> #ifdef CONFIG_HAVE_HW_BREAKPOINT
> int len = 0;
> struct thread_struct *thread = &(child->thread);
> @@ -2919,15 +2938,16 @@ static long ppc_set_hwdebug(struct task_struct *child,
> if (bp_info->addr_mode != PPC_BREAKPOINT_MODE_EXACT)
> return -EINVAL;
>
> - if (child->thread.hw_brk.address)
> + i = empty_hw_brk(&child->thread);
> + if (i < 0)
> return -ENOSPC;
>
> if (!ppc_breakpoint_available())
> return -ENODEV;
>
> - child->thread.hw_brk = brk;
> + child->thread.hw_brk[i] = brk;
>
> - return 1;
> + return i + 1;
> #endif /* !CONFIG_PPC_ADV_DEBUG_DVCS */
> }
>
> @@ -2955,7 +2975,7 @@ static long ppc_del_hwdebug(struct task_struct *child, long data)
> }
> return rc;
> #else
> - if (data != 1)
> + if (data < 1 || data > nr_wp_slots())
> return -EINVAL;
>
> #ifdef CONFIG_HAVE_HW_BREAKPOINT
> @@ -2967,11 +2987,11 @@ static long ppc_del_hwdebug(struct task_struct *child, long data)
> ret = -ENOENT;
> return ret;
> #else /* CONFIG_HAVE_HW_BREAKPOINT */
> - if (child->thread.hw_brk.address == 0)
> + if (child->thread.hw_brk[data - 1].address == 0)
> return -ENOENT;
>
> - child->thread.hw_brk.address = 0;
> - child->thread.hw_brk.type = 0;
> + child->thread.hw_brk[data - 1].address = 0;
> + child->thread.hw_brk[data - 1].type = 0;
> #endif /* CONFIG_HAVE_HW_BREAKPOINT */
>
> return 0;
> @@ -3124,8 +3144,8 @@ long arch_ptrace(struct task_struct *child, long request,
> #ifdef CONFIG_PPC_ADV_DEBUG_REGS
> ret = put_user(child->thread.debug.dac1, datalp);
> #else
> - dabr_fake = ((child->thread.hw_brk.address & (~HW_BRK_TYPE_DABR)) |
> - (child->thread.hw_brk.type & HW_BRK_TYPE_DABR));
> + dabr_fake = ((child->thread.hw_brk[0].address & (~HW_BRK_TYPE_DABR)) |
> + (child->thread.hw_brk[0].type & HW_BRK_TYPE_DABR));
> ret = put_user(dabr_fake, datalp);
> #endif
> break;
> diff --git a/arch/powerpc/kernel/ptrace32.c b/arch/powerpc/kernel/ptrace32.c
> index f37eb53de1a1..e227cd320b46 100644
> --- a/arch/powerpc/kernel/ptrace32.c
> +++ b/arch/powerpc/kernel/ptrace32.c
> @@ -270,8 +270,8 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
> ret = put_user(child->thread.debug.dac1, (u32 __user *)data);
> #else
> dabr_fake = (
> - (child->thread.hw_brk.address & (~HW_BRK_TYPE_DABR)) |
> - (child->thread.hw_brk.type & HW_BRK_TYPE_DABR));
> + (child->thread.hw_brk[0].address & (~HW_BRK_TYPE_DABR)) |
> + (child->thread.hw_brk[0].type & HW_BRK_TYPE_DABR));
> ret = put_user(dabr_fake, (u32 __user *)data);
> #endif
> break;
> diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
> index 8bc6cc55420a..3116896e89a6 100644
> --- a/arch/powerpc/kernel/signal.c
> +++ b/arch/powerpc/kernel/signal.c
> @@ -107,6 +107,9 @@ static void do_signal(struct task_struct *tsk)
> struct ksignal ksig = { .sig = 0 };
> int ret;
> int is32 = is_32bit_task();
> +#ifndef CONFIG_PPC_ADV_DEBUG_REGS
> + int i;
> +#endif
>
> BUG_ON(tsk != current);
>
> @@ -128,8 +131,10 @@ static void do_signal(struct task_struct *tsk)
> * user space. The DABR will have been cleared if it
> * triggered inside the kernel.
> */
> - if (tsk->thread.hw_brk.address && tsk->thread.hw_brk.type)
> - __set_breakpoint(&tsk->thread.hw_brk, 0);
> + for (i = 0; i < nr_wp_slots(); i++) {
> + if (tsk->thread.hw_brk[i].address && tsk->thread.hw_brk[i].type)
> + __set_breakpoint(&tsk->thread.hw_brk[i], i);
> + }
> #endif
> /* Re-enable the breakpoints for the signal stack */
> thread_change_pc(tsk, tsk->thread.regs);
>
^ permalink raw reply
* Re: [PATCH 08/15] powerpc/watchpoint: Disable all available watchpoints when !dawr_force_enable
From: Christophe Leroy @ 2020-03-17 10:35 UTC (permalink / raw)
To: Ravi Bangoria, mpe, mikey
Cc: apopple, peterz, fweisbec, oleg, npiggin, linux-kernel, paulus,
jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200309085806.155823-9-ravi.bangoria@linux.ibm.com>
Le 09/03/2020 à 09:57, Ravi Bangoria a écrit :
> Instead of disabling only first watchpoint, disable all available
> watchpoints while clearing dawr_force_enable.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> arch/powerpc/kernel/dawr.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
> index 311e51ee09f4..5c882f07ac7d 100644
> --- a/arch/powerpc/kernel/dawr.c
> +++ b/arch/powerpc/kernel/dawr.c
> @@ -50,9 +50,13 @@ int set_dawr(struct arch_hw_breakpoint *brk, int nr)
> return 0;
> }
>
> -static void set_dawr_cb(void *info)
> +static void disable_dawrs(void *info)
Can you explain a bit more what you do exactly ? Why do you change the
name of the function and why the parameter becomes NULL ? And why it
doens't take into account the parameter anymore ?
> {
> - set_dawr(info, 0);
> + struct arch_hw_breakpoint null_brk = {0};
> + int i;
> +
> + for (i = 0; i < nr_wp_slots(); i++)
> + set_dawr(&null_brk, i);
> }
>
> static ssize_t dawr_write_file_bool(struct file *file,
> @@ -74,7 +78,7 @@ static ssize_t dawr_write_file_bool(struct file *file,
>
> /* If we are clearing, make sure all CPUs have the DAWR cleared */
> if (!dawr_force_enable)
> - smp_call_function(set_dawr_cb, &null_brk, 0);
> + smp_call_function(disable_dawrs, NULL, 0);
>
> return rc;
> }
>
Christophe
^ permalink raw reply
* Re: [PATCH 07/15] powerpc/watchpoint: Get watchpoint count dynamically while disabling them
From: Christophe Leroy @ 2020-03-17 10:32 UTC (permalink / raw)
To: Ravi Bangoria, mpe, mikey
Cc: apopple, peterz, fweisbec, oleg, npiggin, linux-kernel, paulus,
jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200309085806.155823-8-ravi.bangoria@linux.ibm.com>
Le 09/03/2020 à 09:57, Ravi Bangoria a écrit :
> Instead of disabling only one watchpooint, get num of available
> watchpoints dynamically and disable all of them.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> arch/powerpc/include/asm/hw_breakpoint.h | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
> index 980ac7d9f267..ec61e2b7195c 100644
> --- a/arch/powerpc/include/asm/hw_breakpoint.h
> +++ b/arch/powerpc/include/asm/hw_breakpoint.h
> @@ -75,14 +75,13 @@ extern void ptrace_triggered(struct perf_event *bp,
> struct perf_sample_data *data, struct pt_regs *regs);
> static inline void hw_breakpoint_disable(void)
> {
> - struct arch_hw_breakpoint brk;
> -
> - brk.address = 0;
> - brk.type = 0;
> - brk.len = 0;
> - brk.hw_len = 0;
> - if (ppc_breakpoint_available())
> - __set_breakpoint(&brk, 0);
> + int i;
> + struct arch_hw_breakpoint null_brk = {0};
> +
> + if (ppc_breakpoint_available()) {
I think this test should go into nr_wp_slots() which should return zero
when no breakpoint is available. This would simplify at least here and
patch 4
Christophe
> + for (i = 0; i < nr_wp_slots(); i++)
> + __set_breakpoint(&null_brk, i);
> + }
> }
> extern void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs);
> int hw_breakpoint_handler(struct die_args *args);
>
^ permalink raw reply
* Re: [PATCH 05/15] powerpc/watchpoint: Provide DAWR number to set_dawr
From: Christophe Leroy @ 2020-03-17 10:28 UTC (permalink / raw)
To: Ravi Bangoria, mpe, mikey
Cc: apopple, peterz, fweisbec, oleg, npiggin, linux-kernel, paulus,
jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200309085806.155823-6-ravi.bangoria@linux.ibm.com>
Le 09/03/2020 à 09:57, Ravi Bangoria a écrit :
> Introduce new parameter 'nr' to set_dawr() which indicates which DAWR
> should be programed.
While we are at it (In another patch I think), we should do the same to
set_dabr() so that we can use both DABR and DABR2
Christophe
^ permalink raw reply
* Re: [PATCH 03/15] powerpc/watchpoint: Introduce function to get nr watchpoints dynamically
From: Christophe Leroy @ 2020-03-17 10:21 UTC (permalink / raw)
To: Ravi Bangoria, mpe, mikey
Cc: apopple, peterz, fweisbec, oleg, npiggin, linux-kernel, paulus,
jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200309085806.155823-4-ravi.bangoria@linux.ibm.com>
Le 09/03/2020 à 09:57, Ravi Bangoria a écrit :
> So far we had only one watchpoint, so we have hardcoded HBP_NUM to 1.
> But future Power architecture is introducing 2nd DAWR and thus kernel
> should be able to dynamically find actual number of watchpoints
> supported by hw it's running on. Introduce function for the same.
> Also convert HBP_NUM macro to HBP_NUM_MAX, which will now represent
> maximum number of watchpoints supported by Powerpc.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> arch/powerpc/include/asm/cputable.h | 6 +++++-
> arch/powerpc/include/asm/hw_breakpoint.h | 2 ++
> arch/powerpc/include/asm/processor.h | 2 +-
> arch/powerpc/kernel/hw_breakpoint.c | 2 +-
> arch/powerpc/kernel/process.c | 6 ++++++
> 5 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
> index 40a4d3c6fd99..c67b94f3334c 100644
> --- a/arch/powerpc/include/asm/cputable.h
> +++ b/arch/powerpc/include/asm/cputable.h
> @@ -614,7 +614,11 @@ enum {
> };
> #endif /* __powerpc64__ */
>
> -#define HBP_NUM 1
> +/*
> + * Maximum number of hw breakpoint supported on powerpc. Number of
> + * breakpoints supported by actual hw might be less than this.
> + */
> +#define HBP_NUM_MAX 1
>
> #endif /* !__ASSEMBLY__ */
>
> diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
> index f2f8d8aa8e3b..741c4f7573c4 100644
> --- a/arch/powerpc/include/asm/hw_breakpoint.h
> +++ b/arch/powerpc/include/asm/hw_breakpoint.h
> @@ -43,6 +43,8 @@ struct arch_hw_breakpoint {
> #define DABR_MAX_LEN 8
> #define DAWR_MAX_LEN 512
>
> +extern int nr_wp_slots(void);
'extern' keyword is unneeded and irrelevant here. Please remove it. Even
checkpatch is unhappy
(https://openpower.xyz/job/snowpatch/job/snowpatch-linux-checkpatch/12172//artifact/linux/checkpatch.log)
> +
> #ifdef CONFIG_HAVE_HW_BREAKPOINT
> #include <linux/kdebug.h>
> #include <asm/reg.h>
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index 8387698bd5b6..666b2825278c 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -176,7 +176,7 @@ struct thread_struct {
> int fpexc_mode; /* floating-point exception mode */
> unsigned int align_ctl; /* alignment handling control */
> #ifdef CONFIG_HAVE_HW_BREAKPOINT
> - struct perf_event *ptrace_bps[HBP_NUM];
> + struct perf_event *ptrace_bps[HBP_NUM_MAX];
> /*
> * Helps identify source of single-step exception and subsequent
> * hw-breakpoint enablement
> diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
> index d0854320bb50..e68798cee3fa 100644
> --- a/arch/powerpc/kernel/hw_breakpoint.c
> +++ b/arch/powerpc/kernel/hw_breakpoint.c
> @@ -38,7 +38,7 @@ static DEFINE_PER_CPU(struct perf_event *, bp_per_reg);
> int hw_breakpoint_slots(int type)
> {
> if (type == TYPE_DATA)
> - return HBP_NUM;
> + return nr_wp_slots();
> return 0; /* no instruction breakpoints available */
> }
>
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 110db94cdf3c..6d4b029532e2 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -835,6 +835,12 @@ static inline bool hw_brk_match(struct arch_hw_breakpoint *a,
> return true;
> }
>
> +/* Returns total number of data breakpoints available. */
> +int nr_wp_slots(void)
> +{
> + return HBP_NUM_MAX;
> +}
> +
This is not worth a global function. At least it should be a static
function located in hw_breakpoint.c. But it would be even better to have
it as a static inline in asm/hw_breakpoint.h
Christophe
^ permalink raw reply
* Re: [PATCH 02/15] powerpc/watchpoint: Add SPRN macros for second DAWR
From: Christophe Leroy @ 2020-03-17 10:16 UTC (permalink / raw)
To: Ravi Bangoria, mpe, mikey
Cc: apopple, peterz, fweisbec, oleg, npiggin, linux-kernel, paulus,
jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200309085806.155823-3-ravi.bangoria@linux.ibm.com>
Le 09/03/2020 à 09:57, Ravi Bangoria a écrit :
> Future Power architecture is introducing second DAWR. Add SPRN_ macros
> for the same.
I'm not sure this is called 'macros'. For me a macro is something more
complex.
For me those are 'constants'.
Christophe
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> arch/powerpc/include/asm/reg.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 156ee89fa9be..062e74cf41fd 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -284,6 +284,7 @@
> #define CTRL_TE 0x00c00000 /* thread enable */
> #define CTRL_RUNLATCH 0x1
> #define SPRN_DAWR0 0xB4
> +#define SPRN_DAWR1 0xB5
> #define SPRN_RPR 0xBA /* Relative Priority Register */
> #define SPRN_CIABR 0xBB
> #define CIABR_PRIV 0x3
> @@ -291,6 +292,7 @@
> #define CIABR_PRIV_SUPER 2
> #define CIABR_PRIV_HYPER 3
> #define SPRN_DAWRX0 0xBC
> +#define SPRN_DAWRX1 0xBD
> #define DAWRX_USER __MASK(0)
> #define DAWRX_KERNEL __MASK(1)
> #define DAWRX_HYP __MASK(2)
>
^ permalink raw reply
* Re: [PATCH 01/15] powerpc/watchpoint: Rename current DAWR macros
From: Christophe Leroy @ 2020-03-17 10:14 UTC (permalink / raw)
To: Ravi Bangoria, mpe, mikey
Cc: apopple, peterz, fweisbec, oleg, npiggin, linux-kernel, paulus,
jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200309085806.155823-2-ravi.bangoria@linux.ibm.com>
Le 09/03/2020 à 09:57, Ravi Bangoria a écrit :
> Future Power architecture is introducing second DAWR. Rename current
> DAWR macros as:
> s/SPRN_DAWR/SPRN_DAWR0/
> s/SPRN_DAWRX/SPRN_DAWRX0/
I think you should tell that DAWR0 and DAWRX0 is the real name of the
register as documented in (at least) power8 and power9 user manual.
Otherwise, we can't understand why you change the name of the register.
Christophe
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> arch/powerpc/include/asm/reg.h | 4 ++--
> arch/powerpc/kernel/dawr.c | 4 ++--
> arch/powerpc/kvm/book3s_hv.c | 12 ++++++------
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 18 +++++++++---------
> arch/powerpc/xmon/xmon.c | 2 +-
> 5 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index da5cab038e25..156ee89fa9be 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -283,14 +283,14 @@
> #define CTRL_CT1 0x40000000 /* thread 1 */
> #define CTRL_TE 0x00c00000 /* thread enable */
> #define CTRL_RUNLATCH 0x1
> -#define SPRN_DAWR 0xB4
> +#define SPRN_DAWR0 0xB4
> #define SPRN_RPR 0xBA /* Relative Priority Register */
> #define SPRN_CIABR 0xBB
> #define CIABR_PRIV 0x3
> #define CIABR_PRIV_USER 1
> #define CIABR_PRIV_SUPER 2
> #define CIABR_PRIV_HYPER 3
> -#define SPRN_DAWRX 0xBC
> +#define SPRN_DAWRX0 0xBC
> #define DAWRX_USER __MASK(0)
> #define DAWRX_KERNEL __MASK(1)
> #define DAWRX_HYP __MASK(2)
> diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
> index cc14aa6c4a1b..e91b613bf137 100644
> --- a/arch/powerpc/kernel/dawr.c
> +++ b/arch/powerpc/kernel/dawr.c
> @@ -39,8 +39,8 @@ int set_dawr(struct arch_hw_breakpoint *brk)
> if (ppc_md.set_dawr)
> return ppc_md.set_dawr(dawr, dawrx);
>
> - mtspr(SPRN_DAWR, dawr);
> - mtspr(SPRN_DAWRX, dawrx);
> + mtspr(SPRN_DAWR0, dawr);
> + mtspr(SPRN_DAWRX0, dawrx);
>
> return 0;
> }
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 33be4d93248a..498c57e1f5c9 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -3383,8 +3383,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
> int trap;
> unsigned long host_hfscr = mfspr(SPRN_HFSCR);
> unsigned long host_ciabr = mfspr(SPRN_CIABR);
> - unsigned long host_dawr = mfspr(SPRN_DAWR);
> - unsigned long host_dawrx = mfspr(SPRN_DAWRX);
> + unsigned long host_dawr = mfspr(SPRN_DAWR0);
> + unsigned long host_dawrx = mfspr(SPRN_DAWRX0);
> unsigned long host_psscr = mfspr(SPRN_PSSCR);
> unsigned long host_pidr = mfspr(SPRN_PID);
>
> @@ -3413,8 +3413,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
> mtspr(SPRN_SPURR, vcpu->arch.spurr);
>
> if (dawr_enabled()) {
> - mtspr(SPRN_DAWR, vcpu->arch.dawr);
> - mtspr(SPRN_DAWRX, vcpu->arch.dawrx);
> + mtspr(SPRN_DAWR0, vcpu->arch.dawr);
> + mtspr(SPRN_DAWRX0, vcpu->arch.dawrx);
> }
> mtspr(SPRN_CIABR, vcpu->arch.ciabr);
> mtspr(SPRN_IC, vcpu->arch.ic);
> @@ -3466,8 +3466,8 @@ static int kvmhv_load_hv_regs_and_go(struct kvm_vcpu *vcpu, u64 time_limit,
> (local_paca->kvm_hstate.fake_suspend << PSSCR_FAKE_SUSPEND_LG));
> mtspr(SPRN_HFSCR, host_hfscr);
> mtspr(SPRN_CIABR, host_ciabr);
> - mtspr(SPRN_DAWR, host_dawr);
> - mtspr(SPRN_DAWRX, host_dawrx);
> + mtspr(SPRN_DAWR0, host_dawr);
> + mtspr(SPRN_DAWRX0, host_dawrx);
> mtspr(SPRN_PID, host_pidr);
>
> /*
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index dbc2fecc37f0..f4b412b7cad8 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -707,8 +707,8 @@ BEGIN_FTR_SECTION
> END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
> BEGIN_FTR_SECTION
> mfspr r5, SPRN_CIABR
> - mfspr r6, SPRN_DAWR
> - mfspr r7, SPRN_DAWRX
> + mfspr r6, SPRN_DAWR0
> + mfspr r7, SPRN_DAWRX0
> mfspr r8, SPRN_IAMR
> std r5, STACK_SLOT_CIABR(r1)
> std r6, STACK_SLOT_DAWR(r1)
> @@ -803,8 +803,8 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
> beq 1f
> ld r5, VCPU_DAWR(r4)
> ld r6, VCPU_DAWRX(r4)
> - mtspr SPRN_DAWR, r5
> - mtspr SPRN_DAWRX, r6
> + mtspr SPRN_DAWR0, r5
> + mtspr SPRN_DAWRX0, r6
> 1:
> ld r7, VCPU_CIABR(r4)
> ld r8, VCPU_TAR(r4)
> @@ -1772,8 +1772,8 @@ BEGIN_FTR_SECTION
> * If the DAWR doesn't work, it's ok to write these here as
> * this value should always be zero
> */
> - mtspr SPRN_DAWR, r6
> - mtspr SPRN_DAWRX, r7
> + mtspr SPRN_DAWR0, r6
> + mtspr SPRN_DAWRX0, r7
> END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> BEGIN_FTR_SECTION
> ld r5, STACK_SLOT_TID(r1)
> @@ -2583,8 +2583,8 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> mfmsr r6
> andi. r6, r6, MSR_DR /* in real mode? */
> bne 4f
> - mtspr SPRN_DAWR, r4
> - mtspr SPRN_DAWRX, r5
> + mtspr SPRN_DAWR0, r4
> + mtspr SPRN_DAWRX0, r5
> 4: li r3, 0
> blr
>
> @@ -3340,7 +3340,7 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
> mtspr SPRN_AMR, r0
> mtspr SPRN_IAMR, r0
> mtspr SPRN_CIABR, r0
> - mtspr SPRN_DAWRX, r0
> + mtspr SPRN_DAWRX0, r0
>
> BEGIN_MMU_FTR_SECTION
> b 4f
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index e8c84d265602..6c4a8f8c0bd8 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -1938,7 +1938,7 @@ static void dump_207_sprs(void)
> printf("hfscr = %.16lx dhdes = %.16lx rpr = %.16lx\n",
> mfspr(SPRN_HFSCR), mfspr(SPRN_DHDES), mfspr(SPRN_RPR));
> printf("dawr = %.16lx dawrx = %.16lx ciabr = %.16lx\n",
> - mfspr(SPRN_DAWR), mfspr(SPRN_DAWRX), mfspr(SPRN_CIABR));
> + mfspr(SPRN_DAWR0), mfspr(SPRN_DAWRX0), mfspr(SPRN_CIABR));
> #endif
> }
>
>
^ permalink raw reply
* Re: [PATCH] powerpc/pseries: Fix MCE handling on pseries
From: Nicholas Piggin @ 2020-03-17 10:01 UTC (permalink / raw)
To: Ganesh, linuxppc-dev, mpe; +Cc: mahesh
In-Reply-To: <d22f9ef9-07db-9615-6420-001b85dd2742@linux.ibm.com>
Ganesh's on March 16, 2020 9:47 pm:
>
>
> On 3/14/20 9:18 AM, Nicholas Piggin wrote:
>> Ganesh Goudar's on March 14, 2020 12:04 am:
>>> MCE handling on pSeries platform fails as recent rework to use common
>>> code for pSeries and PowerNV in machine check error handling tries to
>>> access per-cpu variables in realmode. The per-cpu variables may be
>>> outside the RMO region on pSeries platform and needs translation to be
>>> enabled for access. Just moving these per-cpu variable into RMO region
>>> did'nt help because we queue some work to workqueues in real mode, which
>>> again tries to touch per-cpu variables.
>> Which queues are these? We should not be using Linux workqueues, but the
>> powerpc mce code which uses irq_work.
>
> Yes, irq work queues accesses memory outside RMO.
> irq_work_queue()->__irq_work_queue_local()->[this_cpu_ptr(&lazy_list) | this_cpu_ptr(&raised_list)]
Hmm, okay.
>>> Also fwnmi_release_errinfo()
>>> cannot be called when translation is not enabled.
>> Why not?
>
> It crashes when we try to get RTAS token for "ibm, nmi-interlock" device
> tree node. But yes we can avoid it by storing it rtas_token somewhere but haven't
> tried it, here is the backtrace I got when fwnmi_release_errinfo() called from
> realmode handler.
Okay, I actually had problems with that messing up soft-irq state too
and so I sent a patch to get rid of it, but that's the least of your
problems really.
>>> This patch fixes this by enabling translation in the exception handler
>>> when all required real mode handling is done. This change only affects
>>> the pSeries platform.
>> Not supposed to do this, because we might not be in a state
>> where the MMU is ready to be turned on at this point.
>>
>> I'd like to understand better which accesses are a problem, and whether
>> we can fix them all to be in the RMO.
>
> I faced three such access problems,
> * accessing per-cpu data (like mce_event,mce_event_queue and mce_event_queue),
> we can move this inside RMO.
> * calling fwnmi_release_errinfo().
> * And queuing work to irq_work_queue, not sure how to fix this.
Yeah. The irq_work_queue one is the biggest problem.
This code "worked" prior to the series unifying pseries and powernv
machine check handlers, 9ca766f9891d ("powerpc/64s/pseries: machine
check convert to use common event code") and friends. But it does in
basically the same way as your fix (i.e., it runs this early handler
in virtual mode), but that's not really the right fix.
Consider: you get a SLB multi hit on a kernel address due to hardware or
software error. That access causes a MCE, but before the error can be
decode to save and flush the SLB, you turn on relocation and that
causes another SLB multi hit...
I think the irq_work subsystem will have to be changed to use an array
unfortunately.
Thanks,
Nick
^ permalink raw reply
* Slub: Increased mem consumption on cpu,mem-less node powerpc guest
From: Bharata B Rao @ 2020-03-17 9:26 UTC (permalink / raw)
To: linux-mm
Cc: Andrew Morton, aneesh.kumar, Pekka Enberg, linuxppc-dev,
David Rientjes, Christoph Lameter, Joonsoo Kim
Hi,
We are seeing an increased slab memory consumption on PowerPC guest
LPAR (on PowerVM) having an uncommon topology where one NUMA node has no
CPUs or any memory and the other node has all the CPUs and memory. Though
QEMU prevents such topologies for KVM guest, I hacked QEMU to allow such
topology to get some slab numbers. Here is the comparision of such
a KVM guest with a single node KVM guest with equal amount of CPUs and memory.
Case 1: 2 node NUMA, node0 empty
================================
# numactl -H
available: 2 nodes (0-1)
node 0 cpus:
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus: 0 1 2 3 4 5 6 7
node 1 size: 16294 MB
node 1 free: 15453 MB
node distances:
node 0 1
0: 10 40
1: 40 10
Case 2: Single node
===================
# numactl -H
available: 1 nodes (0)
node 0 cpus: 0 1 2 3 4 5 6 7
node 0 size: 16294 MB
node 0 free: 15675 MB
node distances:
node 0
0: 10
Here is how the total slab memory consumptions compare right after boot:
# grep -i slab /proc/meminfo
Case 1: 442560 kB
Case 2: 195904 kB
Closer look at the individual slabs suggests that most of the increased
slab consumption in Case 1 can be attributed to kmalloc-N slabs. In
particular the following two caches account for most of the increase.
Case 1:
# ./slabinfo -S | grep -e kmalloc-1k -e kmalloc-512
kmalloc-1k 2869 1024 101.5M 1549/1540/0 32 0 99 2 U
kmalloc-512 3302 512 100.2M 1530/1522/0 64 0 99 1 U
Case 2:
# ./slabinfo -S | grep -e kmalloc-1k -e kmalloc-512
kmalloc-1k 2811 1024 6.1M 94/29/0 32 0 30 46 U
kmalloc-512 3207 512 3.5M 54/13/0 64 0 24 46 U
Here is the list of slub stats that significantly differ between two cases:
Case 1:
------
alloc_from_partial 6333 C0=1506 C1=525 C2=774 C3=478 C4=413 C5=1036 C6=698 C7=903
alloc_slab 3350 C0=757 C1=336 C2=120 C3=72 C4=120 C5=912 C6=600 C7=433
alloc_slowpath 9792 C0=2329 C1=861 C2=916 C3=571 C4=533 C5=1948 C6=1298 C7=1336
cmpxchg_double_fail 31 C1=3 C2=2 C3=7 C4=3 C5=4 C6=2 C7=10
deactivate_full 38 C0=14 C1=2 C2=13 C5=3 C6=2 C7=4
deactivate_remote_frees 1 C7=1
deactivate_to_head 10092 C0=2654 C1=859 C2=903 C3=571 C4=533 C5=1945 C6=1296 C7=1331
deactivate_to_tail 1 C7=1
free_add_partial 29 C0=7 C2=1 C3=5 C4=3 C5=6 C6=2 C7=5
free_frozen 32 C0=4 C1=3 C2=4 C3=3 C4=7 C5=3 C6=7 C7=1
free_remove_partial 1799 C0=408 C1=47 C2=54 C4=231 C5=752 C6=110 C7=197
free_slab 1799 C0=408 C1=47 C2=54 C4=231 C5=752 C6=110 C7=197
free_slowpath 7415 C0=2014 C1=486 C2=433 C3=525 C4=814 C5=1707 C6=586 C7=850
objects 2875 N1=2875
objects_partial 2587 N1=2587
partial 1542 N1=1542
slabs 1551 N1=1551
total_objects 49632 N1=49632
# cat alloc_calls (truncated)
1952 alloc_fair_sched_group+0x114/0x240 age=147813/152837/153714 pid=1-1074 cpus=0-2,5-7 nodes=1
# cat free_calls (truncated)
2671 <not-available> age=4295094831 pid=0 cpus=0 nodes=1
2 free_fair_sched_group+0xa0/0x120 age=156576/156850/157125 pid=0 cpus=0,5 nodes=1
Case 1:
------
alloc_from_partial 9231 C0=435 C1=2349 C2=2386 C3=1807 C4=882 C5=367 C6=559 C7=446
alloc_slab 114 C0=12 C1=41 C2=28 C3=15 C4=9 C5=1 C6=1 C7=7
alloc_slowpath 9415 C0=448 C1=2390 C2=2414 C3=1891 C4=891 C5=368 C6=560 C7=453
cmpxchg_double_fail 22 C0=1 C1=1 C3=3 C4=8 C5=1 C6=5 C7=3
deactivate_full 512 C0=13 C1=143 C2=147 C3=147 C4=22 C5=10 C6=6 C7=24
deactivate_remote_frees 1 C4=1
deactivate_to_head 9099 C0=437 C1=2247 C2=2267 C3=1937 C4=870 C5=358 C6=554 C7=429
deactivate_to_tail 1 C4=1
free_add_partial 447 C0=21 C1=140 C2=164 C3=60 C4=22 C5=16 C6=14 C7=10
free_frozen 22 C0=3 C2=3 C3=2 C4=1 C5=6 C6=6 C7=1
free_remove_partial 20 C1=5 C2=5 C4=3 C6=7
free_slab 20 C1=5 C2=5 C4=3 C6=7
free_slowpath 6953 C0=194 C1=2123 C2=1729 C3=850 C4=466 C5=725 C6=520 C7=346
objects 2812 N0=2812
objects_partial 733 N0=733
partial 29 N0=29
slabs 94 N0=94
total_objects 3008 N0=3008
# cat alloc_calls (truncated)
1952 alloc_fair_sched_group+0x114/0x240 age=43957/46225/46802 pid=1-1059 cpus=1-5,7
# cat free_calls (truncated)
1516 <not-available> age=4294987281 pid=0 cpus=0
647 free_fair_sched_group+0xa0/0x120 age=48798/49142/49628 pid=0-954 cpus=1-2
We see a significant difference in the number of partial slabs and
the resulting total_objects between the two cases. I was trying to
see if this has got to do anything with the way the node value is
arrived at in difference slub routines. Haven't yet understood slub
code to say anything conclusively, but the following hack in the slub
code completely reduces the increased slab consumption for Case1 and
makes it very similar to Case2
diff --git a/mm/slub.c b/mm/slub.c
index 17dc00e33115..888e4d245444 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1971,10 +1971,8 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
void *object;
int searchnode = node;
- if (node == NUMA_NO_NODE)
+ if (node == NUMA_NO_NODE || !node_present_pages(node))
searchnode = numa_mem_id();
- else if (!node_present_pages(node))
- searchnode = node_to_mem_node(node);
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
Regards,
Bharata.
^ permalink raw reply related
* [PATCH 7/7] powerpc/pseries/ras: fwnmi sreset should not interlock
From: Nicholas Piggin @ 2020-03-17 9:09 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200317090913.343097-1-npiggin@gmail.com>
PAPR does not specify that fwnmi sreset should be interlocked, and
PowerVM (and therefore now QEMU) do not require it.
These "ibm,nmi-interlock" calls are ignored by firmware, but there
is a possibility that the sreset could have interrupted a machine
check and release the machine check's interlock too early, corrupting
it if another machine check came in.
This is an extremely rare case, but it should be fixed for clarity
and reducing the code executed in the sreset path. Firmware also
does not provide error information for the sreset case to look at, so
remove that comment.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/platforms/pseries/ras.c | 48 ++++++++++++++++++++--------
1 file changed, 34 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index a40598e6e525..833ae34b7fec 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -406,6 +406,20 @@ static inline struct rtas_error_log *fwnmi_get_errlog(void)
return (struct rtas_error_log *)local_paca->mce_data_buf;
}
+static unsigned long *fwnmi_get_savep(struct pt_regs *regs)
+{
+ unsigned long savep_ra;
+
+ /* Mask top two bits */
+ savep_ra = regs->gpr[3] & ~(0x3UL << 62);
+ if (!VALID_FWNMI_BUFFER(savep_ra)) {
+ printk(KERN_ERR "FWNMI: corrupt r3 0x%016lx\n", regs->gpr[3]);
+ return NULL;
+ }
+
+ return __va(savep_ra);
+}
+
/*
* Get the error information for errors coming through the
* FWNMI vectors. The pt_regs' r3 will be updated to reflect
@@ -423,20 +437,15 @@ static inline struct rtas_error_log *fwnmi_get_errlog(void)
*/
static struct rtas_error_log *fwnmi_get_errinfo(struct pt_regs *regs)
{
- unsigned long savep_ra;
unsigned long *savep;
struct rtas_error_log *h;
- /* Mask top two bits */
- savep_ra = regs->gpr[3] & ~(0x3UL << 62);
-
- if (!VALID_FWNMI_BUFFER(savep_ra)) {
- printk(KERN_ERR "FWNMI: corrupt r3 0x%016lx\n", regs->gpr[3]);
+ savep = fwnmi_get_savep(regs);
+ if (!savep)
return NULL;
- }
- savep = __va(savep_ra);
- regs->gpr[3] = be64_to_cpu(savep[0]); /* restore original r3 */
+ /* restore original r3 */
+ regs->gpr[3] = be64_to_cpu(savep[0]);
h = (struct rtas_error_log *)&savep[1];
/* Use the per cpu buffer from paca to store rtas error log */
@@ -483,11 +492,22 @@ int pSeries_system_reset_exception(struct pt_regs *regs)
#endif
if (fwnmi_active) {
- struct rtas_error_log *errhdr = fwnmi_get_errinfo(regs);
- if (errhdr) {
- /* XXX Should look at FWNMI information */
- }
- fwnmi_release_errinfo();
+ unsigned long *savep;
+
+ /*
+ * Firmware (PowerVM and KVM) saves r3 to a save area like
+ * machine check, which is not exactly what PAPR (2.9)
+ * suggests but there is no way to detect otherwise, so this
+ * is the interface now.
+ *
+ * System resets do not save any error log or require an
+ * "ibm,nmi-interlock" rtas call to release.
+ */
+
+ savep = fwnmi_get_savep(regs);
+ /* restore original r3 */
+ if (savep)
+ regs->gpr[3] = be64_to_cpu(savep[0]);
}
if (smp_handle_nmi_ipi(regs))
--
2.23.0
^ permalink raw reply related
* [PATCH 6/7] powerpc/pseries/ras: fwnmi avoid modifying r3 in error case
From: Nicholas Piggin @ 2020-03-17 9:09 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200317090913.343097-1-npiggin@gmail.com>
If there is some error with the fwnmi save area, r3 has already been
modified which doesn't help with debugging.
Only update r3 when to restore the saved value.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/platforms/pseries/ras.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 9a37bda47468..a40598e6e525 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -423,18 +423,19 @@ static inline struct rtas_error_log *fwnmi_get_errlog(void)
*/
static struct rtas_error_log *fwnmi_get_errinfo(struct pt_regs *regs)
{
+ unsigned long savep_ra;
unsigned long *savep;
struct rtas_error_log *h;
/* Mask top two bits */
- regs->gpr[3] &= ~(0x3UL << 62);
+ savep_ra = regs->gpr[3] & ~(0x3UL << 62);
- if (!VALID_FWNMI_BUFFER(regs->gpr[3])) {
+ if (!VALID_FWNMI_BUFFER(savep_ra)) {
printk(KERN_ERR "FWNMI: corrupt r3 0x%016lx\n", regs->gpr[3]);
return NULL;
}
- savep = __va(regs->gpr[3]);
+ savep = __va(savep_ra);
regs->gpr[3] = be64_to_cpu(savep[0]); /* restore original r3 */
h = (struct rtas_error_log *)&savep[1];
--
2.23.0
^ permalink raw reply related
* [PATCH 5/7] powerpc/pseries/ras: FWNMI_VALID off by one
From: Nicholas Piggin @ 2020-03-17 9:09 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200317090913.343097-1-npiggin@gmail.com>
This was discovered developing qemu fwnmi sreset support. This
off-by-one bug means the last 16 bytes of the rtas area can not
be used for a 16 byte save area.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/platforms/pseries/ras.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index c74d5e740922..9a37bda47468 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -395,10 +395,11 @@ static irqreturn_t ras_error_interrupt(int irq, void *dev_id)
/*
* Some versions of FWNMI place the buffer inside the 4kB page starting at
* 0x7000. Other versions place it inside the rtas buffer. We check both.
+ * Minimum size of the buffer is 16 bytes.
*/
#define VALID_FWNMI_BUFFER(A) \
- ((((A) >= 0x7000) && ((A) < 0x7ff0)) || \
- (((A) >= rtas.base) && ((A) < (rtas.base + rtas.size - 16))))
+ ((((A) >= 0x7000) && ((A) <= 0x8000 - 16)) || \
+ (((A) >= rtas.base) && ((A) <= (rtas.base + rtas.size - 16))))
static inline struct rtas_error_log *fwnmi_get_errlog(void)
{
--
2.23.0
^ permalink raw reply related
* [PATCH 4/7] powerpc/64s: machine check reconcile irq state
From: Nicholas Piggin @ 2020-03-17 9:09 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200317090913.343097-1-npiggin@gmail.com>
pseries fwnmi machine check code pops the soft-irq checks in rtas_call
(after the previous patch to remove rtas_token from this call path).
Rather than play whack a mole with these and forever having fragile
code, it seems better to have the early machine check handler perform
the same kind of reconcile as the other NMI interrupts.
WARNING: CPU: 0 PID: 493 at arch/powerpc/kernel/irq.c:343
CPU: 0 PID: 493 Comm: a Tainted: G W
NIP: c00000000001ed2c LR: c000000000042c40 CTR: 0000000000000000
REGS: c0000001fffd38b0 TRAP: 0700 Tainted: G W
MSR: 8000000000021003 <SF,ME,RI,LE> CR: 28000488 XER: 00000000
CFAR: c00000000001ec90 IRQMASK: 0
GPR00: c000000000043820 c0000001fffd3b40 c0000000012ba300 0000000000000000
GPR04: 0000000048000488 0000000000000000 0000000000000000 00000000deadbeef
GPR08: 0000000000000080 0000000000000000 0000000000000000 0000000000001001
GPR12: 0000000000000000 c0000000014a0000 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR24: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR28: 0000000000000000 0000000000000001 c000000001360810 0000000000000000
NIP [c00000000001ed2c] arch_local_irq_restore.part.0+0xac/0x100
LR [c000000000042c40] unlock_rtas+0x30/0x90
Call Trace:
[c0000001fffd3b40] [c000000001360810] 0xc000000001360810 (unreliable)
[c0000001fffd3b60] [c000000000043820] rtas_call+0x1c0/0x280
[c0000001fffd3bb0] [c0000000000dc328] fwnmi_release_errinfo+0x38/0x70
[c0000001fffd3c10] [c0000000000dcd8c] pseries_machine_check_realmode+0x1dc/0x540
[c0000001fffd3cd0] [c00000000003fe04] machine_check_early+0x54/0x70
[c0000001fffd3d00] [c000000000008384] machine_check_early_common+0x134/0x1f0
--- interrupt: 200 at 0x13f1307c8
LR = 0x7fff888b8528
Instruction dump:
60000000 7d2000a6 71298000 41820068 39200002 7d210164 4bffff9c 60000000
60000000 7d2000a6 71298000 4c820020 <0fe00000> 4e800020 60000000 60000000
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/exceptions-64s.S | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index d95c4560c038..31bdbb94e477 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1186,11 +1186,30 @@ END_FTR_SECTION_IFSET(CPU_FTR_HVMODE)
li r10,MSR_RI
mtmsrd r10,1
+ /*
+ * Set IRQS_ALL_DISABLED and save PACAIRQHAPPENED (see
+ * system_reset_common)
+ */
+ li r10,IRQS_ALL_DISABLED
+ stb r10,PACAIRQSOFTMASK(r13)
+ lbz r10,PACAIRQHAPPENED(r13)
+ std r10,RESULT(r1)
+ ori r10,r10,PACA_IRQ_HARD_DIS
+ stb r10,PACAIRQHAPPENED(r13)
+
addi r3,r1,STACK_FRAME_OVERHEAD
bl machine_check_early
std r3,RESULT(r1) /* Save result */
ld r12,_MSR(r1)
+ /*
+ * Restore soft mask settings.
+ */
+ ld r10,RESULT(r1)
+ stb r10,PACAIRQHAPPENED(r13)
+ ld r10,SOFTE(r1)
+ stb r10,PACAIRQSOFTMASK(r13)
+
#ifdef CONFIG_PPC_P7_NAP
/*
* Check if thread was in power saving mode. We come here when any
--
2.23.0
^ permalink raw reply related
* [PATCH 3/7] powerpc/64s: Change irq reconcile for NMIs from reusing _DAR to RESULT
From: Nicholas Piggin @ 2020-03-17 9:09 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200317090913.343097-1-npiggin@gmail.com>
A spare interrupt stack slot is needed to save irq state when
reconciling NMIs (sreset and decrementer soft-nmi). _DAR is used
for this, but we want to reconcile machine checks as well, which
do use _DAR. Switch to using RESULT instead, as it's used by
system calls.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/exceptions-64s.S | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 6a936c9199d6..d95c4560c038 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1011,13 +1011,13 @@ EXC_COMMON_BEGIN(system_reset_common)
* the right thing. We do not want to reconcile because that goes
* through irq tracing which we don't want in NMI.
*
- * Save PACAIRQHAPPENED to _DAR (otherwise unused), and set HARD_DIS
+ * Save PACAIRQHAPPENED to RESULT (otherwise unused), and set HARD_DIS
* as we are running with MSR[EE]=0.
*/
li r10,IRQS_ALL_DISABLED
stb r10,PACAIRQSOFTMASK(r13)
lbz r10,PACAIRQHAPPENED(r13)
- std r10,_DAR(r1)
+ std r10,RESULT(r1)
ori r10,r10,PACA_IRQ_HARD_DIS
stb r10,PACAIRQHAPPENED(r13)
@@ -1038,7 +1038,7 @@ EXC_COMMON_BEGIN(system_reset_common)
/*
* Restore soft mask settings.
*/
- ld r10,_DAR(r1)
+ ld r10,RESULT(r1)
stb r10,PACAIRQHAPPENED(r13)
ld r10,SOFTE(r1)
stb r10,PACAIRQSOFTMASK(r13)
@@ -2805,7 +2805,7 @@ EXC_COMMON_BEGIN(soft_nmi_common)
li r10,IRQS_ALL_DISABLED
stb r10,PACAIRQSOFTMASK(r13)
lbz r10,PACAIRQHAPPENED(r13)
- std r10,_DAR(r1)
+ std r10,RESULT(r1)
ori r10,r10,PACA_IRQ_HARD_DIS
stb r10,PACAIRQHAPPENED(r13)
@@ -2819,7 +2819,7 @@ EXC_COMMON_BEGIN(soft_nmi_common)
/*
* Restore soft mask settings.
*/
- ld r10,_DAR(r1)
+ ld r10,RESULT(r1)
stb r10,PACAIRQHAPPENED(r13)
ld r10,SOFTE(r1)
stb r10,PACAIRQSOFTMASK(r13)
--
2.23.0
^ permalink raw reply related
* [PATCH 2/7] powerpc/pseries/ras: avoid calling rtas_token in NMI paths
From: Nicholas Piggin @ 2020-03-17 9:09 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200317090913.343097-1-npiggin@gmail.com>
In the interest of reducing code and possible failures in the
machine check and system reset paths, grab the "ibm,nmi-interlock"
token at init time.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/firmware.h | 1 +
arch/powerpc/platforms/pseries/ras.c | 2 +-
arch/powerpc/platforms/pseries/setup.c | 13 ++++++++++---
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
index ca33f4ef6cb4..6003c2e533a0 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -128,6 +128,7 @@ extern void machine_check_fwnmi(void);
/* This is true if we are using the firmware NMI handler (typically LPAR) */
extern int fwnmi_active;
+extern int ibm_nmi_interlock_token;
extern unsigned int __start___fw_ftr_fixup, __stop___fw_ftr_fixup;
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 1d7f973c647b..c74d5e740922 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -458,7 +458,7 @@ static struct rtas_error_log *fwnmi_get_errinfo(struct pt_regs *regs)
*/
static void fwnmi_release_errinfo(void)
{
- int ret = rtas_call(rtas_token("ibm,nmi-interlock"), 0, 1, NULL);
+ int ret = rtas_call(ibm_nmi_interlock_token, 0, 1, NULL);
if (ret != 0)
printk(KERN_ERR "FWNMI: nmi-interlock failed: %d\n", ret);
}
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 17d17f064a2d..c31acd7ce0c0 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -83,6 +83,7 @@ unsigned long CMO_PageSize = (ASM_CONST(1) << IOMMU_PAGE_SHIFT_4K);
EXPORT_SYMBOL(CMO_PageSize);
int fwnmi_active; /* TRUE if an FWNMI handler is present */
+int ibm_nmi_interlock_token;
static void pSeries_show_cpuinfo(struct seq_file *m)
{
@@ -113,9 +114,14 @@ static void __init fwnmi_init(void)
struct slb_entry *slb_ptr;
size_t size;
#endif
+ int ibm_nmi_register_token;
- int ibm_nmi_register = rtas_token("ibm,nmi-register");
- if (ibm_nmi_register == RTAS_UNKNOWN_SERVICE)
+ ibm_nmi_register_token = rtas_token("ibm,nmi-register");
+ if (ibm_nmi_register_token == RTAS_UNKNOWN_SERVICE)
+ return;
+
+ ibm_nmi_interlock_token = rtas_token("ibm,nmi-interlock");
+ if (WARN_ON(ibm_nmi_interlock_token == RTAS_UNKNOWN_SERVICE))
return;
/* If the kernel's not linked at zero we point the firmware at low
@@ -123,7 +129,8 @@ static void __init fwnmi_init(void)
system_reset_addr = __pa(system_reset_fwnmi) - PHYSICAL_START;
machine_check_addr = __pa(machine_check_fwnmi) - PHYSICAL_START;
- if (0 == rtas_call(ibm_nmi_register, 2, 1, NULL, system_reset_addr,
+ if (0 == rtas_call(ibm_nmi_register_token, 2, 1, NULL,
+ system_reset_addr,
machine_check_addr))
fwnmi_active = 1;
--
2.23.0
^ permalink raw reply related
* [PATCH 1/7] powerpc/64: mark emergency stacks valid to unwind
From: Nicholas Piggin @ 2020-03-17 9:09 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
In-Reply-To: <20200317090913.343097-1-npiggin@gmail.com>
Before:
WARNING: CPU: 0 PID: 494 at arch/powerpc/kernel/irq.c:343
CPU: 0 PID: 494 Comm: a Tainted: G W
NIP: c00000000001ed2c LR: c000000000d13190 CTR: c00000000003f910
REGS: c0000001fffd3870 TRAP: 0700 Tainted: G W
MSR: 8000000000021003 <SF,ME,RI,LE> CR: 28000488 XER: 00000000
CFAR: c00000000001ec90 IRQMASK: 0
GPR00: c000000000aeb12c c0000001fffd3b00 c0000000012ba300 0000000000000000
GPR04: 0000000000000000 0000000000000000 000000010bd207c8 6b00696e74657272
GPR08: 0000000000000000 0000000000000000 0000000000000000 efbeadde00000000
GPR12: 0000000000000000 c0000000014a0000 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR24: 0000000000000000 0000000000000000 0000000000000000 000000010bd207bc
GPR28: 0000000000000000 c00000000148a898 0000000000000000 c0000001ffff3f50
NIP [c00000000001ed2c] arch_local_irq_restore.part.0+0xac/0x100
LR [c000000000d13190] _raw_spin_unlock_irqrestore+0x50/0xc0
Call Trace:
Instruction dump:
60000000 7d2000a6 71298000 41820068 39200002 7d210164 4bffff9c 60000000
60000000 7d2000a6 71298000 4c820020 <0fe00000> 4e800020 60000000 60000000
After:
WARNING: CPU: 0 PID: 499 at arch/powerpc/kernel/irq.c:343
CPU: 0 PID: 499 Comm: a Not tainted
NIP: c00000000001ed2c LR: c000000000d13210 CTR: c00000000003f980
REGS: c0000001fffd3870 TRAP: 0700 Not tainted
MSR: 8000000000021003 <SF,ME,RI,LE> CR: 28000488 XER: 00000000
CFAR: c00000000001ec90 IRQMASK: 0
GPR00: c000000000aeb1ac c0000001fffd3b00 c0000000012ba300 0000000000000000
GPR04: 0000000000000000 0000000000000000 00000001347607c8 6b00696e74657272
GPR08: 0000000000000000 0000000000000000 0000000000000000 efbeadde00000000
GPR12: 0000000000000000 c0000000014a0000 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR24: 0000000000000000 0000000000000000 0000000000000000 00000001347607bc
GPR28: 0000000000000000 c00000000148a898 0000000000000000 c0000001ffff3f50
NIP [c00000000001ed2c] arch_local_irq_restore.part.0+0xac/0x100
LR [c000000000d13210] _raw_spin_unlock_irqrestore+0x50/0xc0
Call Trace:
[c0000001fffd3b20] [c000000000aeb1ac] of_find_property+0x6c/0x90
[c0000001fffd3b70] [c000000000aeb1f0] of_get_property+0x20/0x40
[c0000001fffd3b90] [c000000000042cdc] rtas_token+0x3c/0x70
[c0000001fffd3bb0] [c0000000000dc318] fwnmi_release_errinfo+0x28/0x70
[c0000001fffd3c10] [c0000000000dcd8c] pseries_machine_check_realmode+0x1dc/0x540
[c0000001fffd3cd0] [c00000000003fe04] machine_check_early+0x54/0x70
[c0000001fffd3d00] [c000000000008384] machine_check_early_common+0x134/0x1f0
--- interrupt: 200 at 0x1347607c8
LR = 0x7fffafbd8328
Instruction dump:
60000000 7d2000a6 71298000 41820068 39200002 7d210164 4bffff9c 60000000
60000000 7d2000a6 71298000 4c820020 <0fe00000> 4e800020 60000000 60000000
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/process.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 1dea4d280f6f..d27bf367e929 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1983,6 +1983,32 @@ static inline int valid_irq_stack(unsigned long sp, struct task_struct *p,
return 0;
}
+static inline int valid_emergency_stack(unsigned long sp, struct task_struct *p,
+ unsigned long nbytes)
+{
+#ifdef CONFIG_PPC64
+ unsigned long stack_page;
+ unsigned long cpu = task_cpu(p);
+
+ stack_page = (unsigned long)paca_ptrs[cpu]->emergency_sp - THREAD_SIZE;
+ if (sp >= stack_page && sp <= stack_page + THREAD_SIZE - nbytes)
+ return 1;
+
+# ifdef CONFIG_PPC_BOOK3S_64
+ stack_page = (unsigned long)paca_ptrs[cpu]->nmi_emergency_sp - THREAD_SIZE;
+ if (sp >= stack_page && sp <= stack_page + THREAD_SIZE - nbytes)
+ return 1;
+
+ stack_page = (unsigned long)paca_ptrs[cpu]->mc_emergency_sp - THREAD_SIZE;
+ if (sp >= stack_page && sp <= stack_page + THREAD_SIZE - nbytes)
+ return 1;
+# endif
+#endif
+
+ return 0;
+}
+
+
int validate_sp(unsigned long sp, struct task_struct *p,
unsigned long nbytes)
{
@@ -1994,7 +2020,10 @@ int validate_sp(unsigned long sp, struct task_struct *p,
if (sp >= stack_page && sp <= stack_page + THREAD_SIZE - nbytes)
return 1;
- return valid_irq_stack(sp, p, nbytes);
+ if (valid_irq_stack(sp, p, nbytes))
+ return 1;
+
+ return valid_emergency_stack(sp, p, nbytes);
}
EXPORT_SYMBOL(validate_sp);
--
2.23.0
^ permalink raw reply related
* [PATCH 0/7] powerpc/64: machine check and other RAS fixes
From: Nicholas Piggin @ 2020-03-17 9:09 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar, Nicholas Piggin
There's a bunch of problems we hit bringing up fwnmi sreset and
mces on QEMU, these apply to PowerVM as well, but I haven't done
much testing there and it's much harder.
This series of fixes applies on top of next-test, the machine
check reconcile patch won't apply cleanly to previous kernels but
it might want to be backported. We can do that after upstreaming.
This doesn't solve Ganesh's machine check RMO problem, but at
least the reconciling should help squash some warnings.
Thanks,
Nick
Nicholas Piggin (7):
powerpc/64: mark emergency stacks valid to unwind
powerpc/pseries/ras: avoid calling rtas_token in NMI paths
powerpc/64s: Change irq reconcile for NMIs from reusing _DAR to RESULT
powerpc/64s: machine check reconcile irq state
powerpc/pseries/ras: FWNMI_VALID off by one
powerpc/pseries/ras: fwnmi avoid modifying r3 in error case
powerpc/pseries/ras: fwnmi sreset should not interlock
arch/powerpc/include/asm/firmware.h | 1 +
arch/powerpc/kernel/exceptions-64s.S | 29 +++++++++++---
arch/powerpc/kernel/process.c | 31 ++++++++++++++-
arch/powerpc/platforms/pseries/ras.c | 54 ++++++++++++++++++--------
arch/powerpc/platforms/pseries/setup.c | 13 +++++--
5 files changed, 103 insertions(+), 25 deletions(-)
--
2.23.0
^ permalink raw reply
* Re: [PATCH] mm/hugetlb: Fix build failure with HUGETLB_PAGE but not HUGEBTLBFS
From: Christophe Leroy @ 2020-03-17 8:43 UTC (permalink / raw)
To: Baoquan He
Cc: Nick Piggin, Andi Kleen, linux-kernel, linux-mm, Adam Litke,
Nishanth Aravamudan, Andrew Morton, linuxppc-dev, Mike Kravetz
In-Reply-To: <20200317082550.GA3375@MiWiFi-R3L-srv>
Le 17/03/2020 à 09:25, Baoquan He a écrit :
> On 03/17/20 at 08:04am, Christophe Leroy wrote:
>> When CONFIG_HUGETLB_PAGE is set but not CONFIG_HUGETLBFS, the
>> following build failure is encoutered:
>
> From the definition of HUGETLB_PAGE, isn't it relying on HUGETLBFS?
> I could misunderstand the def_bool, please correct me if I am wrong.
AFAIU, it means that HUGETLBFS rely on HUGETLB_PAGE, by default
HUGETLB_PAGE is not selected when HUGETLBFS is not. But it is still
possible for an arch to select HUGETLB_PAGE without selecting HUGETLBFS
when it uses huge pages for other purpose than hugetlb file system.
Christophe
>
> config HUGETLB_PAGE
> def_bool HUGETLBFS
>
>>
>> In file included from arch/powerpc/mm/fault.c:33:0:
>> ./include/linux/hugetlb.h: In function 'hstate_inode':
>> ./include/linux/hugetlb.h:477:9: error: implicit declaration of function 'HUGETLBFS_SB' [-Werror=implicit-function-declaration]
>> return HUGETLBFS_SB(i->i_sb)->hstate;
>> ^
>> ./include/linux/hugetlb.h:477:30: error: invalid type argument of '->' (have 'int')
>> return HUGETLBFS_SB(i->i_sb)->hstate;
>> ^
>>
>> Gate hstate_inode() with CONFIG_HUGETLBFS instead of CONFIG_HUGETLB_PAGE.
>>
>> Reported-by: kbuild test robot <lkp@intel.com>
>> Link: https://patchwork.ozlabs.org/patch/1255548/#2386036
>> Fixes: a137e1cc6d6e ("hugetlbfs: per mount huge page sizes")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> ---
>> include/linux/hugetlb.h | 19 ++++++++-----------
>> 1 file changed, 8 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
>> index 1e897e4168ac..dafb3d70ff81 100644
>> --- a/include/linux/hugetlb.h
>> +++ b/include/linux/hugetlb.h
>> @@ -390,7 +390,10 @@ static inline bool is_file_hugepages(struct file *file)
>> return is_file_shm_hugepages(file);
>> }
>>
>> -
>> +static inline struct hstate *hstate_inode(struct inode *i)
>> +{
>> + return HUGETLBFS_SB(i->i_sb)->hstate;
>> +}
>> #else /* !CONFIG_HUGETLBFS */
>>
>> #define is_file_hugepages(file) false
>> @@ -402,6 +405,10 @@ hugetlb_file_setup(const char *name, size_t size, vm_flags_t acctflag,
>> return ERR_PTR(-ENOSYS);
>> }
>>
>> +static inline struct hstate *hstate_inode(struct inode *i)
>> +{
>> + return NULL;
>> +}
>> #endif /* !CONFIG_HUGETLBFS */
>>
>> #ifdef HAVE_ARCH_HUGETLB_UNMAPPED_AREA
>> @@ -472,11 +479,6 @@ extern unsigned int default_hstate_idx;
>>
>> #define default_hstate (hstates[default_hstate_idx])
>>
>> -static inline struct hstate *hstate_inode(struct inode *i)
>> -{
>> - return HUGETLBFS_SB(i->i_sb)->hstate;
>> -}
>> -
>> static inline struct hstate *hstate_file(struct file *f)
>> {
>> return hstate_inode(file_inode(f));
>> @@ -729,11 +731,6 @@ static inline struct hstate *hstate_vma(struct vm_area_struct *vma)
>> return NULL;
>> }
>>
>> -static inline struct hstate *hstate_inode(struct inode *i)
>> -{
>> - return NULL;
>> -}
>> -
>> static inline struct hstate *page_hstate(struct page *page)
>> {
>> return NULL;
>> --
>> 2.25.0
>>
>>
^ 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