* Re: [PATCH 07/11] treewide: Replace the use of mem_encrypt_active() with prot_guest_has()
From: Kuppuswamy, Sathyanarayanan @ 2021-08-10 18:45 UTC (permalink / raw)
To: Tom Lendacky, linux-kernel, x86, linuxppc-dev, linux-s390, iommu,
kvm, linux-efi, platform-driver-x86, linux-graphics-maintainer,
amd-gfx, dri-devel, kexec, linux-fsdevel
Cc: Andi Kleen, Tianyu Lan, Daniel Vetter, Baoquan He, Peter Zijlstra,
Thomas Zimmermann, Joerg Roedel, Dave Hansen, Maarten Lankhorst,
Maxime Ripard, David Airlie, Dave Young, Ingo Molnar,
Borislav Petkov, Andy Lutomirski, Brijesh Singh, Thomas Gleixner,
Will Deacon
In-Reply-To: <029791b24c6412f9427cfe6ec598156c64395964.1627424774.git.thomas.lendacky@amd.com>
On 7/27/21 3:26 PM, Tom Lendacky wrote:
> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> index de01903c3735..cafed6456d45 100644
> --- a/arch/x86/kernel/head64.c
> +++ b/arch/x86/kernel/head64.c
> @@ -19,7 +19,7 @@
> #include <linux/start_kernel.h>
> #include <linux/io.h>
> #include <linux/memblock.h>
> -#include <linux/mem_encrypt.h>
> +#include <linux/protected_guest.h>
> #include <linux/pgtable.h>
>
> #include <asm/processor.h>
> @@ -285,7 +285,7 @@ unsigned long __head __startup_64(unsigned long physaddr,
> * there is no need to zero it after changing the memory encryption
> * attribute.
> */
> - if (mem_encrypt_active()) {
> + if (prot_guest_has(PATTR_MEM_ENCRYPT)) {
> vaddr = (unsigned long)__start_bss_decrypted;
> vaddr_end = (unsigned long)__end_bss_decrypted;
Since this change is specific to AMD, can you replace PATTR_MEM_ENCRYPT with
prot_guest_has(PATTR_SME) || prot_guest_has(PATTR_SEV). It is not used in
TDX.
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply
* Re: clang/ld.lld build fails with `can't create dynamic relocation R_PPC64_ADDR64 against local symbol in readonly segment`
From: Paul Menzel @ 2021-08-10 18:38 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras
Cc: Derek Parker, Dmitrii Okunev, linuxppc-dev
In-Reply-To: <b647b710-7ae0-3c7e-6996-92ac974b3b2a@molgen.mpg.de>
Dear Linux folks,
Am 29.07.21 um 10:23 schrieb Paul Menzel:
> I just wanted to make you aware that building Linux for ppc64le with
> clang/lld.ld fails with [1]:
>
> ld.lld: error: can't create dynamic relocation R_PPC64_ADDR64
> against symbol: empty_zero_page in readonly segment; recompile object
> files with -fPIC or pass '-Wl,-z,notext' to allow text relocations in
> the output
> >>> defined in arch/powerpc/kernel/head_64.o
> >>> referenced by
> arch/powerpc/kernel/head_64.o:(___ksymtab+empty_zero_page+0x0)
>
> The patch below from one of the comments [2] fixes it.
>
> --- i/arch/powerpc/Makefile
> +++ w/arch/powerpc/Makefile
> @@ -122,7 +122,7 @@ cflags-$(CONFIG_STACKPROTECTOR) +=
> -mstack-protector-guard-reg=r2
> endif
>
> LDFLAGS_vmlinux-y := -Bstatic
> -LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie
> +LDFLAGS_vmlinux-$(CONFIG_RELOCATABLE) := -pie -z notext
> LDFLAGS_vmlinux := $(LDFLAGS_vmlinux-y)
> LDFLAGS_vmlinux += $(call ld-option,--orphan-handling=warn)
Any comments, if this is the right fix? Current Linux master branch
still fails to build with `LLVM=1` on Ubuntu 21.04 without this change.
Kind regards,
Paul
> [1]: https://github.com/ClangBuiltLinux/linux/issues/811
> [2]: https://github.com/ClangBuiltLinux/linux/issues/811#issuecomment-568316320
^ permalink raw reply
* [powerpc:next-test 60/60] arch/powerpc/kernel/misc_32.S:240: Error: unrecognized opcode: `emit_warn_entry'
From: kernel test robot @ 2021-08-10 18:10 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev, kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2343 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next-test
head: 5e3aa4531ecc7febbfa18218145c903dab17e651
commit: 5e3aa4531ecc7febbfa18218145c903dab17e651 [60/60] powerpc/bug: Provide better flexibility to WARN_ON/__WARN_FLAGS() with asm goto
config: powerpc64-buildonly-randconfig-r001-20210810 (attached as .config)
compiler: powerpc-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=5e3aa4531ecc7febbfa18218145c903dab17e651
git remote add powerpc https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git
git fetch --no-tags powerpc next-test
git checkout 5e3aa4531ecc7febbfa18218145c903dab17e651
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=powerpc SHELL=/bin/bash arch/powerpc/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
arch/powerpc/kernel/misc_32.S: Assembler messages:
>> arch/powerpc/kernel/misc_32.S:240: Error: unrecognized opcode: `emit_warn_entry'
vim +240 arch/powerpc/kernel/misc_32.S
218
219 /*
220 * Copy a whole page. We use the dcbz instruction on the destination
221 * to reduce memory traffic (it eliminates the unnecessary reads of
222 * the destination into cache). This requires that the destination
223 * is cacheable.
224 */
225 #define COPY_16_BYTES \
226 lwz r6,4(r4); \
227 lwz r7,8(r4); \
228 lwz r8,12(r4); \
229 lwzu r9,16(r4); \
230 stw r6,4(r3); \
231 stw r7,8(r3); \
232 stw r8,12(r3); \
233 stwu r9,16(r3)
234
235 _GLOBAL(copy_page)
236 rlwinm r5, r3, 0, L1_CACHE_BYTES - 1
237 addi r3,r3,-4
238
239 0: twnei r5, 0 /* WARN if r3 is not cache aligned */
> 240 EMIT_WARN_ENTRY 0b,__FILE__,__LINE__, BUGFLAG_WARNING
241
242 addi r4,r4,-4
243
244 li r5,4
245
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31951 bytes --]
^ permalink raw reply
* [PATCH] powerpc/interrupt: Do not call single_step_exception() from other exceptions
From: Christophe Leroy @ 2021-08-10 16:13 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
userm57, fthain
Cc: linuxppc-dev, linux-kernel
single_step_exception() is called by emulate_single_step() which
is called from (at least) alignment exception() handler and
program_check_exception() handler.
Redefine it as a regular __single_step_exception() which is called
by both single_step_exception() handler and emulate_single_step()
function.
Fixes: 3a96570ffceb ("powerpc: convert interrupt handlers to use wrappers")
Cc: stable@vger.kernel.org
Cc: Stan Johnson <userm57@yahoo.com>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Finn Thain <fthain@linux-m68k.org>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/traps.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index dfbce527c98e..d56254f05e17 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1104,7 +1104,7 @@ DEFINE_INTERRUPT_HANDLER(RunModeException)
_exception(SIGTRAP, regs, TRAP_UNK, 0);
}
-DEFINE_INTERRUPT_HANDLER(single_step_exception)
+static void __single_step_exception(struct pt_regs *regs)
{
clear_single_step(regs);
clear_br_trace(regs);
@@ -1121,6 +1121,11 @@ DEFINE_INTERRUPT_HANDLER(single_step_exception)
_exception(SIGTRAP, regs, TRAP_TRACE, regs->nip);
}
+DEFINE_INTERRUPT_HANDLER(single_step_exception)
+{
+ __single_step_exception(regs);
+}
+
/*
* After we have successfully emulated an instruction, we have to
* check if the instruction was being single-stepped, and if so,
@@ -1130,7 +1135,7 @@ DEFINE_INTERRUPT_HANDLER(single_step_exception)
static void emulate_single_step(struct pt_regs *regs)
{
if (single_stepping(regs))
- single_step_exception(regs);
+ __single_step_exception(regs);
}
static inline int __parse_fpscr(unsigned long fpscr)
--
2.25.0
^ permalink raw reply related
* [PATCH] powerpc/interrupt: Fix OOPS by not calling do_IRQ() from timer_interrupt()
From: Christophe Leroy @ 2021-08-10 16:13 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
userm57, fthain
Cc: linuxppc-dev, linux-kernel
An interrupt handler shall not be called from another interrupt
handler otherwise this leads to problems like the following:
Kernel attempted to write user page (afd4fa84) - exploit attempt? (uid: 1000)
------------[ cut here ]------------
Bug: Write fault blocked by KUAP!
WARNING: CPU: 0 PID: 1617 at arch/powerpc/mm/fault.c:230 do_page_fault+0x484/0x720
Modules linked in:
CPU: 0 PID: 1617 Comm: sshd Tainted: G W 5.13.0-pmac-00010-g8393422eb77 #7
NIP: c001b77c LR: c001b77c CTR: 00000000
REGS: cb9e5bc0 TRAP: 0700 Tainted: G W (5.13.0-pmac-00010-g8393422eb77)
MSR: 00021032 <ME,IR,DR,RI> CR: 24942424 XER: 00000000
GPR00: c001b77c cb9e5c80 c1582c00 00000021 3ffffbff 085b0000 00000027 c8eb644c
GPR08: 00000023 00000000 00000000 00000000 24942424 0063f8c8 00000000 000186a0
GPR16: afd52dd4 afd52dd0 afd52dcc afd52dc8 0065a990 c07640c4 cb9e5e98 cb9e5e90
GPR24: 00000040 afd4fa96 00000040 02000000 c1fda6c0 afd4fa84 00000300 cb9e5cc0
NIP [c001b77c] do_page_fault+0x484/0x720
LR [c001b77c] do_page_fault+0x484/0x720
Call Trace:
[cb9e5c80] [c001b77c] do_page_fault+0x484/0x720 (unreliable)
[cb9e5cb0] [c000424c] DataAccess_virt+0xd4/0xe4
--- interrupt: 300 at __copy_tofrom_user+0x110/0x20c
NIP: c001f9b4 LR: c03250a0 CTR: 00000004
REGS: cb9e5cc0 TRAP: 0300 Tainted: G W (5.13.0-pmac-00010-g8393422eb77)
MSR: 00009032 <EE,ME,IR,DR,RI> CR: 48028468 XER: 20000000
DAR: afd4fa84 DSISR: 0a000000
GPR00: 20726f6f cb9e5d80 c1582c00 00000004 cb9e5e3a 00000016 afd4fa80 00000000
GPR08: 3835202d 72777872 2d78722d 00000004 28028464 0063f8c8 00000000 000186a0
GPR16: afd52dd4 afd52dd0 afd52dcc afd52dc8 0065a990 c07640c4 cb9e5e98 cb9e5e90
GPR24: 00000040 afd4fa96 00000040 cb9e5e0c 00000daa a0000000 cb9e5e98 afd4fa56
NIP [c001f9b4] __copy_tofrom_user+0x110/0x20c
LR [c03250a0] _copy_to_iter+0x144/0x990
--- interrupt: 300
[cb9e5d80] [c03e89c0] n_tty_read+0xa4/0x598 (unreliable)
[cb9e5df0] [c03e2a0c] tty_read+0xdc/0x2b4
[cb9e5e80] [c0156bf8] vfs_read+0x274/0x340
[cb9e5f00] [c01571ac] ksys_read+0x70/0x118
[cb9e5f30] [c0016048] ret_from_syscall+0x0/0x28
--- interrupt: c00 at 0xa7855c88
NIP: a7855c88 LR: a7855c5c CTR: 00000000
REGS: cb9e5f40 TRAP: 0c00 Tainted: G W (5.13.0-pmac-00010-g8393422eb77)
MSR: 0000d032 <EE,PR,ME,IR,DR,RI> CR: 2402446c XER: 00000000
GPR00: 00000003 afd4ec70 a72137d0 0000000b afd4ecac 00004000 0065a990 00000800
GPR08: 00000000 a7947930 00000000 00000004 c15831b0 0063f8c8 00000000 000186a0
GPR16: afd52dd4 afd52dd0 afd52dcc afd52dc8 0065a990 0065a9e0 00000001 0065fac0
GPR24: 00000000 00000089 00664050 00000000 00668e30 a720c8dc a7943ff4 0065f9b0
NIP [a7855c88] 0xa7855c88
LR [a7855c5c] 0xa7855c5c
--- interrupt: c00
Instruction dump:
3884aa88 38630178 48076861 807f0080 48042e45 2f830000 419e0148 3c80c079
3c60c076 38841be4 386301c0 4801f705 <0fe00000> 3860000b 4bfffe30 3c80c06b
---[ end trace fd69b91a8046c2e5 ]---
Here the problem is that by re-enterring an exception handler,
kuap_save_and_lock() is called a second time with this time KUAP
access locked, leading to regs->kuap being overwritten hence
KUAP not being unlocked at exception exit as expected.
Do not call do_IRQ() from timer_interrupt() directly. Instead,
redefine do_IRQ() as a standard function named __do_IRQ(), and
call it from both do_IRQ() and time_interrupt() handlers.
Reported-by: Stan Johnson <userm57@yahoo.com>
Fixes: 3a96570ffceb ("powerpc: convert interrupt handlers to use wrappers")
Cc: stable@vger.kernel.org
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Finn Thain <fthain@linux-m68k.org>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/interrupt.h | 3 +++
arch/powerpc/include/asm/irq.h | 2 +-
arch/powerpc/kernel/irq.c | 7 ++++++-
arch/powerpc/kernel/time.c | 2 +-
4 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
index fc4702bdd119..1e984a35a39f 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -590,6 +590,9 @@ DECLARE_INTERRUPT_HANDLER_NMI(hmi_exception_realmode);
DECLARE_INTERRUPT_HANDLER_ASYNC(TAUException);
+/* irq.c */
+DECLARE_INTERRUPT_HANDLER_ASYNC(do_IRQ);
+
void __noreturn unrecoverable_exception(struct pt_regs *regs);
void replay_system_reset(void);
diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
index 4982f3711fc3..2b3278534bc1 100644
--- a/arch/powerpc/include/asm/irq.h
+++ b/arch/powerpc/include/asm/irq.h
@@ -52,7 +52,7 @@ extern void *mcheckirq_ctx[NR_CPUS];
extern void *hardirq_ctx[NR_CPUS];
extern void *softirq_ctx[NR_CPUS];
-extern void do_IRQ(struct pt_regs *regs);
+void __do_IRQ(struct pt_regs *regs);
extern void __init init_IRQ(void);
extern void __do_irq(struct pt_regs *regs);
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 91e63eac4e8f..551b653228c4 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -750,7 +750,7 @@ void __do_irq(struct pt_regs *regs)
trace_irq_exit(regs);
}
-DEFINE_INTERRUPT_HANDLER_ASYNC(do_IRQ)
+void __do_IRQ(struct pt_regs *regs)
{
struct pt_regs *old_regs = set_irq_regs(regs);
void *cursp, *irqsp, *sirqsp;
@@ -774,6 +774,11 @@ DEFINE_INTERRUPT_HANDLER_ASYNC(do_IRQ)
set_irq_regs(old_regs);
}
+DEFINE_INTERRUPT_HANDLER_ASYNC(do_IRQ)
+{
+ __do_IRQ(regs);
+}
+
static void *__init alloc_vm_stack(void)
{
return __vmalloc_node(THREAD_SIZE, THREAD_ALIGN, THREADINFO_GFP,
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index e45ce427bffb..c487ba5a6e11 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -586,7 +586,7 @@ DEFINE_INTERRUPT_HANDLER_ASYNC(timer_interrupt)
#if defined(CONFIG_PPC32) && defined(CONFIG_PPC_PMAC)
if (atomic_read(&ppc_n_lost_interrupts) != 0)
- do_IRQ(regs);
+ __do_IRQ(regs);
#endif
old_regs = set_irq_regs(regs);
--
2.25.0
^ permalink raw reply related
* [PATCH v4 6/6] sched/fair: Consider SMT in ASYM_PACKING load balance
From: Ricardo Neri @ 2021-08-10 14:41 UTC (permalink / raw)
To: Peter Zijlstra (Intel), Ingo Molnar, Juri Lelli, Vincent Guittot
Cc: Len Brown, Aubrey Li, Rafael J. Wysocki, Srikar Dronamraju,
Ravi V. Shankar, linuxppc-dev, Aubrey Li, Nicholas Piggin,
Ricardo Neri, Steven Rostedt, Quentin Perret, Ben Segall,
Mel Gorman, Daniel Bristot de Oliveira, Srinivas Pandruvada,
Joel Fernandes (Google), Tim Chen, Dietmar Eggemann, linux-kernel,
Ricardo Neri
In-Reply-To: <20210810144145.18776-1-ricardo.neri-calderon@linux.intel.com>
When deciding to pull tasks in ASYM_PACKING, it is necessary not only to
check for the idle state of the destination CPU, dst_cpu, but also of
its SMT siblings.
If dst_cpu is idle but its SMT siblings are busy, performance suffers
if it pulls tasks from a medium priority CPU that does not have SMT
siblings.
Implement asym_smt_can_pull_tasks() to inspect the state of the SMT
siblings of both dst_cpu and the CPUs in the candidate busiest group.
Cc: Aubrey Li <aubrey.li@intel.com>
Cc: Ben Segall <bsegall@google.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Quentin Perret <qperret@google.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Reviewed-by: Len Brown <len.brown@intel.com>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v3:
* Removed the arch_asym_check_smt_siblings() hook. Discussions with the
powerpc folks showed that this patch should not impact them. Also, more
recent powerpc processor no longer use asym_packing. (PeterZ)
* Removed unnecessary local variable in asym_can_pull_tasks(). (Dietmar)
* Removed unnecessary check for local CPUs when the local group has zero
utilization. (Joel)
* Renamed asym_can_pull_tasks() as asym_smt_can_pull_tasks() to reflect
the fact that it deals with SMT cases.
* Made asym_smt_can_pull_tasks() return false for !CONFIG_SCHED_SMT so
that callers can deal with non-SMT cases.
Changes since v2:
* Reworded the commit message to reflect updates in code.
* Corrected misrepresentation of dst_cpu as the CPU doing the load
balancing. (PeterZ)
* Removed call to arch_asym_check_smt_siblings() as it is now called in
sched_asym().
Changes since v1:
* Don't bailout in update_sd_pick_busiest() if dst_cpu cannot pull
tasks. Instead, reclassify the candidate busiest group, as it
may still be selected. (PeterZ)
* Avoid an expensive and unnecessary call to cpumask_weight() when
determining if a sched_group is comprised of SMT siblings.
(PeterZ).
---
kernel/sched/fair.c | 95 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 95 insertions(+)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index dd411cefb63f..8a1a2a43732c 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8531,10 +8531,99 @@ group_type group_classify(unsigned int imbalance_pct,
return group_has_spare;
}
+/**
+ * asym_smt_can_pull_tasks - Check whether the load balancing CPU can pull tasks
+ * @dst_cpu: Destination CPU of the load balancing
+ * @sds: Load-balancing data with statistics of the local group
+ * @sgs: Load-balancing statistics of the candidate busiest group
+ * @sg: The candidate busiet group
+ *
+ * Check the state of the SMT siblings of both @sds::local and @sg and decide
+ * if @dst_cpu can pull tasks. If @dst_cpu does not have SMT siblings, it can
+ * pull tasks if two or more of the SMT siblings of @sg are busy. If only one
+ * CPU in @sg is busy, pull tasks only if @dst_cpu has higher priority.
+ *
+ * If both @dst_cpu and @sg have SMT siblings, even the number of idle CPUs
+ * between @sds::local and @sg. Thus, pull tasks from @sg if the difference
+ * between the number of busy CPUs is 2 or more. If the difference is of 1,
+ * only pull if @dst_cpu has higher priority. If @sg does not have SMT siblings
+ * only pull tasks if all of the SMT siblings of @dst_cpu are idle and @sg
+ * has lower priority.
+ */
+static bool asym_smt_can_pull_tasks(int dst_cpu, struct sd_lb_stats *sds,
+ struct sg_lb_stats *sgs,
+ struct sched_group *sg)
+{
+#ifdef CONFIG_SCHED_SMT
+ bool local_is_smt, sg_is_smt;
+ int sg_busy_cpus;
+
+ local_is_smt = sds->local->flags & SD_SHARE_CPUCAPACITY;
+ sg_is_smt = sg->flags & SD_SHARE_CPUCAPACITY;
+
+ sg_busy_cpus = sgs->group_weight - sgs->idle_cpus;
+
+ if (!local_is_smt) {
+ /*
+ * If we are here, @dst_cpu is idle and does not have SMT
+ * siblings. Pull tasks if candidate group has two or more
+ * busy CPUs.
+ */
+ if (sg_is_smt && sg_busy_cpus >= 2)
+ return true;
+
+ /*
+ * @dst_cpu does not have SMT siblings. @sg may have SMT
+ * siblings and only one is busy. In such case, @dst_cpu
+ * can help if it has higher priority and is idle.
+ */
+ return !sds->local_stat.group_util &&
+ sched_asym_prefer(dst_cpu, sg->asym_prefer_cpu);
+ }
+
+ /* @dst_cpu has SMT siblings. */
+
+ if (sg_is_smt) {
+ int local_busy_cpus = sds->local->group_weight -
+ sds->local_stat.idle_cpus;
+ int busy_cpus_delta = sg_busy_cpus - local_busy_cpus;
+
+ /* Local can always help to even the number busy CPUs. */
+ if (busy_cpus_delta >= 2)
+ return true;
+
+ if (busy_cpus_delta == 1)
+ return sched_asym_prefer(dst_cpu,
+ sg->asym_prefer_cpu);
+
+ return false;
+ }
+
+ /*
+ * @sg does not have SMT siblings. Ensure that @sds::local does not end
+ * up with more than one busy SMT sibling and only pull tasks if there
+ * are not busy CPUs. As CPUs move in and out of idle state frequently,
+ * also check the group utilization to smoother the decision.
+ */
+ if (!sds->local_stat.group_util)
+ return sched_asym_prefer(dst_cpu, sg->asym_prefer_cpu);
+
+ return false;
+#else
+ /* Always return false so that callers deal with non-SMT cases. */
+ return false;
+#endif
+}
+
static inline bool
sched_asym(struct lb_env *env, struct sd_lb_stats *sds, struct sg_lb_stats *sgs,
struct sched_group *group)
{
+ /* Only do SMT checks if either local or candidate have SMT siblings */
+ if ((sds->local->flags & SD_SHARE_CPUCAPACITY) ||
+ (group->flags & SD_SHARE_CPUCAPACITY))
+ return asym_smt_can_pull_tasks(env->dst_cpu, sds, sgs, group);
+
return sched_asym_prefer(env->dst_cpu, group->asym_prefer_cpu);
}
@@ -9540,6 +9629,12 @@ static struct rq *find_busiest_queue(struct lb_env *env,
nr_running == 1)
continue;
+ /* Make sure we only pull tasks from a CPU of lower priority */
+ if ((env->sd->flags & SD_ASYM_PACKING) &&
+ sched_asym_prefer(i, env->dst_cpu) &&
+ nr_running == 1)
+ continue;
+
switch (env->migration_type) {
case migrate_load:
/*
--
2.17.1
^ permalink raw reply related
* [PATCH v4 4/6] sched/fair: Provide update_sg_lb_stats() with sched domain statistics
From: Ricardo Neri @ 2021-08-10 14:41 UTC (permalink / raw)
To: Peter Zijlstra (Intel), Ingo Molnar, Juri Lelli, Vincent Guittot
Cc: Len Brown, Aubrey Li, Rafael J. Wysocki, Srikar Dronamraju,
Ravi V. Shankar, linuxppc-dev, Aubrey Li, Nicholas Piggin,
Ricardo Neri, Steven Rostedt, Quentin Perret, Ben Segall,
Mel Gorman, Daniel Bristot de Oliveira, Srinivas Pandruvada,
Joel Fernandes (Google), Tim Chen, Dietmar Eggemann, linux-kernel,
Ricardo Neri
In-Reply-To: <20210810144145.18776-1-ricardo.neri-calderon@linux.intel.com>
Before deciding to pull tasks when using asymmetric packing of tasks,
on some architectures (e.g., x86) it is necessary to know not only the
state of dst_cpu but also of its SMT siblings. The decision to classify
a candidate busiest group as group_asym_packing is done in
update_sg_lb_stats(). Give this function access to the scheduling domain
statistics, which contains the statistics of the local group.
Cc: Aubrey Li <aubrey.li@intel.com>
Cc: Ben Segall <bsegall@google.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Quentin Perret <qperret@google.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Reviewed-by: Len Brown <len.brown@intel.com>
Originally-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v3:
* None
Changes since v2:
* Introduced this patch.
Changes since v1:
* N/A
---
kernel/sched/fair.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 3b686e18a39b..ae3d2bc59d8d 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8539,6 +8539,7 @@ group_type group_classify(unsigned int imbalance_pct,
* @sg_status: Holds flag indicating the status of the sched_group
*/
static inline void update_sg_lb_stats(struct lb_env *env,
+ struct sd_lb_stats *sds,
struct sched_group *group,
struct sg_lb_stats *sgs,
int *sg_status)
@@ -8547,7 +8548,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
memset(sgs, 0, sizeof(*sgs));
- local_group = cpumask_test_cpu(env->dst_cpu, sched_group_span(group));
+ local_group = group == sds->local;
for_each_cpu_and(i, sched_group_span(group), env->cpus) {
struct rq *rq = cpu_rq(i);
@@ -9110,7 +9111,7 @@ static inline void update_sd_lb_stats(struct lb_env *env, struct sd_lb_stats *sd
update_group_capacity(env->sd, env->dst_cpu);
}
- update_sg_lb_stats(env, sg, sgs, &sg_status);
+ update_sg_lb_stats(env, sds, sg, sgs, &sg_status);
if (local_group)
goto next_group;
--
2.17.1
^ permalink raw reply related
* [PATCH v4 3/6] sched/fair: Optimize checking for group_asym_packing
From: Ricardo Neri @ 2021-08-10 14:41 UTC (permalink / raw)
To: Peter Zijlstra (Intel), Ingo Molnar, Juri Lelli, Vincent Guittot
Cc: Len Brown, Aubrey Li, Rafael J. Wysocki, Srikar Dronamraju,
Ravi V. Shankar, linuxppc-dev, Aubrey Li, Nicholas Piggin,
Ricardo Neri, Steven Rostedt, Quentin Perret, Ben Segall,
Mel Gorman, Daniel Bristot de Oliveira, Srinivas Pandruvada,
Joel Fernandes (Google), Tim Chen, Dietmar Eggemann, linux-kernel,
Ricardo Neri
In-Reply-To: <20210810144145.18776-1-ricardo.neri-calderon@linux.intel.com>
sched_asmy_prefer() always returns false when called on the local group. By
checking local_group, we can avoid additional checks and invoking
sched_asmy_prefer() when it is not needed. No functional changes are
introduced.
Cc: Aubrey Li <aubrey.li@intel.com>
Cc: Ben Segall <bsegall@google.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Quentin Perret <qperret@google.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Reviewed-by: Len Brown <len.brown@intel.com>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v3:
* Further rewording of the commit message. (Len)
Changes since v2:
* Reworded the commit message for clarity. (Peter Z)
Changes since v1:
* None
---
kernel/sched/fair.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 44e44c235f1f..3b686e18a39b 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8591,7 +8591,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
}
/* Check if dst CPU is idle and preferred to this group */
- if (env->sd->flags & SD_ASYM_PACKING &&
+ if (!local_group && env->sd->flags & SD_ASYM_PACKING &&
env->idle != CPU_NOT_IDLE &&
sgs->sum_h_nr_running &&
sched_asym_prefer(env->dst_cpu, group->asym_prefer_cpu)) {
--
2.17.1
^ permalink raw reply related
* [PATCH v4 2/6] sched/topology: Introduce sched_group::flags
From: Ricardo Neri @ 2021-08-10 14:41 UTC (permalink / raw)
To: Peter Zijlstra (Intel), Ingo Molnar, Juri Lelli, Vincent Guittot
Cc: Len Brown, Aubrey Li, Rafael J. Wysocki, Srikar Dronamraju,
Ravi V. Shankar, linuxppc-dev, Aubrey Li, Nicholas Piggin,
Ricardo Neri, Steven Rostedt, Quentin Perret, Ben Segall,
Mel Gorman, Daniel Bristot de Oliveira, Srinivas Pandruvada,
Joel Fernandes (Google), Tim Chen, Dietmar Eggemann, linux-kernel,
Ricardo Neri
In-Reply-To: <20210810144145.18776-1-ricardo.neri-calderon@linux.intel.com>
There exist situations in which the load balance needs to know the
properties of the CPUs in a scheduling group. When using asymmetric
packing, for instance, the load balancer needs to know not only the
state of dst_cpu but also of its SMT siblings, if any.
Use the flags of the child scheduling domains to initialize scheduling
group flags. This will reflect the properties of the CPUs in the
group.
A subsequent changeset will make use of these new flags. No functional
changes are introduced.
Cc: Aubrey Li <aubrey.li@intel.com>
Cc: Ben Segall <bsegall@google.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Quentin Perret <qperret@google.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Reviewed-by: Len Brown <len.brown@intel.com>
Originally-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v3:
* Clear the flags of the scheduling groups of a domain if its child is
destroyed.
* Minor rewording of the commit message.
Changes since v2:
* Introduced this patch.
Changes since v1:
* N/A
---
kernel/sched/sched.h | 1 +
kernel/sched/topology.c | 21 ++++++++++++++++++---
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index a2c6f6ae6392..fb716e78974d 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1799,6 +1799,7 @@ struct sched_group {
unsigned int group_weight;
struct sched_group_capacity *sgc;
int asym_prefer_cpu; /* CPU of highest priority in group */
+ int flags;
/*
* The CPUs this group covers.
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index b77ad49dc14f..27c959de175d 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -716,8 +716,20 @@ cpu_attach_domain(struct sched_domain *sd, struct root_domain *rd, int cpu)
tmp = sd;
sd = sd->parent;
destroy_sched_domain(tmp);
- if (sd)
+ if (sd) {
+ struct sched_group *sg = sd->groups;
+
+ /*
+ * sched groups hold the flags of the child sched
+ * domain for convenience. Clear such flags since
+ * the child is being destroyed.
+ */
+ do {
+ sg->flags = 0;
+ } while (sg != sd->groups);
+
sd->child = NULL;
+ }
}
for (tmp = sd; tmp; tmp = tmp->parent)
@@ -916,10 +928,12 @@ build_group_from_child_sched_domain(struct sched_domain *sd, int cpu)
return NULL;
sg_span = sched_group_span(sg);
- if (sd->child)
+ if (sd->child) {
cpumask_copy(sg_span, sched_domain_span(sd->child));
- else
+ sg->flags = sd->child->flags;
+ } else {
cpumask_copy(sg_span, sched_domain_span(sd));
+ }
atomic_inc(&sg->ref);
return sg;
@@ -1169,6 +1183,7 @@ static struct sched_group *get_group(int cpu, struct sd_data *sdd)
if (child) {
cpumask_copy(sched_group_span(sg), sched_domain_span(child));
cpumask_copy(group_balance_mask(sg), sched_group_span(sg));
+ sg->flags = child->flags;
} else {
cpumask_set_cpu(cpu, sched_group_span(sg));
cpumask_set_cpu(cpu, group_balance_mask(sg));
--
2.17.1
^ permalink raw reply related
* [PATCH v4 1/6] x86/sched: Decrease further the priorities of SMT siblings
From: Ricardo Neri @ 2021-08-10 14:41 UTC (permalink / raw)
To: Peter Zijlstra (Intel), Ingo Molnar, Juri Lelli, Vincent Guittot
Cc: Len Brown, Aubrey Li, Rafael J. Wysocki, Srikar Dronamraju,
Ravi V. Shankar, linuxppc-dev, Aubrey Li, Nicholas Piggin,
Ricardo Neri, Steven Rostedt, Quentin Perret, Ben Segall,
Mel Gorman, Daniel Bristot de Oliveira, Srinivas Pandruvada,
Joel Fernandes (Google), Tim Chen, Dietmar Eggemann, linux-kernel,
Ricardo Neri
In-Reply-To: <20210810144145.18776-1-ricardo.neri-calderon@linux.intel.com>
When scheduling, it is better to prefer a separate physical core rather
than the SMT sibling of a high priority core. The existing formula to
compute priorities takes such fact in consideration. There may exist,
however, combinations of priorities (i.e., maximum frequencies) in which
the priority of high-numbered SMT siblings of high-priority cores collides
with the priority of low-numbered SMT siblings of low-priority cores.
Consider for instance an SMT2 system with CPUs [0, 1] with priority 60 and
[2, 3] with priority 30(CPUs in brackets are SMT siblings. In such a case,
the resulting priorities would be [120, 60], [60, 30]. Thus, to ensure
that CPU2 has higher priority than CPU1, divide the raw priority by the
squared SMT iterator. The resulting priorities are [120, 30]. [60, 15].
Cc: Aubrey Li <aubrey.li@intel.com>
Cc: Ben Segall <bsegall@google.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Joel Fernandes (Google) <joel@joelfernandes.org>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Quentin Perret <qperret@google.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Originally-by: Len Brown <len.brown@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v3:
* Introduced this patch
Changes since v2:
* N/A
Changes since v1:
* N/A
---
arch/x86/kernel/itmt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/itmt.c b/arch/x86/kernel/itmt.c
index 1afbdd1dd777..9ff480e94511 100644
--- a/arch/x86/kernel/itmt.c
+++ b/arch/x86/kernel/itmt.c
@@ -198,7 +198,7 @@ void sched_set_itmt_core_prio(int prio, int core_cpu)
* of the priority chain and only used when
* all other high priority cpus are out of capacity.
*/
- smt_prio = prio * smp_num_siblings / i;
+ smt_prio = prio * smp_num_siblings / (i * i);
per_cpu(sched_core_priority, cpu) = smt_prio;
i++;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v4 0/6] sched/fair: Fix load balancing of SMT siblings with ASYM_PACKING
From: Ricardo Neri @ 2021-08-10 14:41 UTC (permalink / raw)
To: Peter Zijlstra (Intel), Ingo Molnar, Juri Lelli, Vincent Guittot
Cc: Len Brown, Rafael J. Wysocki, Srikar Dronamraju, Ravi V. Shankar,
linuxppc-dev, Aubrey Li, Nicholas Piggin, Ricardo Neri,
Steven Rostedt, Quentin Perret, Ben Segall, Mel Gorman,
Srinivas Pandruvada, Joel Fernandes (Google), Tim Chen,
Dietmar Eggemann, linux-kernel, Ricardo Neri
This is v4 the series. v1, v2, and v3 patches and test results can be found
in [1], [2], and [3], respectively.
=== Problem statement ===
++ Load balancing ++
When using asymmetric packing, there exists CPU topologies with three
priority levels in which only a subset of the physical cores support SMT.
An instance of such topology is Intel Alderlake, a hybrid processor with
a mixture of Intel Core (with support for SMT) and Intel Atom CPUs.
On Alderlake, it is almost always beneficial to spread work by picking
first the Core CPUs, then the Atoms and at last the SMT siblings.
The current load balancer, however, does not behave as described when using
ASYM_PACKING. Instead, the load balancer will choose higher-priority CPUs
(an Intel Core) over medium-priority CPUs (an Intel Atom), and subsequently
overflow the load to a low priority SMT sibling CPU. This leaves medium-
priority CPUs idle while low-priority CPUs are busy.
This patchset fixes this behavior by also checking the idle state of the
SMT siblings of both the CPU doing the load balance and the busiest
candidate group when deciding whether the destination CPUs can pull tasks
from the busiest CPU.
++ Rework ASYM_PACKING priorities with ITMT
We also reworked the priority computation of the SMT siblings to ensure
that higher-numbered SMT siblings are always low priority. The current
computation may lead to situations in which in some processors those
higher-numbered SMT siblings have the same priority as the Intel Atom
CPUs.
=== Testing ===
I ran a few benchmarks with and without this version of the patchset on
an Intel Alderlake system with 8 Intel Core (with SMT) and 8 Intel
Atom CPUs.
The baseline for the results is an unmodified v5.13-rc4 kernel. Results
show a comparative percentage of improvement (positive) or degradation
(negative). Each test case is repeated eight times, and the standard
deviation among repetitions is also documented.
Table 1 shows the results when using hardware-controlled performance
performance states (HWP), a common use case. The impact of the patches
is overall positive with a few test cases showing slight degradation.
hackbench is especially difficult to assess it shows a high degree of
variability.
Thanks and BR,
Ricardo
ITMT: Intel Turbo Boost Max Technology 3.0
========
Changes since v3:
* Reworked the ITMT priority computation to further reduce the priority
of SMT siblings (patch 1).
* Clear scheduling group flags when a child scheduling level
degenerates (patch 2).
* Removed arch-specific hooks (patch 6, PeterZ)
* Removed redundant checks for the local group. (patch 5, Dietmar)
* Removed redundant check for local idle CPUs and local group
utilization. (patch 6, Joel)
* Reworded commit messages of patches 2, 3, 5, and 6 for clarity.
(Len, PeterZ)
* Added Joel's Reviewed-by tag.
* Unchanged patches: 4
Changes since v2:
* Removed arch_sched_asym_prefer_early() and re-evaluation of the
candidate busiest group in update_sd_pick_busiest(). (PeterZ)
* Introduced sched_group::flags to reflect the properties of CPUs
in the scheduling group. This helps to identify scheduling groups
whose CPUs are SMT siblings. (PeterZ)
* Modified update_sg_lb_stats() to get statistics of the scheduling
domain as an argument. This provides it with the statistics of the
local domain. (PeterZ)
* Introduced sched_asym() to decide if a busiest candidate group can
be marked for asymmetric packing.
* Reworded patch 1 for clarity. (PeterZ)
Changes since v1:
* Don't bailout in update_sd_pick_busiest() if dst_cpu cannot pull
tasks. Instead, reclassify the candidate busiest group, as it
may still be selected. (PeterZ)
* Avoid an expensive and unnecessary call to cpumask_weight() when
determining if a sched_group is comprised of SMT siblings.
(PeterZ).
* Updated test results using the v2 patches.
======== Table 1. Test results of patches with HWP ========
=======================================================================
hackbench
=========
case load baseline(std%) compare%( std%)
process-pipe group-1 1.00 ( 1.45) -2.99 ( 4.35)
process-pipe group-2 1.00 ( 18.32) +2.14 ( 8.33)
process-pipe group-4 1.00 ( 17.27) -10.68 ( 15.85)
process-pipe group-8 1.00 ( 12.33) +2.26 ( 13.28)
process-pipe group-12 1.00 ( 6.52) -4.07 ( 7.97)
process-pipe group-16 1.00 ( 9.70) -7.71 ( 6.01)
process-pipe group-20 1.00 ( 2.52) -4.15 ( 6.35)
process-pipe group-24 1.00 ( 4.84) +1.04 ( 4.60)
process-pipe group-32 1.00 ( 4.79) +1.72 ( 5.13)
process-pipe group-48 1.00 ( 6.77) +4.68 ( 4.24)
process-sockets group-1 1.00 ( 1.89) +0.53 ( 3.48)
process-sockets group-2 1.00 ( 7.57) -6.16 ( 4.52)
process-sockets group-4 1.00 ( 14.62) +4.93 ( 7.11)
process-sockets group-8 1.00 ( 7.69) +3.15 ( 7.19)
process-sockets group-12 1.00 ( 4.97) +2.49 ( 2.80)
process-sockets group-16 1.00 ( 3.93) -1.57 ( 3.86)
process-sockets group-20 1.00 ( 2.56) -3.63 ( 2.88)
process-sockets group-24 1.00 ( 3.00) +0.74 ( 3.01)
process-sockets group-32 1.00 ( 7.63) +1.79 ( 3.67)
process-sockets group-48 1.00 ( 4.15) -0.44 ( 3.70)
threads-pipe group-1 1.00 ( 2.34) -0.55 ( 3.78)
threads-pipe group-2 1.00 ( 12.74) -2.24 ( 12.96)
threads-pipe group-4 1.00 ( 10.03) +5.80 ( 16.02)
threads-pipe group-8 1.00 ( 7.45) -12.09 ( 22.91)
threads-pipe group-12 1.00 ( 5.00) -15.25 ( 10.86)
threads-pipe group-16 1.00 ( 7.41) +1.95 ( 11.73)
threads-pipe group-20 1.00 ( 7.31) -1.72 ( 5.17)
threads-pipe group-24 1.00 ( 4.48) +0.43 ( 6.39)
threads-pipe group-32 1.00 ( 3.75) -0.62 ( 3.87)
threads-pipe group-48 1.00 ( 1.56) -3.69 ( 5.99)
threads-sockets group-1 1.00 ( 2.27) +3.51 ( 3.79)
threads-sockets group-2 1.00 ( 6.86) -8.42 ( 11.39)
threads-sockets group-4 1.00 ( 5.28) -14.35 ( 8.73)
threads-sockets group-8 1.00 ( 11.74) +5.04 ( 5.18)
threads-sockets group-12 1.00 ( 3.29) -6.15 ( 7.08)
threads-sockets group-16 1.00 ( 5.07) -1.40 ( 6.49)
threads-sockets group-20 1.00 ( 4.38) -5.44 ( 5.68)
threads-sockets group-24 1.00 ( 7.20) +3.67 ( 3.99)
threads-sockets group-32 1.00 ( 3.30) +0.10 ( 3.08)
threads-sockets group-48 1.00 ( 4.83) +4.83 ( 3.61)
netperf
=======
case load baseline(std%) compare%( std%)
TCP_RR thread-1 1.00 ( 0.00) +1.20 ( 0.71)
TCP_RR thread-2 1.00 ( 3.03) +0.25 ( 3.05)
TCP_RR thread-4 1.00 ( 1.99) +1.21 ( 3.40)
TCP_RR thread-8 1.00 ( 1.36) -2.00 ( 6.79)
TCP_RR thread-12 1.00 ( 4.53) +1.68 ( 0.95)
TCP_RR thread-16 1.00 ( 0.56) -1.20 ( 1.27)
TCP_RR thread-20 1.00 ( 2.41) +0.68 ( 0.36)
TCP_RR thread-24 1.00 ( 1.27) -0.16 ( 0.29)
TCP_RR thread-32 1.00 ( 11.14) -0.35 ( 11.10)
TCP_RR thread-48 1.00 ( 18.56) -0.12 ( 19.98)
UDP_RR thread-1 1.00 ( 0.00) +1.85 ( 1.53)
UDP_RR thread-2 1.00 ( 2.78) -2.02 ( 3.14)
UDP_RR thread-4 1.00 ( 2.26) -0.54 ( 2.42)
UDP_RR thread-8 1.00 ( 1.46) -1.49 ( 5.31)
UDP_RR thread-12 1.00 ( 0.70) +0.21 ( 1.64)
UDP_RR thread-16 1.00 ( 1.26) -1.90 ( 2.87)
UDP_RR thread-20 1.00 ( 0.29) +0.29 ( 0.27)
UDP_RR thread-24 1.00 ( 2.75) +2.61 ( 0.97)
UDP_RR thread-32 1.00 ( 11.16) -2.90 ( 11.26)
UDP_RR thread-48 1.00 ( 19.22) +3.12 ( 17.32)
tbench
======
case load baseline(std%) compare%( std%)
loopback thread-1 1.00 ( 1.33) +1.41 ( 0.38)
loopback thread-2 1.00 ( 1.03) +0.06 ( 1.08)
loopback thread-4 1.00 ( 1.05) -0.74 ( 1.65)
loopback thread-8 1.00 ( 5.81) +6.66 ( 0.31)
loopback thread-12 1.00 ( 0.60) +0.32 ( 0.20)
loopback thread-16 1.00 ( 1.38) +1.67 ( 0.34)
loopback thread-20 1.00 ( 2.49) +0.24 ( 2.96)
loopback thread-24 1.00 ( 2.26) +0.75 ( 0.29)
loopback thread-32 1.00 ( 2.54) +0.07 ( 2.83)
loopback thread-48 1.00 ( 2.40) +1.90 ( 2.32)
schbench
========
case load baseline(std%) compare%( std%)
normal mthread-1 1.00 ( 3.51) +1.03 ( 2.96)
normal mthread-2 1.00 ( 2.33) +6.35 ( 6.19)
normal mthread-4 1.00 ( 9.58) +0.12 ( 8.02)
normal mthread-8 1.00 ( 1.59) +0.40 ( 0.07)
normal mthread-12 1.00 ( 3.18) +1.51 ( 4.19)
normal mthread-16 1.00 ( 1.72) +0.00 ( 1.72)
normal mthread-20 1.00 ( 1.43) +0.80 ( 3.16)
normal mthread-24 1.00 ( 1.38) -1.69 ( 1.71)
normal mthread-32 1.00 ( 1.86) -0.78 ( 2.25)
normal mthread-48 1.00 ( 3.50) -2.81 ( 2.82)
========
[1]. https://lore.kernel.org/lkml/20210406041108.7416-1-ricardo.neri-calderon@linux.intel.com/
[2]. https://lore.kernel.org/lkml/20210414020436.12980-1-ricardo.neri-calderon@linux.intel.com/
[3]. https://lore.kernel.org/lkml/20210513154909.6385-1-ricardo.neri-calderon@linux.intel.com/
Ricardo Neri (6):
x86/sched: Decrease further the priorities of SMT siblings
sched/topology: Introduce sched_group::flags
sched/fair: Optimize checking for group_asym_packing
sched/fair: Provide update_sg_lb_stats() with sched domain statistics
sched/fair: Carve out logic to mark a group for asymmetric packing
sched/fair: Consider SMT in ASYM_PACKING load balance
arch/x86/kernel/itmt.c | 2 +-
kernel/sched/fair.c | 122 ++++++++++++++++++++++++++++++++++++----
kernel/sched/sched.h | 1 +
kernel/sched/topology.c | 21 ++++++-
4 files changed, 132 insertions(+), 14 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v4 5/6] sched/fair: Carve out logic to mark a group for asymmetric packing
From: Ricardo Neri @ 2021-08-10 14:41 UTC (permalink / raw)
To: Peter Zijlstra (Intel), Ingo Molnar, Juri Lelli, Vincent Guittot
Cc: Len Brown, Aubrey Li, Rafael J. Wysocki, Srikar Dronamraju,
Ravi V. Shankar, linuxppc-dev, Aubrey Li, Nicholas Piggin,
Ricardo Neri, Steven Rostedt, Quentin Perret, Ben Segall,
Mel Gorman, Daniel Bristot de Oliveira, Srinivas Pandruvada,
Joel Fernandes (Google), Tim Chen, Dietmar Eggemann, linux-kernel,
Ricardo Neri
In-Reply-To: <20210810144145.18776-1-ricardo.neri-calderon@linux.intel.com>
Create a separate function, sched_asym(). A subsequent changeset will
introduce logic to deal with SMT in conjunction with asmymmetric
packing. Such logic will need the statistics of the scheduling
group provided as argument. Update them before calling sched_asym().
Cc: Aubrey Li <aubrey.li@intel.com>
Cc: Ben Segall <bsegall@google.com>
Cc: Daniel Bristot de Oliveira <bristot@redhat.com>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Quentin Perret <qperret@google.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tim Chen <tim.c.chen@linux.intel.com>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Reviewed-by: Len Brown <len.brown@intel.com>
Co-developed-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v3:
* Remove a redundant check for the local group in sched_asym().
(Dietmar)
* Reworded commit message for clarity. (Len)
Changes since v2:
* Introduced this patch.
Changes since v1:
* N/A
---
kernel/sched/fair.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index ae3d2bc59d8d..dd411cefb63f 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8531,6 +8531,13 @@ group_type group_classify(unsigned int imbalance_pct,
return group_has_spare;
}
+static inline bool
+sched_asym(struct lb_env *env, struct sd_lb_stats *sds, struct sg_lb_stats *sgs,
+ struct sched_group *group)
+{
+ return sched_asym_prefer(env->dst_cpu, group->asym_prefer_cpu);
+}
+
/**
* update_sg_lb_stats - Update sched_group's statistics for load balancing.
* @env: The load balancing environment.
@@ -8591,18 +8598,17 @@ static inline void update_sg_lb_stats(struct lb_env *env,
}
}
+ sgs->group_capacity = group->sgc->capacity;
+
+ sgs->group_weight = group->group_weight;
+
/* Check if dst CPU is idle and preferred to this group */
if (!local_group && env->sd->flags & SD_ASYM_PACKING &&
- env->idle != CPU_NOT_IDLE &&
- sgs->sum_h_nr_running &&
- sched_asym_prefer(env->dst_cpu, group->asym_prefer_cpu)) {
+ env->idle != CPU_NOT_IDLE && sgs->sum_h_nr_running &&
+ sched_asym(env, sds, sgs, group)) {
sgs->group_asym_packing = 1;
}
- sgs->group_capacity = group->sgc->capacity;
-
- sgs->group_weight = group->group_weight;
-
sgs->group_type = group_classify(env->sd->imbalance_pct, group, sgs);
/* Computing avg_load makes sense only when group is overloaded */
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2] powerpc/xive: Do not skip CPU-less nodes when creating the IPIs
From: Cédric Le Goater @ 2021-08-10 12:10 UTC (permalink / raw)
To: linuxppc-dev
Cc: Laurent Vivier, Geetika Moolchandani, Srikar Dronamraju, stable
In-Reply-To: <20210807072057.184698-1-clg@kaod.org>
On 8/7/21 9:20 AM, Cédric Le Goater wrote:
> On PowerVM, CPU-less nodes can be populated with hot-plugged CPUs at
> runtime. Today, the IPI is not created for such nodes, and hot-plugged
> CPUs use a bogus IPI, which leads to soft lockups.
>
> We can not directly allocate and request the IPI on demand because
> bringup_up() is called under the IRQ sparse lock. The alternative is
> to allocate the IPIs for all possible nodes at startup and to request
> the mapping on demand when the first CPU of a node is brought up.
>
> Fixes: 7dcc37b3eff9 ("powerpc/xive: Map one IPI interrupt per node")
> Cc: stable@vger.kernel.org # v5.13
> Reported-by: Geetika Moolchandani <Geetika.Moolchandani1@ibm.com>
> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Cc: Laurent Vivier <lvivier@redhat.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Message-Id: <20210629131542.743888-1-clg@kaod.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> arch/powerpc/sysdev/xive/common.c | 35 +++++++++++++++++++++----------
> 1 file changed, 24 insertions(+), 11 deletions(-)
I forgot to add that this version does break irqbalance anymore, since
Linux is not mapping interrupts of CPU-less nodes.
Anyhow, irqbalance is now fixed :
https://github.com/Irqbalance/irqbalance/commit/a7f81483a95a94d6a62ca7fb999a090e01c0de9b
So v1 (plus irqbalance patch above) or v2 are safe to use. I do prefer
v2.
Thanks to Laurent and Srikar for the extra tests,
C.
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index dbdbbc2f1dc5..943fd30095af 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -67,6 +67,7 @@ static struct irq_domain *xive_irq_domain;
> static struct xive_ipi_desc {
> unsigned int irq;
> char name[16];
> + atomic_t started;
> } *xive_ipis;
>
> /*
> @@ -1120,7 +1121,7 @@ static const struct irq_domain_ops xive_ipi_irq_domain_ops = {
> .alloc = xive_ipi_irq_domain_alloc,
> };
>
> -static int __init xive_request_ipi(void)
> +static int __init xive_init_ipis(void)
> {
> struct fwnode_handle *fwnode;
> struct irq_domain *ipi_domain;
> @@ -1144,10 +1145,6 @@ static int __init xive_request_ipi(void)
> struct xive_ipi_desc *xid = &xive_ipis[node];
> struct xive_ipi_alloc_info info = { node };
>
> - /* Skip nodes without CPUs */
> - if (cpumask_empty(cpumask_of_node(node)))
> - continue;
> -
> /*
> * Map one IPI interrupt per node for all cpus of that node.
> * Since the HW interrupt number doesn't have any meaning,
> @@ -1159,11 +1156,6 @@ static int __init xive_request_ipi(void)
> xid->irq = ret;
>
> snprintf(xid->name, sizeof(xid->name), "IPI-%d", node);
> -
> - ret = request_irq(xid->irq, xive_muxed_ipi_action,
> - IRQF_PERCPU | IRQF_NO_THREAD, xid->name, NULL);
> -
> - WARN(ret < 0, "Failed to request IPI %d: %d\n", xid->irq, ret);
> }
>
> return ret;
> @@ -1178,6 +1170,22 @@ static int __init xive_request_ipi(void)
> return ret;
> }
>
> +static int __init xive_request_ipi(unsigned int cpu)
> +{
> + struct xive_ipi_desc *xid = &xive_ipis[early_cpu_to_node(cpu)];
> + int ret;
> +
> + if (atomic_inc_return(&xid->started) > 1)
> + return 0;
> +
> + ret = request_irq(xid->irq, xive_muxed_ipi_action,
> + IRQF_PERCPU | IRQF_NO_THREAD,
> + xid->name, NULL);
> +
> + WARN(ret < 0, "Failed to request IPI %d: %d\n", xid->irq, ret);
> + return ret;
> +}
> +
> static int xive_setup_cpu_ipi(unsigned int cpu)
> {
> unsigned int xive_ipi_irq = xive_ipi_cpu_to_irq(cpu);
> @@ -1192,6 +1200,9 @@ static int xive_setup_cpu_ipi(unsigned int cpu)
> if (xc->hw_ipi != XIVE_BAD_IRQ)
> return 0;
>
> + /* Register the IPI */
> + xive_request_ipi(cpu);
> +
> /* Grab an IPI from the backend, this will populate xc->hw_ipi */
> if (xive_ops->get_ipi(cpu, xc))
> return -EIO;
> @@ -1231,6 +1242,8 @@ static void xive_cleanup_cpu_ipi(unsigned int cpu, struct xive_cpu *xc)
> if (xc->hw_ipi == XIVE_BAD_IRQ)
> return;
>
> + /* TODO: clear IPI mapping */
> +
> /* Mask the IPI */
> xive_do_source_set_mask(&xc->ipi_data, true);
>
> @@ -1253,7 +1266,7 @@ void __init xive_smp_probe(void)
> smp_ops->cause_ipi = xive_cause_ipi;
>
> /* Register the IPI */
> - xive_request_ipi();
> + xive_init_ipis();
>
> /* Allocate and setup IPI for the boot CPU */
> xive_setup_cpu_ipi(smp_processor_id());
>
^ permalink raw reply
* Re: [PATCH v2] powerpc/xive: Do not skip CPU-less nodes when creating the IPIs
From: Cédric Le Goater @ 2021-08-10 12:29 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Laurent Vivier, Geetika Moolchandani, Srikar Dronamraju
In-Reply-To: <dbcf71dc-2e5e-a6bc-9b55-f4e2f023dfd5@kaod.org>
> I forgot to add that this version does break irqbalance anymore, since
... does not break ...
Sorry for the noise,
> Linux is not mapping interrupts of CPU-less nodes.
>
> Anyhow, irqbalance is now fixed :
>
> https://github.com/Irqbalance/irqbalance/commit/a7f81483a95a94d6a62ca7fb999a090e01c0de9b
>
> So v1 (plus irqbalance patch above) or v2 are safe to use. I do prefer
> v2.
>
> Thanks to Laurent and Srikar for the extra tests,
>
> C.
^ permalink raw reply
* Re: [PATCH v2 1/2] sched/topology: Skip updating masks for non-online nodes
From: Srikar Dronamraju @ 2021-08-10 11:47 UTC (permalink / raw)
To: Valentin Schneider
Cc: Nathan Lynch, Gautham R Shenoy, Vincent Guittot, Rik van Riel,
Peter Zijlstra, linuxppc-dev, Geetika Moolchandani, LKML,
Dietmar Eggemann, Thomas Gleixner, Laurent Dufour, Mel Gorman,
Ingo Molnar
In-Reply-To: <875yweaig9.mognet@arm.com>
* Valentin Schneider <valentin.schneider@arm.com> [2021-08-09 13:52:38]:
> On 09/08/21 12:22, Srikar Dronamraju wrote:
> > * Valentin Schneider <valentin.schneider@arm.com> [2021-08-08 16:56:47]:
> >> Wait, doesn't the distance matrix (without any offline node) say
> >>
> >> distance(0, 3) == 40
> >>
> >> ? We should have at the very least:
> >>
> >> node 0 1 2 3
> >> 0: 10 20 ?? 40
> >> 1: 20 20 ?? 40
> >> 2: ?? ?? ?? ??
> >> 3: 40 40 ?? 10
> >>
> >
> > Before onlining node 3 and CPU 3 (node/CPU 0 and 1 are already online)
> > Note: Node 2-7 and CPU 2-7 are still offline.
> >
> > node 0 1 2 3
> > 0: 10 20 40 10
> > 1: 20 20 40 10
> > 2: 40 40 10 10
> > 3: 10 10 10 10
> >
> > NODE->mask(0) == 0
> > NODE->mask(1) == 1
> > NODE->mask(2) == 0
> > NODE->mask(3) == 0
> >
> > Note: This is with updating Node 2's distance as 40 for figuring out
> > the number of numa levels. Since we have all possible distances, we
> > dont update Node 3 distance, so it will be as if its local to node 0.
> >
> > Now when Node 3 and CPU 3 are onlined
> > Note: Node 2, 3-7 and CPU 2, 3-7 are still offline.
> >
> > node 0 1 2 3
> > 0: 10 20 40 40
> > 1: 20 20 40 40
> > 2: 40 40 10 40
> > 3: 40 40 40 10
> >
> > NODE->mask(0) == 0
> > NODE->mask(1) == 1
> > NODE->mask(2) == 0
> > NODE->mask(3) == 0,3
> >
> > CPU 0 continues to be part of Node->mask(3) because when we online and
> > we find the right distance, there is no API to reset the numa mask of
> > 3 to remove CPU 0 from the numa masks.
> >
> > If we had an API to clear/set sched_domains_numa_masks[node][] when
> > the node state changes, we could probably plug-in to clear/set the
> > node masks whenever node state changes.
> >
>
> Gotcha, this is now coming back to me...
>
> [...]
>
> >> Ok, so it looks like we really can't do without that part - even if we get
> >> "sensible" distance values for the online nodes, we can't divine values for
> >> the offline ones.
> >>
> >
> > Yes
> >
>
> Argh, while your approach does take care of the masks, it leaves
> sched_numa_topology_type unchanged. You *can* force an update of it, but
> yuck :(
>
> I got to the below...
>
Yes, I completely missed that we should update sched_numa_topology_type.
> ---
> From: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Date: Thu, 1 Jul 2021 09:45:51 +0530
> Subject: [PATCH 1/1] sched/topology: Skip updating masks for non-online nodes
>
> The scheduler currently expects NUMA node distances to be stable from init
> onwards, and as a consequence builds the related data structures
> once-and-for-all at init (see sched_init_numa()).
>
> Unfortunately, on some architectures node distance is unreliable for
> offline nodes and may very well change upon onlining.
>
> Skip over offline nodes during sched_init_numa(). Track nodes that have
> been onlined at least once, and trigger a build of a node's NUMA masks when
> it is first onlined post-init.
>
Your version is much much better than mine.
And I have verified that it works as expected.
> Reported-by: Geetika Moolchandani <Geetika.Moolchandani1@ibm.com>
> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Signed-off-by: Valentin Schneider <valentin.schneider@arm.com>
> ---
> kernel/sched/topology.c | 65 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 65 insertions(+)
>
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index b77ad49dc14f..cba95793a9b7 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -1482,6 +1482,8 @@ int sched_max_numa_distance;
> static int *sched_domains_numa_distance;
> static struct cpumask ***sched_domains_numa_masks;
> int __read_mostly node_reclaim_distance = RECLAIM_DISTANCE;
> +
> +static unsigned long __read_mostly *sched_numa_onlined_nodes;
> #endif
>
> /*
> @@ -1833,6 +1835,16 @@ void sched_init_numa(void)
> sched_domains_numa_masks[i][j] = mask;
>
> for_each_node(k) {
> + /*
> + * Distance information can be unreliable for
> + * offline nodes, defer building the node
> + * masks to its bringup.
> + * This relies on all unique distance values
> + * still being visible at init time.
> + */
> + if (!node_online(j))
> + continue;
> +
> if (sched_debug() && (node_distance(j, k) != node_distance(k, j)))
> sched_numa_warn("Node-distance not symmetric");
>
> @@ -1886,6 +1898,53 @@ void sched_init_numa(void)
> sched_max_numa_distance = sched_domains_numa_distance[nr_levels - 1];
>
> init_numa_topology_type();
> +
> + sched_numa_onlined_nodes = bitmap_alloc(nr_node_ids, GFP_KERNEL);
> + if (!sched_numa_onlined_nodes)
> + return;
> +
> + bitmap_zero(sched_numa_onlined_nodes, nr_node_ids);
> + for_each_online_node(i)
> + bitmap_set(sched_numa_onlined_nodes, i, 1);
> +}
> +
> +void __sched_domains_numa_masks_set(unsigned int node)
> +{
> + int i, j;
> +
> + /*
> + * NUMA masks are not built for offline nodes in sched_init_numa().
> + * Thus, when a CPU of a never-onlined-before node gets plugged in,
> + * adding that new CPU to the right NUMA masks is not sufficient: the
> + * masks of that CPU's node must also be updated.
> + */
> + if (test_bit(node, sched_numa_onlined_nodes))
> + return;
> +
> + bitmap_set(sched_numa_onlined_nodes, node, 1);
> +
> + for (i = 0; i < sched_domains_numa_levels; i++) {
> + for (j = 0; j < nr_node_ids; j++) {
> + if (!node_online(j) || node == j)
> + continue;
> +
> + if (node_distance(j, node) > sched_domains_numa_distance[i])
> + continue;
> +
> + /* Add remote nodes in our masks */
> + cpumask_or(sched_domains_numa_masks[i][node],
> + sched_domains_numa_masks[i][node],
> + sched_domains_numa_masks[0][j]);
> + }
> + }
> +
> + /*
> + * A new node has been brought up, potentially changing the topology
> + * classification.
> + *
> + * Note that this is racy vs any use of sched_numa_topology_type :/
> + */
> + init_numa_topology_type();
> }
>
> void sched_domains_numa_masks_set(unsigned int cpu)
> @@ -1893,8 +1952,14 @@ void sched_domains_numa_masks_set(unsigned int cpu)
> int node = cpu_to_node(cpu);
> int i, j;
>
> + __sched_domains_numa_masks_set(node);
> +
> for (i = 0; i < sched_domains_numa_levels; i++) {
> for (j = 0; j < nr_node_ids; j++) {
> + if (!node_online(j))
> + continue;
> +
> + /* Set ourselves in the remote node's masks */
> if (node_distance(j, node) <= sched_domains_numa_distance[i])
> cpumask_set_cpu(cpu, sched_domains_numa_masks[i][j]);
> }
> --
> 2.25.1
>
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* Re: [PATCH v2] powerpc/xive: Do not skip CPU-less nodes when creating the IPIs
From: Srikar Dronamraju @ 2021-08-10 11:45 UTC (permalink / raw)
To: C?dric Le Goater
Cc: Laurent Vivier, Geetika Moolchandani, linuxppc-dev, stable
In-Reply-To: <20210807072057.184698-1-clg@kaod.org>
* C?dric Le Goater <clg@kaod.org> [2021-08-07 09:20:57]:
> On PowerVM, CPU-less nodes can be populated with hot-plugged CPUs at
> runtime. Today, the IPI is not created for such nodes, and hot-plugged
> CPUs use a bogus IPI, which leads to soft lockups.
>
> We can not directly allocate and request the IPI on demand because
> bringup_up() is called under the IRQ sparse lock. The alternative is
> to allocate the IPIs for all possible nodes at startup and to request
> the mapping on demand when the first CPU of a node is brought up.
>
Thank you, this version too works for me.
Tested-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Fixes: 7dcc37b3eff9 ("powerpc/xive: Map one IPI interrupt per node")
> Cc: stable@vger.kernel.org # v5.13
> Reported-by: Geetika Moolchandani <Geetika.Moolchandani1@ibm.com>
> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Cc: Laurent Vivier <lvivier@redhat.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Message-Id: <20210629131542.743888-1-clg@kaod.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> arch/powerpc/sysdev/xive/common.c | 35 +++++++++++++++++++++----------
> 1 file changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index dbdbbc2f1dc5..943fd30095af 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -67,6 +67,7 @@ static struct irq_domain *xive_irq_domain;
> static struct xive_ipi_desc {
> unsigned int irq;
> char name[16];
> + atomic_t started;
> } *xive_ipis;
>
> /*
> @@ -1120,7 +1121,7 @@ static const struct irq_domain_ops xive_ipi_irq_domain_ops = {
> .alloc = xive_ipi_irq_domain_alloc,
> };
>
> -static int __init xive_request_ipi(void)
> +static int __init xive_init_ipis(void)
> {
> struct fwnode_handle *fwnode;
> struct irq_domain *ipi_domain;
> @@ -1144,10 +1145,6 @@ static int __init xive_request_ipi(void)
> struct xive_ipi_desc *xid = &xive_ipis[node];
> struct xive_ipi_alloc_info info = { node };
>
> - /* Skip nodes without CPUs */
> - if (cpumask_empty(cpumask_of_node(node)))
> - continue;
> -
> /*
> * Map one IPI interrupt per node for all cpus of that node.
> * Since the HW interrupt number doesn't have any meaning,
> @@ -1159,11 +1156,6 @@ static int __init xive_request_ipi(void)
> xid->irq = ret;
>
> snprintf(xid->name, sizeof(xid->name), "IPI-%d", node);
> -
> - ret = request_irq(xid->irq, xive_muxed_ipi_action,
> - IRQF_PERCPU | IRQF_NO_THREAD, xid->name, NULL);
> -
> - WARN(ret < 0, "Failed to request IPI %d: %d\n", xid->irq, ret);
> }
>
> return ret;
> @@ -1178,6 +1170,22 @@ static int __init xive_request_ipi(void)
> return ret;
> }
>
> +static int __init xive_request_ipi(unsigned int cpu)
> +{
> + struct xive_ipi_desc *xid = &xive_ipis[early_cpu_to_node(cpu)];
> + int ret;
> +
> + if (atomic_inc_return(&xid->started) > 1)
> + return 0;
> +
> + ret = request_irq(xid->irq, xive_muxed_ipi_action,
> + IRQF_PERCPU | IRQF_NO_THREAD,
> + xid->name, NULL);
> +
> + WARN(ret < 0, "Failed to request IPI %d: %d\n", xid->irq, ret);
> + return ret;
> +}
> +
> static int xive_setup_cpu_ipi(unsigned int cpu)
> {
> unsigned int xive_ipi_irq = xive_ipi_cpu_to_irq(cpu);
> @@ -1192,6 +1200,9 @@ static int xive_setup_cpu_ipi(unsigned int cpu)
> if (xc->hw_ipi != XIVE_BAD_IRQ)
> return 0;
>
> + /* Register the IPI */
> + xive_request_ipi(cpu);
> +
> /* Grab an IPI from the backend, this will populate xc->hw_ipi */
> if (xive_ops->get_ipi(cpu, xc))
> return -EIO;
> @@ -1231,6 +1242,8 @@ static void xive_cleanup_cpu_ipi(unsigned int cpu, struct xive_cpu *xc)
> if (xc->hw_ipi == XIVE_BAD_IRQ)
> return;
>
> + /* TODO: clear IPI mapping */
> +
> /* Mask the IPI */
> xive_do_source_set_mask(&xc->ipi_data, true);
>
> @@ -1253,7 +1266,7 @@ void __init xive_smp_probe(void)
> smp_ops->cause_ipi = xive_cause_ipi;
>
> /* Register the IPI */
> - xive_request_ipi();
> + xive_init_ipis();
>
> /* Allocate and setup IPI for the boot CPU */
> xive_setup_cpu_ipi(smp_processor_id());
> --
> 2.31.1
>
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* Re: [PATCH v4 0/5] nvmem: nintendo-otp: Add new driver for the Wii and Wii U OTP
From: Srinivas Kandagatla @ 2021-08-10 10:57 UTC (permalink / raw)
To: Emmanuel Gil Peyrot, Rob Herring, linuxppc-dev, devicetree
Cc: linux-kernel, Paul Mackerras, Ash Logan, Jonathan Neuschäfer
In-Reply-To: <20210801073822.12452-1-linkmauve@linkmauve.fr>
On 01/08/2021 08:38, Emmanuel Gil Peyrot wrote:
> The OTP is a read-only memory area which contains various keys and
> signatures used to decrypt, encrypt or verify various pieces of storage.
>
> Its size depends on the console, it is 128 bytes on the Wii and
> 1024 bytes on the Wii U (split into eight 128 bytes banks).
>
> It can be used directly by writing into one register and reading from
> the other one, without any additional synchronisation.
>
> This series has been tested on both the Wii U (using my downstream
> master-wiiu branch[1]), as well as on the Wii on mainline.
>
> [1] https://gitlab.com/linkmauve/linux-wiiu/-/commits/master-wiiu
>
> Changes since v1:
> - Fixed the commit messages so they can be accepted by other email
> servers, sorry about that.
>
> Changes since v2:
> - Switched the dt binding documentation to YAML.
> - Used more obvious register arithmetic, and tested that gcc (at -O1 and
> above) outputs the exact same rlwinm instructions for them.
> - Use more #defines to make the code easier to read.
> - Include some links to the reversed documentation.
> - Avoid overlapping dt regions by changing the existing control@d800100
> node to end before the OTP registers, with some bigger dt refactoring
> left for a future series.
>
> Changes since v3:
> - Relicense the dt-binding documentation under GPLv2-only or
> BSD-2-clauses.
>
> Emmanuel Gil Peyrot (5):
> nvmem: nintendo-otp: Add new driver for the Wii and Wii U OTP
> dt-bindings: nintendo-otp: Document the Wii and Wii U OTP support
Applied 1/5 and 2/5 to nvmem next,
rest of the patches should go via powerpc dts tree.
thanks,
--srini
> powerpc: wii.dts: Reduce the size of the control area
> powerpc: wii.dts: Expose the OTP on this platform
> powerpc: wii_defconfig: Enable OTP by default
>
> .../bindings/nvmem/nintendo-otp.yaml | 44 +++++++
> arch/powerpc/boot/dts/wii.dts | 13 +-
> arch/powerpc/configs/wii_defconfig | 1 +
> drivers/nvmem/Kconfig | 11 ++
> drivers/nvmem/Makefile | 2 +
> drivers/nvmem/nintendo-otp.c | 124 ++++++++++++++++++
> 6 files changed, 194 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/nvmem/nintendo-otp.yaml
> create mode 100644 drivers/nvmem/nintendo-otp.c
>
^ permalink raw reply
* Re: [PATCH v2] powerpc/xive: Do not skip CPU-less nodes when creating the IPIs
From: Laurent Vivier @ 2021-08-10 9:16 UTC (permalink / raw)
To: Cédric Le Goater, linuxppc-dev
Cc: Geetika Moolchandani, Srikar Dronamraju, stable
In-Reply-To: <20210807072057.184698-1-clg@kaod.org>
On 07/08/2021 09:20, Cédric Le Goater wrote:
> On PowerVM, CPU-less nodes can be populated with hot-plugged CPUs at
> runtime. Today, the IPI is not created for such nodes, and hot-plugged
> CPUs use a bogus IPI, which leads to soft lockups.
>
> We can not directly allocate and request the IPI on demand because
> bringup_up() is called under the IRQ sparse lock. The alternative is
> to allocate the IPIs for all possible nodes at startup and to request
> the mapping on demand when the first CPU of a node is brought up.
>
> Fixes: 7dcc37b3eff9 ("powerpc/xive: Map one IPI interrupt per node")
> Cc: stable@vger.kernel.org # v5.13
> Reported-by: Geetika Moolchandani <Geetika.Moolchandani1@ibm.com>
> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Cc: Laurent Vivier <lvivier@redhat.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Message-Id: <20210629131542.743888-1-clg@kaod.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> arch/powerpc/sysdev/xive/common.c | 35 +++++++++++++++++++++----------
> 1 file changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index dbdbbc2f1dc5..943fd30095af 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -67,6 +67,7 @@ static struct irq_domain *xive_irq_domain;
> static struct xive_ipi_desc {
> unsigned int irq;
> char name[16];
> + atomic_t started;
> } *xive_ipis;
>
> /*
> @@ -1120,7 +1121,7 @@ static const struct irq_domain_ops xive_ipi_irq_domain_ops = {
> .alloc = xive_ipi_irq_domain_alloc,
> };
>
> -static int __init xive_request_ipi(void)
> +static int __init xive_init_ipis(void)
> {
> struct fwnode_handle *fwnode;
> struct irq_domain *ipi_domain;
> @@ -1144,10 +1145,6 @@ static int __init xive_request_ipi(void)
> struct xive_ipi_desc *xid = &xive_ipis[node];
> struct xive_ipi_alloc_info info = { node };
>
> - /* Skip nodes without CPUs */
> - if (cpumask_empty(cpumask_of_node(node)))
> - continue;
> -
> /*
> * Map one IPI interrupt per node for all cpus of that node.
> * Since the HW interrupt number doesn't have any meaning,
> @@ -1159,11 +1156,6 @@ static int __init xive_request_ipi(void)
> xid->irq = ret;
>
> snprintf(xid->name, sizeof(xid->name), "IPI-%d", node);
> -
> - ret = request_irq(xid->irq, xive_muxed_ipi_action,
> - IRQF_PERCPU | IRQF_NO_THREAD, xid->name, NULL);
> -
> - WARN(ret < 0, "Failed to request IPI %d: %d\n", xid->irq, ret);
> }
>
> return ret;
> @@ -1178,6 +1170,22 @@ static int __init xive_request_ipi(void)
> return ret;
> }
>
> +static int __init xive_request_ipi(unsigned int cpu)
> +{
> + struct xive_ipi_desc *xid = &xive_ipis[early_cpu_to_node(cpu)];
> + int ret;
> +
> + if (atomic_inc_return(&xid->started) > 1)
> + return 0;
> +
> + ret = request_irq(xid->irq, xive_muxed_ipi_action,
> + IRQF_PERCPU | IRQF_NO_THREAD,
> + xid->name, NULL);
> +
> + WARN(ret < 0, "Failed to request IPI %d: %d\n", xid->irq, ret);
> + return ret;
> +}
> +
> static int xive_setup_cpu_ipi(unsigned int cpu)
> {
> unsigned int xive_ipi_irq = xive_ipi_cpu_to_irq(cpu);
> @@ -1192,6 +1200,9 @@ static int xive_setup_cpu_ipi(unsigned int cpu)
> if (xc->hw_ipi != XIVE_BAD_IRQ)
> return 0;
>
> + /* Register the IPI */
> + xive_request_ipi(cpu);
> +
> /* Grab an IPI from the backend, this will populate xc->hw_ipi */
> if (xive_ops->get_ipi(cpu, xc))
> return -EIO;
> @@ -1231,6 +1242,8 @@ static void xive_cleanup_cpu_ipi(unsigned int cpu, struct xive_cpu *xc)
> if (xc->hw_ipi == XIVE_BAD_IRQ)
> return;
>
> + /* TODO: clear IPI mapping */
> +
> /* Mask the IPI */
> xive_do_source_set_mask(&xc->ipi_data, true);
>
> @@ -1253,7 +1266,7 @@ void __init xive_smp_probe(void)
> smp_ops->cause_ipi = xive_cause_ipi;
>
> /* Register the IPI */
> - xive_request_ipi();
> + xive_init_ipis();
>
> /* Allocate and setup IPI for the boot CPU */
> xive_setup_cpu_ipi(smp_processor_id());
>
Tested-by: Laurent Vivier <lvivier@redhat.com>
^ permalink raw reply
* Re: [PATCH v2 3/4] powerpc: Optimize register usage for dear register
From: Xiongwei Song @ 2021-08-10 8:33 UTC (permalink / raw)
To: Christophe Leroy
Cc: ravi.bangoria, Xiongwei Song, oleg, npiggin,
Linux Kernel Mailing List, efremov, Paul Mackerras, aneesh.kumar,
PowerPC
In-Reply-To: <dfd24c4a-c43f-912c-5876-f1cac704f336@csgroup.eu>
On Sat, Aug 7, 2021 at 2:58 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 07/08/2021 à 03:02, sxwjean@me.com a écrit :
> > From: Xiongwei Song <sxwjean@gmail.com>
> >
> > Create an anonymous union for dar and dear regsiters, we can reference
> > dear to get the effective address when CONFIG_4xx=y or CONFIG_BOOKE=y.
> > Otherwise, reference dar. This makes code more clear.
> >
> > Signed-off-by: Xiongwei Song <sxwjean@gmail.com>
> > ---
> > arch/powerpc/include/asm/ptrace.h | 5 ++++-
> > arch/powerpc/kernel/process.c | 2 +-
> > arch/powerpc/kernel/ptrace/ptrace.c | 2 ++
> > 3 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> > index c252d04b1206..fa725e3238c2 100644
> > --- a/arch/powerpc/include/asm/ptrace.h
> > +++ b/arch/powerpc/include/asm/ptrace.h
> > @@ -43,7 +43,10 @@ struct pt_regs
> > unsigned long mq;
> > #endif
> > unsigned long trap;
> > - unsigned long dar;
> > + union {
> > + unsigned long dar;
> > + unsigned long dear;
> > + };
> > union {
> > unsigned long dsisr;
> > unsigned long esr;
> > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> > index f74af8f9133c..50436b52c213 100644
> > --- a/arch/powerpc/kernel/process.c
> > +++ b/arch/powerpc/kernel/process.c
> > @@ -1499,7 +1499,7 @@ static void __show_regs(struct pt_regs *regs)
> > trap == INTERRUPT_DATA_STORAGE ||
> > trap == INTERRUPT_ALIGNMENT) {
> > if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
> > - pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar, regs->esr);
> > + pr_cont("DEAR: "REG" ESR: "REG" ", regs->dear, regs->esr);
> > else
> > pr_cont("DAR: "REG" DSISR: %08lx ", regs->dar, regs->dsisr);
> > }
> > diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
> > index a222fd4d6334..7c7093c17c45 100644
> > --- a/arch/powerpc/kernel/ptrace/ptrace.c
> > +++ b/arch/powerpc/kernel/ptrace/ptrace.c
> > @@ -373,6 +373,8 @@ void __init pt_regs_check(void)
> > offsetof(struct user_pt_regs, trap));
> > BUILD_BUG_ON(offsetof(struct pt_regs, dar) !=
> > offsetof(struct user_pt_regs, dar));
> > + BUILD_BUG_ON(offsetof(struct pt_regs, dear) !=
> > + offsetof(struct user_pt_regs, dar));
>
> dar and dear are the same, so checking the same thing a second time is pointless.
Same reply as the patch 1.
Regards,
Xiongwei
>
> > BUILD_BUG_ON(offsetof(struct pt_regs, dsisr) !=
> > offsetof(struct user_pt_regs, dsisr));
> > BUILD_BUG_ON(offsetof(struct pt_regs, esr) !=
> >
^ permalink raw reply
* Re: [PATCH v2 1/4] powerpc: Optimize register usage for esr register
From: Xiongwei Song @ 2021-08-10 8:33 UTC (permalink / raw)
To: Christophe Leroy
Cc: ravi.bangoria, Xiongwei Song, oleg, npiggin,
Linux Kernel Mailing List, efremov, Paul Mackerras, aneesh.kumar,
PowerPC
In-Reply-To: <127de0c7-5e30-6797-1e72-6414d4a60119@csgroup.eu>
On Sat, Aug 7, 2021 at 2:57 PM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 07/08/2021 à 03:02, sxwjean@me.com a écrit :
> > From: Xiongwei Song <sxwjean@gmail.com>
> >
> > Create an anonymous union for dsisr and esr regsiters, we can reference
> > esr to get the exception detail when CONFIG_4xx=y or CONFIG_BOOKE=y.
> > Otherwise, reference dsisr. This makes code more clear.
> >
> > Signed-off-by: Xiongwei Song <sxwjean@gmail.com>
> > ---
> > arch/powerpc/include/asm/ptrace.h | 5 ++++-
> > arch/powerpc/kernel/process.c | 2 +-
> > arch/powerpc/kernel/ptrace/ptrace.c | 2 ++
> > arch/powerpc/kernel/traps.c | 2 +-
> > arch/powerpc/platforms/44x/machine_check.c | 4 ++--
> > arch/powerpc/platforms/4xx/machine_check.c | 2 +-
> > 6 files changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> > index 3e5d470a6155..c252d04b1206 100644
> > --- a/arch/powerpc/include/asm/ptrace.h
> > +++ b/arch/powerpc/include/asm/ptrace.h
> > @@ -44,7 +44,10 @@ struct pt_regs
> > #endif
> > unsigned long trap;
> > unsigned long dar;
> > - unsigned long dsisr;
> > + union {
> > + unsigned long dsisr;
> > + unsigned long esr;
> > + };
> > unsigned long result;
> > };
> > };
> > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> > index 185beb290580..f74af8f9133c 100644
> > --- a/arch/powerpc/kernel/process.c
> > +++ b/arch/powerpc/kernel/process.c
> > @@ -1499,7 +1499,7 @@ static void __show_regs(struct pt_regs *regs)
> > trap == INTERRUPT_DATA_STORAGE ||
> > trap == INTERRUPT_ALIGNMENT) {
> > if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
> > - pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar, regs->dsisr);
> > + pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar, regs->esr);
> > else
> > pr_cont("DAR: "REG" DSISR: %08lx ", regs->dar, regs->dsisr);
> > }
> > diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
> > index 0a0a33eb0d28..a222fd4d6334 100644
> > --- a/arch/powerpc/kernel/ptrace/ptrace.c
> > +++ b/arch/powerpc/kernel/ptrace/ptrace.c
> > @@ -375,6 +375,8 @@ void __init pt_regs_check(void)
> > offsetof(struct user_pt_regs, dar));
> > BUILD_BUG_ON(offsetof(struct pt_regs, dsisr) !=
> > offsetof(struct user_pt_regs, dsisr));
> > + BUILD_BUG_ON(offsetof(struct pt_regs, esr) !=
> > + offsetof(struct user_pt_regs, dsisr));
>
> esr and dsisr are the same, so checking the same thing a second time is pointless.
Hmm...it's better to check if the changes on pt_regs structure is
expected I think.
Regards,
Xiongwei
>
> > BUILD_BUG_ON(offsetof(struct pt_regs, result) !=
> > offsetof(struct user_pt_regs, result));
> >
> > diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> > index dfbce527c98e..2164f5705a0b 100644
> > --- a/arch/powerpc/kernel/traps.c
> > +++ b/arch/powerpc/kernel/traps.c
> > @@ -562,7 +562,7 @@ static inline int check_io_access(struct pt_regs *regs)
> > #ifdef CONFIG_PPC_ADV_DEBUG_REGS
> > /* On 4xx, the reason for the machine check or program exception
> > is in the ESR. */
> > -#define get_reason(regs) ((regs)->dsisr)
> > +#define get_reason(regs) ((regs)->esr)
> > #define REASON_FP ESR_FP
> > #define REASON_ILLEGAL (ESR_PIL | ESR_PUO)
> > #define REASON_PRIVILEGED ESR_PPR
> > diff --git a/arch/powerpc/platforms/44x/machine_check.c b/arch/powerpc/platforms/44x/machine_check.c
> > index a5c898bb9bab..5d19daacd78a 100644
> > --- a/arch/powerpc/platforms/44x/machine_check.c
> > +++ b/arch/powerpc/platforms/44x/machine_check.c
> > @@ -11,7 +11,7 @@
> >
> > int machine_check_440A(struct pt_regs *regs)
> > {
> > - unsigned long reason = regs->dsisr;
> > + unsigned long reason = regs->esr;
> >
> > printk("Machine check in kernel mode.\n");
> > if (reason & ESR_IMCP){
> > @@ -48,7 +48,7 @@ int machine_check_440A(struct pt_regs *regs)
> > #ifdef CONFIG_PPC_47x
> > int machine_check_47x(struct pt_regs *regs)
> > {
> > - unsigned long reason = regs->dsisr;
> > + unsigned long reason = regs->esr;
> > u32 mcsr;
> >
> > printk(KERN_ERR "Machine check in kernel mode.\n");
> > diff --git a/arch/powerpc/platforms/4xx/machine_check.c b/arch/powerpc/platforms/4xx/machine_check.c
> > index a71c29892a91..a905da1d6f41 100644
> > --- a/arch/powerpc/platforms/4xx/machine_check.c
> > +++ b/arch/powerpc/platforms/4xx/machine_check.c
> > @@ -10,7 +10,7 @@
> >
> > int machine_check_4xx(struct pt_regs *regs)
> > {
> > - unsigned long reason = regs->dsisr;
> > + unsigned long reason = regs->esr;
> >
> > if (reason & ESR_IMCP) {
> > printk("Instruction");
> >
^ permalink raw reply
* [bug report] soc: fsl: qe: convert QE interrupt controller to platform_device
From: Dan Carpenter @ 2021-08-10 7:48 UTC (permalink / raw)
To: fido_max; +Cc: linuxppc-dev
Hello Maxim Kochetkov,
The patch be7ecbd240b2: "soc: fsl: qe: convert QE interrupt
controller to platform_device" from Aug 3, 2021, leads to the
following static checker warning:
drivers/soc/fsl/qe/qe_ic.c:438 qe_ic_init()
warn: unsigned 'qe_ic->virq_low' is never less than zero.
drivers/soc/fsl/qe/qe_ic.c
408 static int qe_ic_init(struct platform_device *pdev)
409 {
410 struct device *dev = &pdev->dev;
411 void (*low_handler)(struct irq_desc *desc);
412 void (*high_handler)(struct irq_desc *desc);
413 struct qe_ic *qe_ic;
414 struct resource *res;
415 struct device_node *node = pdev->dev.of_node;
416
417 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
418 if (res == NULL) {
419 dev_err(dev, "no memory resource defined\n");
420 return -ENODEV;
421 }
422
423 qe_ic = devm_kzalloc(dev, sizeof(*qe_ic), GFP_KERNEL);
424 if (qe_ic == NULL)
425 return -ENOMEM;
426
427 qe_ic->regs = devm_ioremap(dev, res->start, resource_size(res));
428 if (qe_ic->regs == NULL) {
429 dev_err(dev, "failed to ioremap() registers\n");
430 return -ENODEV;
431 }
432
433 qe_ic->hc_irq = qe_ic_irq_chip;
434
435 qe_ic->virq_high = platform_get_irq(pdev, 0);
436 qe_ic->virq_low = platform_get_irq(pdev, 1);
437
--> 438 if (qe_ic->virq_low < 0) {
439 return -ENODEV;
440 }
Unsigned can't be less than zero. It's weird that it doesn't check
qe_ic->virq_high as well. Also remove the curly braces to make
checkpatch happy?
441
442 if (qe_ic->virq_high != qe_ic->virq_low) {
443 low_handler = qe_ic_cascade_low;
444 high_handler = qe_ic_cascade_high;
445 } else {
446 low_handler = qe_ic_cascade_muxed_mpic;
447 high_handler = NULL;
448 }
449
450 qe_ic->irqhost = irq_domain_add_linear(node, NR_QE_IC_INTS,
451 &qe_ic_host_ops, qe_ic);
452 if (qe_ic->irqhost == NULL) {
453 dev_err(dev, "failed to add irq domain\n");
454 return -ENODEV;
455 }
456
457 qe_ic_write(qe_ic->regs, QEIC_CICR, 0);
458
459 irq_set_handler_data(qe_ic->virq_low, qe_ic);
460 irq_set_chained_handler(qe_ic->virq_low, low_handler);
461
462 if (qe_ic->virq_high && qe_ic->virq_high != qe_ic->virq_low) {
463 irq_set_handler_data(qe_ic->virq_high, qe_ic);
464 irq_set_chained_handler(qe_ic->virq_high, high_handler);
465 }
466 return 0;
467 }
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] powerpc/book3s64/radix: make tlb_single_page_flush_ceiling a debugfs entry
From: Christophe Leroy @ 2021-08-10 5:17 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev, mpe
In-Reply-To: <20210810045307.11892-1-aneesh.kumar@linux.ibm.com>
Le 10/08/2021 à 06:53, Aneesh Kumar K.V a écrit :
> Similar to x86/s390 add a debugfs file to tune tlb_single_page_flush_ceiling.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> arch/powerpc/mm/book3s64/radix_tlb.c | 48 ++++++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
> index aefc100d79a7..5cca0fe130e7 100644
> --- a/arch/powerpc/mm/book3s64/radix_tlb.c
> +++ b/arch/powerpc/mm/book3s64/radix_tlb.c
> @@ -17,6 +17,7 @@
> #include <asm/trace.h>
> #include <asm/cputhreads.h>
> #include <asm/plpar_wrappers.h>
> +#include <asm/debugfs.h>
>
> #include "internal.h"
>
> @@ -1524,3 +1525,50 @@ void do_h_rpt_invalidate_prt(unsigned long pid, unsigned long lpid,
> EXPORT_SYMBOL_GPL(do_h_rpt_invalidate_prt);
>
> #endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
> +
> +static ssize_t tlbflush_read_file(struct file *file, char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> + char buf[32];
> + unsigned int len;
> +
> + len = sprintf(buf, "%ld\n", tlb_single_page_flush_ceiling);
> + return simple_read_from_buffer(user_buf, count, ppos, buf, len);
> +}
> +
> +static ssize_t tlbflush_write_file(struct file *file,
> + const char __user *user_buf, size_t count, loff_t *ppos)
> +{
> + char buf[32];
> + ssize_t len;
> + int ceiling;
> +
> + len = min(count, sizeof(buf) - 1);
> + if (copy_from_user(buf, user_buf, len))
> + return -EFAULT;
> +
> + buf[len] = '\0';
> + if (kstrtoint(buf, 0, &ceiling))
> + return -EINVAL;
> +
> + if (ceiling < 0)
> + return -EINVAL;
> +
> + tlb_single_page_flush_ceiling = ceiling;
> + return count;
> +}
> +
> +static const struct file_operations fops_tlbflush = {
> + .read = tlbflush_read_file,
> + .write = tlbflush_write_file,
> + .llseek = default_llseek,
> +};
> +
> +static int __init create_tlb_single_page_flush_ceiling(void)
> +{
> + debugfs_create_file("tlb_single_page_flush_ceiling", S_IRUSR | S_IWUSR,
> + powerpc_debugfs_root, NULL, &fops_tlbflush);
Could you just use debugfs_create_u32() instead of re-implementing simple read and write ?
Or at least use DEFINE_DEBUGFS_ATTRIBUTE() if you need something a bit more elaborated ?
> + return 0;
> +}
> +late_initcall(create_tlb_single_page_flush_ceiling);
> +
>
^ permalink raw reply
* [PATCH] powerpc: rename powerpc_debugfs_root to arch_debugfs_dir
From: Aneesh Kumar K.V @ 2021-08-10 4:54 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V
No functional change in this patch. arch_debugfs_dir is the generic kernel
name declared in linux/debugfs.h for arch-specific debugfs directory.
Architectures like x86/s390 already use the name. Rename powerpc
specific powerpc_debugfs_root to arch_debugfs_dir.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/debugfs.h | 13 -------------
arch/powerpc/kernel/Makefile | 3 ++-
arch/powerpc/kernel/dawr.c | 3 +--
arch/powerpc/kernel/eeh.c | 16 ++++++++--------
arch/powerpc/kernel/eeh_cache.c | 4 ++--
arch/powerpc/kernel/fadump.c | 4 ++--
arch/powerpc/kernel/hw_breakpoint.c | 1 -
arch/powerpc/kernel/kdebugfs.c | 14 ++++++++++++++
arch/powerpc/kernel/security.c | 16 ++++++++--------
arch/powerpc/kernel/setup-common.c | 13 -------------
arch/powerpc/kernel/setup_64.c | 1 -
arch/powerpc/kernel/traps.c | 4 ++--
arch/powerpc/kvm/book3s_xics.c | 6 +++---
arch/powerpc/kvm/book3s_xive.c | 3 +--
arch/powerpc/kvm/book3s_xive_native.c | 3 +--
arch/powerpc/mm/book3s64/hash_utils.c | 4 ++--
arch/powerpc/mm/book3s64/pgtable.c | 4 ++--
arch/powerpc/mm/book3s64/radix_tlb.c | 4 ++--
arch/powerpc/mm/ptdump/bats.c | 4 ++--
arch/powerpc/mm/ptdump/segment_regs.c | 4 ++--
arch/powerpc/platforms/cell/axon_msi.c | 4 ++--
arch/powerpc/platforms/powernv/memtrace.c | 3 +--
arch/powerpc/platforms/powernv/opal-imc.c | 4 ++--
arch/powerpc/platforms/powernv/opal-lpc.c | 4 ++--
arch/powerpc/platforms/powernv/opal-xscom.c | 4 ++--
arch/powerpc/platforms/powernv/pci-ioda.c | 4 ++--
arch/powerpc/platforms/pseries/dtl.c | 4 ++--
arch/powerpc/platforms/pseries/lpar.c | 5 +++--
arch/powerpc/sysdev/xive/common.c | 3 +--
arch/powerpc/xmon/xmon.c | 6 +++---
30 files changed, 74 insertions(+), 91 deletions(-)
delete mode 100644 arch/powerpc/include/asm/debugfs.h
create mode 100644 arch/powerpc/kernel/kdebugfs.c
diff --git a/arch/powerpc/include/asm/debugfs.h b/arch/powerpc/include/asm/debugfs.h
deleted file mode 100644
index 2c5c48571d75..000000000000
--- a/arch/powerpc/include/asm/debugfs.h
+++ /dev/null
@@ -1,13 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-#ifndef _ASM_POWERPC_DEBUGFS_H
-#define _ASM_POWERPC_DEBUGFS_H
-
-/*
- * Copyright 2017, Michael Ellerman, IBM Corporation.
- */
-
-#include <linux/debugfs.h>
-
-extern struct dentry *powerpc_debugfs_root;
-
-#endif /* _ASM_POWERPC_DEBUGFS_H */
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index f66b63e81c3b..7be36c1e1db6 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -46,7 +46,8 @@ obj-y := cputable.o syscalls.o \
prom.o traps.o setup-common.o \
udbg.o misc.o io.o misc_$(BITS).o \
of_platform.o prom_parse.o firmware.o \
- hw_breakpoint_constraints.o interrupt.o
+ hw_breakpoint_constraints.o interrupt.o \
+ kdebugfs.o
obj-y += ptrace/
obj-$(CONFIG_PPC64) += setup_64.o \
paca.o nvram_64.o note.o
diff --git a/arch/powerpc/kernel/dawr.c b/arch/powerpc/kernel/dawr.c
index cdc2dccb987d..64e423d2fe0f 100644
--- a/arch/powerpc/kernel/dawr.c
+++ b/arch/powerpc/kernel/dawr.c
@@ -9,7 +9,6 @@
#include <linux/export.h>
#include <linux/fs.h>
#include <linux/debugfs.h>
-#include <asm/debugfs.h>
#include <asm/machdep.h>
#include <asm/hvcall.h>
@@ -101,7 +100,7 @@ static int __init dawr_force_setup(void)
if (PVR_VER(mfspr(SPRN_PVR)) == PVR_POWER9) {
/* Turn DAWR off by default, but allow admin to turn it on */
debugfs_create_file_unsafe("dawr_enable_dangerous", 0600,
- powerpc_debugfs_root,
+ arch_debugfs_dir,
&dawr_force_enable,
&dawr_enable_fops);
}
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 3bbdcc86d01b..e9b597ed423c 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -21,9 +21,9 @@
#include <linux/spinlock.h>
#include <linux/export.h>
#include <linux/of.h>
+#include <linux/debugfs.h>
#include <linux/atomic.h>
-#include <asm/debugfs.h>
#include <asm/eeh.h>
#include <asm/eeh_event.h>
#include <asm/io.h>
@@ -1901,24 +1901,24 @@ static int __init eeh_init_proc(void)
proc_create_single("powerpc/eeh", 0, NULL, proc_eeh_show);
#ifdef CONFIG_DEBUG_FS
debugfs_create_file_unsafe("eeh_enable", 0600,
- powerpc_debugfs_root, NULL,
+ arch_debugfs_dir, NULL,
&eeh_enable_dbgfs_ops);
debugfs_create_u32("eeh_max_freezes", 0600,
- powerpc_debugfs_root, &eeh_max_freezes);
+ arch_debugfs_dir, &eeh_max_freezes);
debugfs_create_bool("eeh_disable_recovery", 0600,
- powerpc_debugfs_root,
+ arch_debugfs_dir,
&eeh_debugfs_no_recover);
debugfs_create_file_unsafe("eeh_dev_check", 0600,
- powerpc_debugfs_root, NULL,
+ arch_debugfs_dir, NULL,
&eeh_dev_check_fops);
debugfs_create_file_unsafe("eeh_dev_break", 0600,
- powerpc_debugfs_root, NULL,
+ arch_debugfs_dir, NULL,
&eeh_dev_break_fops);
debugfs_create_file_unsafe("eeh_force_recover", 0600,
- powerpc_debugfs_root, NULL,
+ arch_debugfs_dir, NULL,
&eeh_force_recover_fops);
debugfs_create_file_unsafe("eeh_dev_can_recover", 0600,
- powerpc_debugfs_root, NULL,
+ arch_debugfs_dir, NULL,
&eeh_dev_can_recover_fops);
eeh_cache_debugfs_init();
#endif
diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
index bf3270426d82..9bdaaf7fddc9 100644
--- a/arch/powerpc/kernel/eeh_cache.c
+++ b/arch/powerpc/kernel/eeh_cache.c
@@ -12,8 +12,8 @@
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/atomic.h>
+#include <linux/debugfs.h>
#include <asm/pci-bridge.h>
-#include <asm/debugfs.h>
#include <asm/ppc-pci.h>
@@ -283,6 +283,6 @@ DEFINE_SHOW_ATTRIBUTE(eeh_addr_cache);
void eeh_cache_debugfs_init(void)
{
debugfs_create_file_unsafe("eeh_address_cache", 0400,
- powerpc_debugfs_root, NULL,
+ arch_debugfs_dir, NULL,
&eeh_addr_cache_fops);
}
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index b990075285f5..b7ceb041743c 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -24,8 +24,8 @@
#include <linux/slab.h>
#include <linux/cma.h>
#include <linux/hugetlb.h>
+#include <linux/debugfs.h>
-#include <asm/debugfs.h>
#include <asm/page.h>
#include <asm/prom.h>
#include <asm/fadump.h>
@@ -1557,7 +1557,7 @@ static void fadump_init_files(void)
return;
}
- debugfs_create_file("fadump_region", 0444, powerpc_debugfs_root, NULL,
+ debugfs_create_file("fadump_region", 0444, arch_debugfs_dir, NULL,
&fadump_region_fops);
if (fw_dump.dump_active) {
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 21a638aff72f..91a3be14808b 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -22,7 +22,6 @@
#include <asm/processor.h>
#include <asm/sstep.h>
#include <asm/debug.h>
-#include <asm/debugfs.h>
#include <asm/hvcall.h>
#include <asm/inst.h>
#include <linux/uaccess.h>
diff --git a/arch/powerpc/kernel/kdebugfs.c b/arch/powerpc/kernel/kdebugfs.c
new file mode 100644
index 000000000000..36d3124d5a8b
--- /dev/null
+++ b/arch/powerpc/kernel/kdebugfs.c
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/debugfs.h>
+#include <linux/export.h>
+#include <linux/init.h>
+
+struct dentry *arch_debugfs_dir;
+EXPORT_SYMBOL(arch_debugfs_dir);
+
+static int __init arch_kdebugfs_init(void)
+{
+ arch_debugfs_dir = debugfs_create_dir("powerpc", NULL);
+ return 0;
+}
+arch_initcall(arch_kdebugfs_init);
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index cc51fa52e783..1a998490fe60 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -11,10 +11,10 @@
#include <linux/nospec.h>
#include <linux/prctl.h>
#include <linux/seq_buf.h>
+#include <linux/debugfs.h>
#include <asm/asm-prototypes.h>
#include <asm/code-patching.h>
-#include <asm/debugfs.h>
#include <asm/security_features.h>
#include <asm/setup.h>
#include <asm/inst.h>
@@ -106,7 +106,7 @@ DEFINE_DEBUGFS_ATTRIBUTE(fops_barrier_nospec, barrier_nospec_get,
static __init int barrier_nospec_debugfs_init(void)
{
debugfs_create_file_unsafe("barrier_nospec", 0600,
- powerpc_debugfs_root, NULL,
+ arch_debugfs_dir, NULL,
&fops_barrier_nospec);
return 0;
}
@@ -114,7 +114,7 @@ device_initcall(barrier_nospec_debugfs_init);
static __init int security_feature_debugfs_init(void)
{
- debugfs_create_x64("security_features", 0400, powerpc_debugfs_root,
+ debugfs_create_x64("security_features", 0400, arch_debugfs_dir,
&powerpc_security_features);
return 0;
}
@@ -420,7 +420,7 @@ DEFINE_DEBUGFS_ATTRIBUTE(fops_stf_barrier, stf_barrier_get, stf_barrier_set,
static __init int stf_barrier_debugfs_init(void)
{
- debugfs_create_file_unsafe("stf_barrier", 0600, powerpc_debugfs_root,
+ debugfs_create_file_unsafe("stf_barrier", 0600, arch_debugfs_dir,
NULL, &fops_stf_barrier);
return 0;
}
@@ -748,7 +748,7 @@ DEFINE_DEBUGFS_ATTRIBUTE(fops_count_cache_flush, count_cache_flush_get,
static __init int count_cache_flush_debugfs_init(void)
{
debugfs_create_file_unsafe("count_cache_flush", 0600,
- powerpc_debugfs_root, NULL,
+ arch_debugfs_dir, NULL,
&fops_count_cache_flush);
return 0;
}
@@ -834,9 +834,9 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_uaccess_flush, uaccess_flush_get, uaccess_flush_set
static __init int rfi_flush_debugfs_init(void)
{
- debugfs_create_file("rfi_flush", 0600, powerpc_debugfs_root, NULL, &fops_rfi_flush);
- debugfs_create_file("entry_flush", 0600, powerpc_debugfs_root, NULL, &fops_entry_flush);
- debugfs_create_file("uaccess_flush", 0600, powerpc_debugfs_root, NULL, &fops_uaccess_flush);
+ debugfs_create_file("rfi_flush", 0600, arch_debugfs_dir, NULL, &fops_rfi_flush);
+ debugfs_create_file("entry_flush", 0600, arch_debugfs_dir, NULL, &fops_entry_flush);
+ debugfs_create_file("uaccess_flush", 0600, arch_debugfs_dir, NULL, &fops_uaccess_flush);
return 0;
}
device_initcall(rfi_flush_debugfs_init);
diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index aa9c2d01424a..b1e43b69a559 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -33,7 +33,6 @@
#include <linux/of_platform.h>
#include <linux/hugetlb.h>
#include <linux/pgtable.h>
-#include <asm/debugfs.h>
#include <asm/io.h>
#include <asm/paca.h>
#include <asm/prom.h>
@@ -773,18 +772,6 @@ static int __init check_cache_coherency(void)
late_initcall(check_cache_coherency);
#endif /* CONFIG_CHECK_CACHE_COHERENCY */
-#ifdef CONFIG_DEBUG_FS
-struct dentry *powerpc_debugfs_root;
-EXPORT_SYMBOL(powerpc_debugfs_root);
-
-static int powerpc_debugfs_init(void)
-{
- powerpc_debugfs_root = debugfs_create_dir("powerpc", NULL);
- return 0;
-}
-arch_initcall(powerpc_debugfs_init);
-#endif
-
void ppc_printk_progress(char *s, unsigned short hex)
{
pr_info("%s\n", s);
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 1ff258f6c76c..eaa79a0996d1 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -32,7 +32,6 @@
#include <linux/nmi.h>
#include <linux/pgtable.h>
-#include <asm/debugfs.h>
#include <asm/kvm_guest.h>
#include <asm/io.h>
#include <asm/kdump.h>
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index dfbce527c98e..c8f648727d36 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -37,10 +37,10 @@
#include <linux/smp.h>
#include <linux/console.h>
#include <linux/kmsg_dump.h>
+#include <linux/debugfs.h>
#include <asm/emulated_ops.h>
#include <linux/uaccess.h>
-#include <asm/debugfs.h>
#include <asm/interrupt.h>
#include <asm/io.h>
#include <asm/machdep.h>
@@ -2267,7 +2267,7 @@ static int __init ppc_warn_emulated_init(void)
struct ppc_emulated_entry *entries = (void *)&ppc_emulated;
dir = debugfs_create_dir("emulated_instructions",
- powerpc_debugfs_root);
+ arch_debugfs_dir);
debugfs_create_u32("do_warn", 0644, dir, &ppc_warn_emulated);
diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c
index 303e3cb096db..ebd5d920de8c 100644
--- a/arch/powerpc/kvm/book3s_xics.c
+++ b/arch/powerpc/kvm/book3s_xics.c
@@ -10,13 +10,13 @@
#include <linux/gfp.h>
#include <linux/anon_inodes.h>
#include <linux/spinlock.h>
-
+#include <linux/debugfs.h>
#include <linux/uaccess.h>
+
#include <asm/kvm_book3s.h>
#include <asm/kvm_ppc.h>
#include <asm/hvcall.h>
#include <asm/xics.h>
-#include <asm/debugfs.h>
#include <asm/time.h>
#include <linux/seq_file.h>
@@ -1024,7 +1024,7 @@ static void xics_debugfs_init(struct kvmppc_xics *xics)
return;
}
- xics->dentry = debugfs_create_file(name, 0444, powerpc_debugfs_root,
+ xics->dentry = debugfs_create_file(name, 0444, arch_debugfs_dir,
xics, &xics_debug_fops);
pr_debug("%s: created %s\n", __func__, name);
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index 8cfab3547494..19adbb1dda04 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -22,7 +22,6 @@
#include <asm/xive.h>
#include <asm/xive-regs.h>
#include <asm/debug.h>
-#include <asm/debugfs.h>
#include <asm/time.h>
#include <asm/opal.h>
@@ -2329,7 +2328,7 @@ static void xive_debugfs_init(struct kvmppc_xive *xive)
return;
}
- xive->dentry = debugfs_create_file(name, S_IRUGO, powerpc_debugfs_root,
+ xive->dentry = debugfs_create_file(name, S_IRUGO, arch_debugfs_dir,
xive, &xive_debug_fops);
pr_debug("%s: created %s\n", __func__, name);
diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
index 573ecaab3597..4ce800c7454f 100644
--- a/arch/powerpc/kvm/book3s_xive_native.c
+++ b/arch/powerpc/kvm/book3s_xive_native.c
@@ -20,7 +20,6 @@
#include <asm/xive.h>
#include <asm/xive-regs.h>
#include <asm/debug.h>
-#include <asm/debugfs.h>
#include <asm/opal.h>
#include <linux/debugfs.h>
@@ -1257,7 +1256,7 @@ static void xive_native_debugfs_init(struct kvmppc_xive *xive)
return;
}
- xive->dentry = debugfs_create_file(name, 0444, powerpc_debugfs_root,
+ xive->dentry = debugfs_create_file(name, 0444, arch_debugfs_dir,
xive, &xive_native_debug_fops);
pr_debug("%s: created %s\n", __func__, name);
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index ac5720371c0d..c145776d3ae5 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -36,8 +36,8 @@
#include <linux/hugetlb.h>
#include <linux/cpu.h>
#include <linux/pgtable.h>
+#include <linux/debugfs.h>
-#include <asm/debugfs.h>
#include <asm/interrupt.h>
#include <asm/processor.h>
#include <asm/mmu.h>
@@ -2072,7 +2072,7 @@ DEFINE_DEBUGFS_ATTRIBUTE(fops_hpt_order, hpt_order_get, hpt_order_set, "%llu\n")
static int __init hash64_debugfs(void)
{
- debugfs_create_file("hpt_order", 0600, powerpc_debugfs_root, NULL,
+ debugfs_create_file("hpt_order", 0600, arch_debugfs_dir, NULL,
&fops_hpt_order);
return 0;
}
diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
index 9ffa65074cb0..7011a8f16d7a 100644
--- a/arch/powerpc/mm/book3s64/pgtable.c
+++ b/arch/powerpc/mm/book3s64/pgtable.c
@@ -6,9 +6,9 @@
#include <linux/sched.h>
#include <linux/mm_types.h>
#include <linux/memblock.h>
+#include <linux/debugfs.h>
#include <misc/cxl-base.h>
-#include <asm/debugfs.h>
#include <asm/pgalloc.h>
#include <asm/tlb.h>
#include <asm/trace.h>
@@ -520,7 +520,7 @@ static int __init pgtable_debugfs_setup(void)
* invalidated as expected.
*/
debugfs_create_bool("tlbie_enabled", 0600,
- powerpc_debugfs_root,
+ arch_debugfs_dir,
&tlbie_enabled);
return 0;
diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
index 5cca0fe130e7..05ee64a1cf4a 100644
--- a/arch/powerpc/mm/book3s64/radix_tlb.c
+++ b/arch/powerpc/mm/book3s64/radix_tlb.c
@@ -10,6 +10,7 @@
#include <linux/memblock.h>
#include <linux/mmu_context.h>
#include <linux/sched/mm.h>
+#include <linux/debugfs.h>
#include <asm/ppc-opcode.h>
#include <asm/tlb.h>
@@ -17,7 +18,6 @@
#include <asm/trace.h>
#include <asm/cputhreads.h>
#include <asm/plpar_wrappers.h>
-#include <asm/debugfs.h>
#include "internal.h"
@@ -1567,7 +1567,7 @@ static const struct file_operations fops_tlbflush = {
static int __init create_tlb_single_page_flush_ceiling(void)
{
debugfs_create_file("tlb_single_page_flush_ceiling", S_IRUSR | S_IWUSR,
- powerpc_debugfs_root, NULL, &fops_tlbflush);
+ arch_debugfs_dir, NULL, &fops_tlbflush);
return 0;
}
late_initcall(create_tlb_single_page_flush_ceiling);
diff --git a/arch/powerpc/mm/ptdump/bats.c b/arch/powerpc/mm/ptdump/bats.c
index c4c628b03cf8..8bf7383fb26c 100644
--- a/arch/powerpc/mm/ptdump/bats.c
+++ b/arch/powerpc/mm/ptdump/bats.c
@@ -7,7 +7,7 @@
*/
#include <linux/pgtable.h>
-#include <asm/debugfs.h>
+#include <linux/debugfs.h>
#include <asm/cpu_has_feature.h>
#include "ptdump.h"
@@ -103,7 +103,7 @@ static const struct file_operations bats_fops = {
static int __init bats_init(void)
{
debugfs_create_file("block_address_translation", 0400,
- powerpc_debugfs_root, NULL, &bats_fops);
+ arch_debugfs_dir, NULL, &bats_fops);
return 0;
}
device_initcall(bats_init);
diff --git a/arch/powerpc/mm/ptdump/segment_regs.c b/arch/powerpc/mm/ptdump/segment_regs.c
index 565048a0c9be..9223dfb85c51 100644
--- a/arch/powerpc/mm/ptdump/segment_regs.c
+++ b/arch/powerpc/mm/ptdump/segment_regs.c
@@ -6,7 +6,7 @@
* This dumps the content of Segment Registers
*/
-#include <asm/debugfs.h>
+#include <linux/debugfs.h>
static void seg_show(struct seq_file *m, int i)
{
@@ -55,7 +55,7 @@ static const struct file_operations sr_fops = {
static int __init sr_init(void)
{
- debugfs_create_file("segment_registers", 0400, powerpc_debugfs_root,
+ debugfs_create_file("segment_registers", 0400, arch_debugfs_dir,
NULL, &sr_fops);
return 0;
}
diff --git a/arch/powerpc/platforms/cell/axon_msi.c b/arch/powerpc/platforms/cell/axon_msi.c
index ca2555b8a0c2..82335e364c44 100644
--- a/arch/powerpc/platforms/cell/axon_msi.c
+++ b/arch/powerpc/platforms/cell/axon_msi.c
@@ -12,8 +12,8 @@
#include <linux/export.h>
#include <linux/of_platform.h>
#include <linux/slab.h>
+#include <linux/debugfs.h>
-#include <asm/debugfs.h>
#include <asm/dcr.h>
#include <asm/machdep.h>
#include <asm/prom.h>
@@ -480,6 +480,6 @@ void axon_msi_debug_setup(struct device_node *dn, struct axon_msic *msic)
snprintf(name, sizeof(name), "msic_%d", of_node_to_nid(dn));
- debugfs_create_file(name, 0600, powerpc_debugfs_root, msic, &fops_msic);
+ debugfs_create_file(name, 0600, arch_debugfs_dir, msic, &fops_msic);
}
#endif /* DEBUG */
diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
index 537a4daed614..877720c64515 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -18,7 +18,6 @@
#include <linux/memory_hotplug.h>
#include <linux/numa.h>
#include <asm/machdep.h>
-#include <asm/debugfs.h>
#include <asm/cacheflush.h>
/* This enables us to keep track of the memory removed from each node. */
@@ -330,7 +329,7 @@ DEFINE_SIMPLE_ATTRIBUTE(memtrace_init_fops, memtrace_enable_get,
static int memtrace_init(void)
{
memtrace_debugfs_dir = debugfs_create_dir("memtrace",
- powerpc_debugfs_root);
+ arch_debugfs_dir);
debugfs_create_file("enable", 0600, memtrace_debugfs_dir,
NULL, &memtrace_init_fops);
diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
index 7824cc364bc4..63e4738baf33 100644
--- a/arch/powerpc/platforms/powernv/opal-imc.c
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -13,11 +13,11 @@
#include <linux/of_address.h>
#include <linux/of_platform.h>
#include <linux/crash_dump.h>
+#include <linux/debugfs.h>
#include <asm/opal.h>
#include <asm/io.h>
#include <asm/imc-pmu.h>
#include <asm/cputhreads.h>
-#include <asm/debugfs.h>
static struct dentry *imc_debugfs_parent;
@@ -56,7 +56,7 @@ static void export_imc_mode_and_cmd(struct device_node *node,
u32 cb_offset;
struct imc_mem_info *ptr = pmu_ptr->mem_info;
- imc_debugfs_parent = debugfs_create_dir("imc", powerpc_debugfs_root);
+ imc_debugfs_parent = debugfs_create_dir("imc", arch_debugfs_dir);
if (of_property_read_u32(node, "cb_offset", &cb_offset))
cb_offset = IMC_CNTL_BLK_OFFSET;
diff --git a/arch/powerpc/platforms/powernv/opal-lpc.c b/arch/powerpc/platforms/powernv/opal-lpc.c
index 608569082ba0..1e5d51db40f8 100644
--- a/arch/powerpc/platforms/powernv/opal-lpc.c
+++ b/arch/powerpc/platforms/powernv/opal-lpc.c
@@ -10,13 +10,13 @@
#include <linux/bug.h>
#include <linux/io.h>
#include <linux/slab.h>
+#include <linux/debugfs.h>
#include <asm/machdep.h>
#include <asm/firmware.h>
#include <asm/opal.h>
#include <asm/prom.h>
#include <linux/uaccess.h>
-#include <asm/debugfs.h>
#include <asm/isa-bridge.h>
static int opal_lpc_chip_id = -1;
@@ -371,7 +371,7 @@ static int opal_lpc_init_debugfs(void)
if (opal_lpc_chip_id < 0)
return -ENODEV;
- root = debugfs_create_dir("lpc", powerpc_debugfs_root);
+ root = debugfs_create_dir("lpc", arch_debugfs_dir);
rc |= opal_lpc_debugfs_create_type(root, "io", OPAL_LPC_IO);
rc |= opal_lpc_debugfs_create_type(root, "mem", OPAL_LPC_MEM);
diff --git a/arch/powerpc/platforms/powernv/opal-xscom.c b/arch/powerpc/platforms/powernv/opal-xscom.c
index fd510d961b8c..6b4eed2ef4fa 100644
--- a/arch/powerpc/platforms/powernv/opal-xscom.c
+++ b/arch/powerpc/platforms/powernv/opal-xscom.c
@@ -14,11 +14,11 @@
#include <linux/gfp.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
+#include <linux/debugfs.h>
#include <asm/machdep.h>
#include <asm/firmware.h>
#include <asm/opal.h>
-#include <asm/debugfs.h>
#include <asm/prom.h>
static u64 opal_scom_unmangle(u64 addr)
@@ -189,7 +189,7 @@ static int scom_debug_init(void)
if (!firmware_has_feature(FW_FEATURE_OPAL))
return 0;
- root = debugfs_create_dir("scom", powerpc_debugfs_root);
+ root = debugfs_create_dir("scom", arch_debugfs_dir);
if (!root)
return -1;
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 7de464679292..eb4897f499ab 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -20,6 +20,7 @@
#include <linux/iommu.h>
#include <linux/rculist.h>
#include <linux/sizes.h>
+#include <linux/debugfs.h>
#include <asm/sections.h>
#include <asm/io.h>
@@ -32,7 +33,6 @@
#include <asm/iommu.h>
#include <asm/tce.h>
#include <asm/xics.h>
-#include <asm/debugfs.h>
#include <asm/firmware.h>
#include <asm/pnv-pci.h>
#include <asm/mmzone.h>
@@ -2259,7 +2259,7 @@ static void pnv_pci_ioda_create_dbgfs(void)
phb = hose->private_data;
sprintf(name, "PCI%04x", hose->global_number);
- phb->dbgfs = debugfs_create_dir(name, powerpc_debugfs_root);
+ phb->dbgfs = debugfs_create_dir(name, arch_debugfs_dir);
debugfs_create_file_unsafe("dump_diag_regs", 0200, phb->dbgfs,
phb, &pnv_pci_diag_data_fops);
diff --git a/arch/powerpc/platforms/pseries/dtl.c b/arch/powerpc/platforms/pseries/dtl.c
index 982f069e4c31..352af5b14a0f 100644
--- a/arch/powerpc/platforms/pseries/dtl.c
+++ b/arch/powerpc/platforms/pseries/dtl.c
@@ -11,10 +11,10 @@
#include <linux/spinlock.h>
#include <asm/smp.h>
#include <linux/uaccess.h>
+#include <linux/debugfs.h>
#include <asm/firmware.h>
#include <asm/dtl.h>
#include <asm/lppaca.h>
-#include <asm/debugfs.h>
#include <asm/plpar_wrappers.h>
#include <asm/machdep.h>
@@ -338,7 +338,7 @@ static int dtl_init(void)
/* set up common debugfs structure */
- dtl_dir = debugfs_create_dir("dtl", powerpc_debugfs_root);
+ dtl_dir = debugfs_create_dir("dtl", arch_debugfs_dir);
debugfs_create_x8("dtl_event_mask", 0600, dtl_dir, &dtl_event_mask);
debugfs_create_u32("dtl_buf_entries", 0400, dtl_dir, &dtl_buf_entries);
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index dab356e3ff87..2c1d66b5fde3 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -22,6 +22,8 @@
#include <linux/workqueue.h>
#include <linux/proc_fs.h>
#include <linux/pgtable.h>
+#include <linux/debugfs.h>
+
#include <asm/processor.h>
#include <asm/mmu.h>
#include <asm/page.h>
@@ -39,7 +41,6 @@
#include <asm/kexec.h>
#include <asm/fadump.h>
#include <asm/asm-prototypes.h>
-#include <asm/debugfs.h>
#include <asm/dtl.h>
#include "pseries.h"
@@ -2016,7 +2017,7 @@ static int __init vpa_debugfs_init(void)
if (!firmware_has_feature(FW_FEATURE_SPLPAR))
return 0;
- vpa_dir = debugfs_create_dir("vpa", powerpc_debugfs_root);
+ vpa_dir = debugfs_create_dir("vpa", arch_debugfs_dir);
/* set up the per-cpu vpa file*/
for_each_possible_cpu(i) {
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index dbdbbc2f1dc5..f85915ca0f98 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -21,7 +21,6 @@
#include <linux/msi.h>
#include <linux/vmalloc.h>
-#include <asm/debugfs.h>
#include <asm/prom.h>
#include <asm/io.h>
#include <asm/smp.h>
@@ -1716,7 +1715,7 @@ DEFINE_SHOW_ATTRIBUTE(xive_core_debug);
int xive_core_debug_init(void)
{
if (xive_enabled())
- debugfs_create_file("xive", 0400, powerpc_debugfs_root,
+ debugfs_create_file("xive", 0400, arch_debugfs_dir,
NULL, &xive_core_debug_fops);
return 0;
}
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index da4d7f225a40..ead460b80905 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -26,8 +26,8 @@
#include <linux/ctype.h>
#include <linux/highmem.h>
#include <linux/security.h>
+#include <linux/debugfs.h>
-#include <asm/debugfs.h>
#include <asm/ptrace.h>
#include <asm/smp.h>
#include <asm/string.h>
@@ -4077,8 +4077,8 @@ DEFINE_SIMPLE_ATTRIBUTE(xmon_dbgfs_ops, xmon_dbgfs_get,
static int __init setup_xmon_dbgfs(void)
{
- debugfs_create_file("xmon", 0600, powerpc_debugfs_root, NULL,
- &xmon_dbgfs_ops);
+ debugfs_create_file("xmon", 0600, arch_debugfs_dir, NULL,
+ &xmon_dbgfs_ops);
return 0;
}
device_initcall(setup_xmon_dbgfs);
--
2.31.1
^ permalink raw reply related
* [PATCH] powerpc/book3s64/radix: make tlb_single_page_flush_ceiling a debugfs entry
From: Aneesh Kumar K.V @ 2021-08-10 4:53 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: Aneesh Kumar K.V
Similar to x86/s390 add a debugfs file to tune tlb_single_page_flush_ceiling.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/mm/book3s64/radix_tlb.c | 48 ++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/arch/powerpc/mm/book3s64/radix_tlb.c b/arch/powerpc/mm/book3s64/radix_tlb.c
index aefc100d79a7..5cca0fe130e7 100644
--- a/arch/powerpc/mm/book3s64/radix_tlb.c
+++ b/arch/powerpc/mm/book3s64/radix_tlb.c
@@ -17,6 +17,7 @@
#include <asm/trace.h>
#include <asm/cputhreads.h>
#include <asm/plpar_wrappers.h>
+#include <asm/debugfs.h>
#include "internal.h"
@@ -1524,3 +1525,50 @@ void do_h_rpt_invalidate_prt(unsigned long pid, unsigned long lpid,
EXPORT_SYMBOL_GPL(do_h_rpt_invalidate_prt);
#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
+
+static ssize_t tlbflush_read_file(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ char buf[32];
+ unsigned int len;
+
+ len = sprintf(buf, "%ld\n", tlb_single_page_flush_ceiling);
+ return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+static ssize_t tlbflush_write_file(struct file *file,
+ const char __user *user_buf, size_t count, loff_t *ppos)
+{
+ char buf[32];
+ ssize_t len;
+ int ceiling;
+
+ len = min(count, sizeof(buf) - 1);
+ if (copy_from_user(buf, user_buf, len))
+ return -EFAULT;
+
+ buf[len] = '\0';
+ if (kstrtoint(buf, 0, &ceiling))
+ return -EINVAL;
+
+ if (ceiling < 0)
+ return -EINVAL;
+
+ tlb_single_page_flush_ceiling = ceiling;
+ return count;
+}
+
+static const struct file_operations fops_tlbflush = {
+ .read = tlbflush_read_file,
+ .write = tlbflush_write_file,
+ .llseek = default_llseek,
+};
+
+static int __init create_tlb_single_page_flush_ceiling(void)
+{
+ debugfs_create_file("tlb_single_page_flush_ceiling", S_IRUSR | S_IWUSR,
+ powerpc_debugfs_root, NULL, &fops_tlbflush);
+ return 0;
+}
+late_initcall(create_tlb_single_page_flush_ceiling);
+
--
2.31.1
^ permalink raw reply related
* Re: [PATCH v7 3/6] powerpc/pseries: Consolidate different NUMA distance update code paths
From: David Gibson @ 2021-08-10 2:40 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: Nathan Lynch, Daniel Henrique Barboza, linuxppc-dev
In-Reply-To: <20210809052434.53978-4-aneesh.kumar@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 11592 bytes --]
On Mon, Aug 09, 2021 at 10:54:31AM +0530, Aneesh Kumar K.V wrote:
> The associativity details of the newly added resourced are collected from
> the hypervisor via "ibm,configure-connector" rtas call. Update the numa
> distance details of the newly added numa node after the above call.
>
> Instead of updating NUMA distance every time we lookup a node id
> from the associativity property, add helpers that can be used
> during boot which does this only once. Also remove the distance
> update from node id lookup helpers.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
It would make review easier if you'd folded the fixes/cleanups from
6/6 in here, rather than having them separate.
> ---
> arch/powerpc/include/asm/topology.h | 2 +
> arch/powerpc/mm/numa.c | 178 +++++++++++++-----
> arch/powerpc/platforms/pseries/hotplug-cpu.c | 2 +
> .../platforms/pseries/hotplug-memory.c | 2 +
> 4 files changed, 138 insertions(+), 46 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
> index e4db64c0e184..a6425a70c37b 100644
> --- a/arch/powerpc/include/asm/topology.h
> +++ b/arch/powerpc/include/asm/topology.h
> @@ -64,6 +64,7 @@ static inline int early_cpu_to_node(int cpu)
> }
>
> int of_drconf_to_nid_single(struct drmem_lmb *lmb);
> +void update_numa_distance(struct device_node *node);
>
> #else
>
> @@ -93,6 +94,7 @@ static inline int of_drconf_to_nid_single(struct drmem_lmb *lmb)
> return first_online_node;
> }
>
> +static inline void update_numa_distance(struct device_node *node) {}
> #endif /* CONFIG_NUMA */
>
> #if defined(CONFIG_NUMA) && defined(CONFIG_PPC_SPLPAR)
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index 368719b14dcc..c695faf67d68 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -208,22 +208,6 @@ int __node_distance(int a, int b)
> }
> EXPORT_SYMBOL(__node_distance);
>
> -static void initialize_distance_lookup_table(int nid,
> - const __be32 *associativity)
> -{
> - int i;
> -
> - if (affinity_form != FORM1_AFFINITY)
> - return;
> -
> - for (i = 0; i < distance_ref_points_depth; i++) {
> - const __be32 *entry;
> -
> - entry = &associativity[be32_to_cpu(distance_ref_points[i]) - 1];
> - distance_lookup_table[nid][i] = of_read_number(entry, 1);
> - }
> -}
> -
> /*
> * Returns nid in the range [0..nr_node_ids], or -1 if no useful NUMA
> * info is found.
> @@ -241,15 +225,6 @@ static int associativity_to_nid(const __be32 *associativity)
> /* POWER4 LPAR uses 0xffff as invalid node */
> if (nid == 0xffff || nid >= nr_node_ids)
> nid = NUMA_NO_NODE;
> -
> - if (nid > 0 &&
> - of_read_number(associativity, 1) >= distance_ref_points_depth) {
> - /*
> - * Skip the length field and send start of associativity array
So.. this highlights a point. In your comments you're treating
"associativity array" as meaning the value including the length plus
the assoc IDs, but this comment you've removed is treating it as just
the assoc IDs. Personally I think things become clearer to talk about
if you treat the canonical form of "associativity array" as just the
assoc IDs (with implicit length), then treat the ibm,associativity
properties as containing (length + associativity array).
> - */
> - initialize_distance_lookup_table(nid, associativity + 1);
> - }
> -
> out:
> return nid;
> }
> @@ -287,6 +262,48 @@ int of_node_to_nid(struct device_node *device)
> }
> EXPORT_SYMBOL(of_node_to_nid);
>
> +static void __initialize_form1_numa_distance(const __be32 *associativity)
> +{
> + int i, nid;
> +
> + if (affinity_form != FORM1_AFFINITY)
> + return;
> +
> + nid = associativity_to_nid(associativity);
> + if (nid != NUMA_NO_NODE) {
> + for (i = 0; i < distance_ref_points_depth; i++) {
> + const __be32 *entry;
> +
> + entry = &associativity[be32_to_cpu(distance_ref_points[i])];
So, here you've now removed all checks that distance_ref_points[i]
doesn't overflow the length of associativity. It comes back in 6/6,
but it's still not ideal.
> + distance_lookup_table[nid][i] = of_read_number(entry, 1);
> + }
> + }
> +}
> +
> +static void initialize_form1_numa_distance(struct device_node *node)
> +{
> + const __be32 *associativity;
> +
> + associativity = of_get_associativity(node);
> + if (!associativity)
> + return;
> +
> + __initialize_form1_numa_distance(associativity);
> +}
> +
> +/*
> + * Used to update distance information w.r.t newly added node.
> + */
> +void update_numa_distance(struct device_node *node)
> +{
> + if (affinity_form == FORM0_AFFINITY)
> + return;
> + else if (affinity_form == FORM1_AFFINITY) {
> + initialize_form1_numa_distance(node);
> + return;
> + }
> +}
> +
> static int __init find_primary_domain_index(void)
> {
> int index;
> @@ -433,6 +450,48 @@ static int of_get_assoc_arrays(struct assoc_arrays *aa)
> return 0;
> }
>
> +static int get_nid_and_numa_distance(struct drmem_lmb *lmb)
> +{
> + struct assoc_arrays aa = { .arrays = NULL };
> + int default_nid = NUMA_NO_NODE;
There's still no point I can see to this intermediate variable.
> + int nid = default_nid;
> + int rc, index;
> +
> + if ((primary_domain_index < 0) || !numa_enabled)
> + return default_nid;
> +
> + rc = of_get_assoc_arrays(&aa);
> + if (rc)
> + return default_nid;
> +
> + if (primary_domain_index <= aa.array_sz &&
> + !(lmb->flags & DRCONF_MEM_AI_INVALID) && lmb->aa_index < aa.n_arrays) {
> + index = lmb->aa_index * aa.array_sz + primary_domain_index - 1;
> + nid = of_read_number(&aa.arrays[index], 1);
> +
> + if (nid == 0xffff || nid >= nr_node_ids)
> + nid = default_nid;
> + if (nid > 0 && affinity_form == FORM1_AFFINITY) {
> + int i;
> + const __be32 *associativity;
> +
> + index = lmb->aa_index * aa.array_sz;
> + associativity = &aa.arrays[index];
> + /*
> + * lookup array associativity entries have different format
> + * There is no length of the array as the first element.
> + */
> + for (i = 0; i < distance_ref_points_depth; i++) {
> + const __be32 *entry;
> +
> + entry = &associativity[be32_to_cpu(distance_ref_points[i]) - 1];
> + distance_lookup_table[nid][i] = of_read_number(entry, 1);
> + }
> + }
> + }
> + return nid;
> +}
> +
> /*
> * This is like of_node_to_nid_single() for memory represented in the
> * ibm,dynamic-reconfiguration-memory node.
> @@ -458,21 +517,14 @@ int of_drconf_to_nid_single(struct drmem_lmb *lmb)
>
> if (nid == 0xffff || nid >= nr_node_ids)
> nid = default_nid;
> -
> - if (nid > 0) {
> - index = lmb->aa_index * aa.array_sz;
> - initialize_distance_lookup_table(nid,
> - &aa.arrays[index]);
> - }
> }
> -
> return nid;
> }
>
> #ifdef CONFIG_PPC_SPLPAR
> -static int vphn_get_nid(long lcpu)
> +
> +static int __vphn_get_associativity(long lcpu, __be32 *associativity)
> {
> - __be32 associativity[VPHN_ASSOC_BUFSIZE] = {0};
> long rc, hwid;
>
> /*
> @@ -492,12 +544,30 @@ static int vphn_get_nid(long lcpu)
>
> rc = hcall_vphn(hwid, VPHN_FLAG_VCPU, associativity);
> if (rc == H_SUCCESS)
> - return associativity_to_nid(associativity);
> + return 0;
> }
>
> + return -1;
> +}
> +
> +static int vphn_get_nid(long lcpu)
> +{
> + __be32 associativity[VPHN_ASSOC_BUFSIZE] = {0};
> +
> +
> + if (!__vphn_get_associativity(lcpu, associativity))
> + return associativity_to_nid(associativity);
> +
> return NUMA_NO_NODE;
> +
> }
> #else
> +
> +static int __vphn_get_associativity(long lcpu, __be32 *associativity)
> +{
> + return -1;
> +}
> +
> static int vphn_get_nid(long unused)
> {
> return NUMA_NO_NODE;
> @@ -692,7 +762,7 @@ static int __init numa_setup_drmem_lmb(struct drmem_lmb *lmb,
> size = read_n_cells(n_mem_size_cells, usm);
> }
>
> - nid = of_drconf_to_nid_single(lmb);
> + nid = get_nid_and_numa_distance(lmb);
> fake_numa_create_new_node(((base + size) >> PAGE_SHIFT),
> &nid);
> node_set_online(nid);
> @@ -709,6 +779,7 @@ static int __init parse_numa_properties(void)
> struct device_node *memory;
> int default_nid = 0;
> unsigned long i;
> + const __be32 *associativity;
>
> if (numa_enabled == 0) {
> printk(KERN_WARNING "NUMA disabled by user\n");
> @@ -734,18 +805,30 @@ static int __init parse_numa_properties(void)
> * each node to be onlined must have NODE_DATA etc backing it.
> */
> for_each_present_cpu(i) {
> + __be32 vphn_assoc[VPHN_ASSOC_BUFSIZE];
> struct device_node *cpu;
> - int nid = vphn_get_nid(i);
> + int nid = NUMA_NO_NODE;
>
> - /*
> - * Don't fall back to default_nid yet -- we will plug
> - * cpus into nodes once the memory scan has discovered
> - * the topology.
> - */
> - if (nid == NUMA_NO_NODE) {
> + memset(vphn_assoc, 0, VPHN_ASSOC_BUFSIZE * sizeof(__be32));
> +
> + if (__vphn_get_associativity(i, vphn_assoc) == 0) {
> + nid = associativity_to_nid(vphn_assoc);
> + __initialize_form1_numa_distance(vphn_assoc);
> + } else {
> +
> + /*
> + * Don't fall back to default_nid yet -- we will plug
> + * cpus into nodes once the memory scan has discovered
> + * the topology.
> + */
> cpu = of_get_cpu_node(i, NULL);
> BUG_ON(!cpu);
> - nid = of_node_to_nid_single(cpu);
> +
> + associativity = of_get_associativity(cpu);
> + if (associativity) {
> + nid = associativity_to_nid(associativity);
> + __initialize_form1_numa_distance(associativity);
> + }
> of_node_put(cpu);
> }
>
> @@ -781,8 +864,11 @@ static int __init parse_numa_properties(void)
> * have associativity properties. If none, then
> * everything goes to default_nid.
> */
> - nid = of_node_to_nid_single(memory);
> - if (nid < 0)
> + associativity = of_get_associativity(memory);
> + if (associativity) {
> + nid = associativity_to_nid(associativity);
> + __initialize_form1_numa_distance(associativity);
> + } else
> nid = default_nid;
>
> fake_numa_create_new_node(((start + size) >> PAGE_SHIFT), &nid);
> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> index 7e970f81d8ff..778b6ab35f0d 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> @@ -498,6 +498,8 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
> return saved_rc;
> }
>
> + update_numa_distance(dn);
> +
> rc = dlpar_online_cpu(dn);
> if (rc) {
> saved_rc = rc;
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index 377d852f5a9a..ee1d81d7e54a 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -180,6 +180,8 @@ static int update_lmb_associativity_index(struct drmem_lmb *lmb)
> return -ENODEV;
> }
>
> + update_numa_distance(lmb_node);
> +
> dr_node = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
> if (!dr_node) {
> dlpar_free_cc_nodes(lmb_node);
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ 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