* Re: [Intel-gfx] [PATCH v7 00/12] Introduce CAP_PERFMON to secure system performance monitoring and observability
From: James Morris @ 2020-03-26 23:28 UTC (permalink / raw)
To: Serge Hallyn
Cc: linux-man, linux-doc@vger.kernel.org, Peter Zijlstra,
joonas.lahtinen@linux.intel.com, Alexei Starovoitov,
Stephane Eranian, Paul Mackerras, Will Deacon, Ingo Molnar,
Andi Kleen, Jiri Olsa, Alexey Budankov, Igor Lubashev,
oprofile-list, Stephen Smalley, selinux@vger.kernel.org,
intel-gfx@lists.freedesktop.org, Arnaldo Carvalho de Melo,
Thomas Gleixner, linux-arm-kernel, linux-parisc@vger.kernel.org,
linux-kernel, linux-security-module@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, Helge Deller
In-Reply-To: <20200302001913.GA21145@sl>
On Sun, 1 Mar 2020, Serge Hallyn wrote:
> Thanks, this looks good to me, in keeping with the CAP_SYSLOG break.
>
> Acked-by: Serge E. Hallyn <serge@hallyn.com>
>
> for the set.
>
> James/Ingo/Peter, if noone has remaining objections, whose branch
> should these go in through?
>
> thanks,
> -serge
>
> On Tue, Feb 25, 2020 at 12:55:54PM +0300, Alexey Budankov wrote:
> >
> > Hi,
> >
> > Is there anything else I could do in order to move the changes forward
> > or is something still missing from this patch set?
> > Could you please share you mind?
Alexey,
It seems some of the previous Acks are not included in this patchset, e.g.
https://lkml.org/lkml/2020/1/22/655
Every patch needs a Reviewed-by or Acked-by from maintainers of the code
being changed.
You have enough from the security folk, but I can't see any included from
the perf folk.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH 1/1] ppc/crash: Skip spinlocks during crash
From: Leonardo Bras @ 2020-03-26 23:26 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Will Deacon, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Enrico Weigelt, Allison Randal,
Christophe Leroy, Thomas Gleixner
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200326222836.501404-1-leonardo@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 2613 bytes --]
oops, forgot to EXPORT_SYMBOL.
arch_spin_lock*() is used on modules.
Sending v2.
On Thu, 2020-03-26 at 19:28 -0300, Leonardo Bras wrote:
> During a crash, there is chance that the cpus that handle the NMI IPI
> are holding a spin_lock. If this spin_lock is needed by crashing_cpu it
> will cause a deadlock. (rtas_lock and printk logbuf_log as of today)
>
> This is a problem if the system has kdump set up, given if it crashes
> for any reason kdump may not be saved for crash analysis.
>
> Skip spinlocks after NMI IPI is sent to all other cpus.
>
> Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
> ---
> arch/powerpc/include/asm/spinlock.h | 6 ++++++
> arch/powerpc/kexec/crash.c | 3 +++
> 2 files changed, 9 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/spinlock.h b/arch/powerpc/include/asm/spinlock.h
> index 860228e917dc..a6381d110795 100644
> --- a/arch/powerpc/include/asm/spinlock.h
> +++ b/arch/powerpc/include/asm/spinlock.h
> @@ -111,6 +111,8 @@ static inline void splpar_spin_yield(arch_spinlock_t *lock) {};
> static inline void splpar_rw_yield(arch_rwlock_t *lock) {};
> #endif
>
> +extern bool crash_skip_spinlock __read_mostly;
> +
> static inline bool is_shared_processor(void)
> {
> #ifdef CONFIG_PPC_SPLPAR
> @@ -142,6 +144,8 @@ static inline void arch_spin_lock(arch_spinlock_t *lock)
> if (likely(__arch_spin_trylock(lock) == 0))
> break;
> do {
> + if (unlikely(crash_skip_spinlock))
> + return;
> HMT_low();
> if (is_shared_processor())
> splpar_spin_yield(lock);
> @@ -161,6 +165,8 @@ void arch_spin_lock_flags(arch_spinlock_t *lock, unsigned long flags)
> local_save_flags(flags_dis);
> local_irq_restore(flags);
> do {
> + if (unlikely(crash_skip_spinlock))
> + return;
> HMT_low();
> if (is_shared_processor())
> splpar_spin_yield(lock);
> diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
> index d488311efab1..8a522380027d 100644
> --- a/arch/powerpc/kexec/crash.c
> +++ b/arch/powerpc/kexec/crash.c
> @@ -66,6 +66,8 @@ static int handle_fault(struct pt_regs *regs)
>
> #ifdef CONFIG_SMP
>
> +bool crash_skip_spinlock;
> +
> static atomic_t cpus_in_crash;
> void crash_ipi_callback(struct pt_regs *regs)
> {
> @@ -129,6 +131,7 @@ static void crash_kexec_prepare_cpus(int cpu)
> /* Would it be better to replace the trap vector here? */
>
> if (atomic_read(&cpus_in_crash) >= ncpus) {
> + crash_skip_spinlock = true;
> printk(KERN_EMERG "IPI complete\n");
> return;
> }
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v2 1/1] ppc/crash: Skip spinlocks during crash
From: Leonardo Bras @ 2020-03-26 23:25 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Will Deacon, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Greg Kroah-Hartman,
Thomas Gleixner, Alexios Zavras, Christophe Leroy, Leonardo Bras
Cc: linuxppc-dev, linux-kernel
During a crash, there is chance that the cpus that handle the NMI IPI
are holding a spin_lock. If this spin_lock is needed by crashing_cpu it
will cause a deadlock. (rtas_lock and printk logbuf_log as of today)
This is a problem if the system has kdump set up, given if it crashes
for any reason kdump may not be saved for crash analysis.
Skip spinlocks after NMI IPI is sent to all other cpus.
Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
---
arch/powerpc/include/asm/spinlock.h | 6 ++++++
arch/powerpc/kexec/crash.c | 4 ++++
2 files changed, 10 insertions(+)
diff --git a/arch/powerpc/include/asm/spinlock.h b/arch/powerpc/include/asm/spinlock.h
index 860228e917dc..a6381d110795 100644
--- a/arch/powerpc/include/asm/spinlock.h
+++ b/arch/powerpc/include/asm/spinlock.h
@@ -111,6 +111,8 @@ static inline void splpar_spin_yield(arch_spinlock_t *lock) {};
static inline void splpar_rw_yield(arch_rwlock_t *lock) {};
#endif
+extern bool crash_skip_spinlock __read_mostly;
+
static inline bool is_shared_processor(void)
{
#ifdef CONFIG_PPC_SPLPAR
@@ -142,6 +144,8 @@ static inline void arch_spin_lock(arch_spinlock_t *lock)
if (likely(__arch_spin_trylock(lock) == 0))
break;
do {
+ if (unlikely(crash_skip_spinlock))
+ return;
HMT_low();
if (is_shared_processor())
splpar_spin_yield(lock);
@@ -161,6 +165,8 @@ void arch_spin_lock_flags(arch_spinlock_t *lock, unsigned long flags)
local_save_flags(flags_dis);
local_irq_restore(flags);
do {
+ if (unlikely(crash_skip_spinlock))
+ return;
HMT_low();
if (is_shared_processor())
splpar_spin_yield(lock);
diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
index d488311efab1..ae081f0f2472 100644
--- a/arch/powerpc/kexec/crash.c
+++ b/arch/powerpc/kexec/crash.c
@@ -66,6 +66,9 @@ static int handle_fault(struct pt_regs *regs)
#ifdef CONFIG_SMP
+bool crash_skip_spinlock;
+EXPORT_SYMBOL(crash_skip_spinlock);
+
static atomic_t cpus_in_crash;
void crash_ipi_callback(struct pt_regs *regs)
{
@@ -129,6 +132,7 @@ static void crash_kexec_prepare_cpus(int cpu)
/* Would it be better to replace the trap vector here? */
if (atomic_read(&cpus_in_crash) >= ncpus) {
+ crash_skip_spinlock = true;
printk(KERN_EMERG "IPI complete\n");
return;
}
--
2.24.1
^ permalink raw reply related
* Re: [PATCH 1/4] hugetlbfs: add arch_hugetlb_valid_size
From: Dave Hansen @ 2020-03-26 23:10 UTC (permalink / raw)
To: Mike Kravetz, linux-mm, linux-kernel, linux-arm-kernel,
linuxppc-dev, linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Albert Ou, Andrew Morton, Vasily Gorbik, Jonathan Corbet,
Catalin Marinas, Dave Hansen, Heiko Carstens, Shutemov, Kirill,
Christian Borntraeger, Ingo Molnar, Palmer Dabbelt, Paul Walmsley,
Paul Mackerras, Thomas Gleixner, Longpeng, Will Deacon,
David S.Miller
In-Reply-To: <530e6e11-ad1a-55bc-e61e-9da6eb7fea21@oracle.com>
On 3/26/20 2:56 PM, Mike Kravetz wrote:
> Perhaps it would be best to check hugepages_supported() when parsing
> hugetlb command line options. If not enabled, throw an error. This
> will be much easier to do after moving all command line parsing to
> arch independent code.
Yeah, that sounds sane.
> Is that a sufficient way to address this concern? I think it is a good
> change in any case.
(Thanks to Kirill for pointing this out.)
So, it turns out the x86 huge page enumeration is totally buggered.
X86_FEATURE_PSE is actually meaningless on 64-bit (and 32-bit PAE). All
CPUs architecturally support 2MB pages regardless of X86_FEATURE_PSE and
the state of CR4.PSE.
So, on x86_64 at least, hugepages_supported() should *always* return 1.
1GB page support can continue to be dependent on X86_FEATURE_GBPAGES.
^ permalink raw reply
* [PATCH 1/1] ppc/crash: Skip spinlocks during crash
From: Leonardo Bras @ 2020-03-26 22:28 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Will Deacon, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Enrico Weigelt, Leonardo Bras,
Allison Randal, Christophe Leroy, Thomas Gleixner
Cc: linuxppc-dev, linux-kernel
During a crash, there is chance that the cpus that handle the NMI IPI
are holding a spin_lock. If this spin_lock is needed by crashing_cpu it
will cause a deadlock. (rtas_lock and printk logbuf_log as of today)
This is a problem if the system has kdump set up, given if it crashes
for any reason kdump may not be saved for crash analysis.
Skip spinlocks after NMI IPI is sent to all other cpus.
Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
---
arch/powerpc/include/asm/spinlock.h | 6 ++++++
arch/powerpc/kexec/crash.c | 3 +++
2 files changed, 9 insertions(+)
diff --git a/arch/powerpc/include/asm/spinlock.h b/arch/powerpc/include/asm/spinlock.h
index 860228e917dc..a6381d110795 100644
--- a/arch/powerpc/include/asm/spinlock.h
+++ b/arch/powerpc/include/asm/spinlock.h
@@ -111,6 +111,8 @@ static inline void splpar_spin_yield(arch_spinlock_t *lock) {};
static inline void splpar_rw_yield(arch_rwlock_t *lock) {};
#endif
+extern bool crash_skip_spinlock __read_mostly;
+
static inline bool is_shared_processor(void)
{
#ifdef CONFIG_PPC_SPLPAR
@@ -142,6 +144,8 @@ static inline void arch_spin_lock(arch_spinlock_t *lock)
if (likely(__arch_spin_trylock(lock) == 0))
break;
do {
+ if (unlikely(crash_skip_spinlock))
+ return;
HMT_low();
if (is_shared_processor())
splpar_spin_yield(lock);
@@ -161,6 +165,8 @@ void arch_spin_lock_flags(arch_spinlock_t *lock, unsigned long flags)
local_save_flags(flags_dis);
local_irq_restore(flags);
do {
+ if (unlikely(crash_skip_spinlock))
+ return;
HMT_low();
if (is_shared_processor())
splpar_spin_yield(lock);
diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
index d488311efab1..8a522380027d 100644
--- a/arch/powerpc/kexec/crash.c
+++ b/arch/powerpc/kexec/crash.c
@@ -66,6 +66,8 @@ static int handle_fault(struct pt_regs *regs)
#ifdef CONFIG_SMP
+bool crash_skip_spinlock;
+
static atomic_t cpus_in_crash;
void crash_ipi_callback(struct pt_regs *regs)
{
@@ -129,6 +131,7 @@ static void crash_kexec_prepare_cpus(int cpu)
/* Would it be better to replace the trap vector here? */
if (atomic_read(&cpus_in_crash) >= ncpus) {
+ crash_skip_spinlock = true;
printk(KERN_EMERG "IPI complete\n");
return;
}
--
2.24.1
^ permalink raw reply related
* Re: [PATCH v2] powerpc/boot: Delete unneeded .globl _zimage_start
From: Fangrui Song @ 2020-03-26 22:26 UTC (permalink / raw)
To: Segher Boessenkool
Cc: Alan Modra, Nick Desaulniers, clang-built-linux, Joel Stanley,
linuxppc-dev
In-Reply-To: <20200326221625.GA22482@gate.crashing.org>
On 2020-03-26, Segher Boessenkool wrote:
>On Wed, Mar 25, 2020 at 09:42:57AM -0700, Fangrui Song wrote:
>> .globl sets the symbol binding to STB_GLOBAL while .weak sets the
>> binding to STB_WEAK. GNU as let .weak override .globl since binutils-gdb
>> 5ca547dc2399a0a5d9f20626d4bf5547c3ccfddd (1996). Clang integrated
>> assembler let the last win but it may error in the future.
>
>GNU AS works for more than just ELF. The way the assembler language
>is defined, it is not .weak overriding .globl -- instead, .weak sets a
>symbol attribute. On an existing symbol (but it creates on if there is
>none yet).
>
>Clang is buggy if it does not allow valid (and perfectly normal)
>assembler code like this.
https://sourceware.org/pipermail/binutils/2020-March/110399.html
Alan: "I think it is completely fine for you to make the llvm assembler
error on inconsistent binding, or the last directive win. Either of
those behaviours is logical and good, but you quite possibly will run
into a need to fix more user assembly.
I am doing some experiments whether making clang integrated assembler
error is feasible.
^ permalink raw reply
* Re: [PATCH v2] powerpc/boot: Delete unneeded .globl _zimage_start
From: Segher Boessenkool @ 2020-03-26 22:16 UTC (permalink / raw)
To: Fangrui Song
Cc: Alan Modra, Nick Desaulniers, clang-built-linux, Joel Stanley,
linuxppc-dev
In-Reply-To: <20200325164257.170229-1-maskray@google.com>
On Wed, Mar 25, 2020 at 09:42:57AM -0700, Fangrui Song wrote:
> .globl sets the symbol binding to STB_GLOBAL while .weak sets the
> binding to STB_WEAK. GNU as let .weak override .globl since binutils-gdb
> 5ca547dc2399a0a5d9f20626d4bf5547c3ccfddd (1996). Clang integrated
> assembler let the last win but it may error in the future.
GNU AS works for more than just ELF. The way the assembler language
is defined, it is not .weak overriding .globl -- instead, .weak sets a
symbol attribute. On an existing symbol (but it creates on if there is
none yet).
Clang is buggy if it does not allow valid (and perfectly normal)
assembler code like this.
Segher
^ permalink raw reply
* Re: [PATCH v2] mm/sparse: Fix kernel crash with pfn_section_valid check
From: Wei Yang @ 2020-03-26 22:12 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Sachin Sant, Pankaj Gupta, Michal Hocko, Baoquan He,
David Hildenbrand, linux-kernel, stable, Mike Rapoport, linux-mm,
Wei Yang, akpm, linuxppc-dev, Dan Williams, Oscar Salvador
In-Reply-To: <20200326133235.343616-1-aneesh.kumar@linux.ibm.com>
On Thu, Mar 26, 2020 at 07:02:35PM +0530, Aneesh Kumar K.V wrote:
>Fixes the below crash
>
>BUG: Kernel NULL pointer dereference on read at 0x00000000
>Faulting instruction address: 0xc000000000c3447c
>Oops: Kernel access of bad area, sig: 11 [#1]
>LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
>CPU: 11 PID: 7519 Comm: lt-ndctl Not tainted 5.6.0-rc7-autotest #1
>...
>NIP [c000000000c3447c] vmemmap_populated+0x98/0xc0
>LR [c000000000088354] vmemmap_free+0x144/0x320
>Call Trace:
> section_deactivate+0x220/0x240
> __remove_pages+0x118/0x170
> arch_remove_memory+0x3c/0x150
> memunmap_pages+0x1cc/0x2f0
> devm_action_release+0x30/0x50
> release_nodes+0x2f8/0x3e0
> device_release_driver_internal+0x168/0x270
> unbind_store+0x130/0x170
> drv_attr_store+0x44/0x60
> sysfs_kf_write+0x68/0x80
> kernfs_fop_write+0x100/0x290
> __vfs_write+0x3c/0x70
> vfs_write+0xcc/0x240
> ksys_write+0x7c/0x140
> system_call+0x5c/0x68
>
>The crash is due to NULL dereference at
>
>test_bit(idx, ms->usage->subsection_map); due to ms->usage = NULL; in pfn_section_valid()
>
>With commit: d41e2f3bd546 ("mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case")
>section_mem_map is set to NULL after depopulate_section_mem(). This
>was done so that pfn_page() can work correctly with kernel config that disables
>SPARSEMEM_VMEMMAP. With that config pfn_to_page does
>
> __section_mem_map_addr(__sec) + __pfn;
>where
>
>static inline struct page *__section_mem_map_addr(struct mem_section *section)
>{
> unsigned long map = section->section_mem_map;
> map &= SECTION_MAP_MASK;
> return (struct page *)map;
>}
>
>Now with SPASEMEM_VMEMAP enabled, mem_section->usage->subsection_map is used to
>check the pfn validity (pfn_valid()). Since section_deactivate release
>mem_section->usage if a section is fully deactivated, pfn_valid() check after
>a subsection_deactivate cause a kernel crash.
>
>static inline int pfn_valid(unsigned long pfn)
>{
>...
> return early_section(ms) || pfn_section_valid(ms, pfn);
>}
>
>where
>
>static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
>{
> int idx = subsection_map_index(pfn);
>
> return test_bit(idx, ms->usage->subsection_map);
>}
>
>Avoid this by clearing SECTION_HAS_MEM_MAP when mem_section->usage is freed.
>For architectures like ppc64 where large pages are used for vmmemap mapping (16MB),
>a specific vmemmap mapping can cover multiple sections. Hence before a vmemmap
>mapping page can be freed, the kernel needs to make sure there are no valid sections
>within that mapping. Clearing the section valid bit before
>depopulate_section_memap enables this.
>
>Fixes: d41e2f3bd546 ("mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case")
>Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
>Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
>Cc: Baoquan He <bhe@redhat.com>
>Cc: Michael Ellerman <mpe@ellerman.id.au>
>Cc: Dan Williams <dan.j.williams@intel.com>
>Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
>Cc: David Hildenbrand <david@redhat.com>
>Cc: Michal Hocko <mhocko@suse.com>
>Cc: Wei Yang <richardw.yang@linux.intel.com>
>Cc: Oscar Salvador <osalvador@suse.de>
>Cc: Mike Rapoport <rppt@linux.ibm.com>
>Cc: <stable@vger.kernel.org>
>Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
--
Wei Yang
Help you, Help me
^ permalink raw reply
* Re: [RFC PATCH 1/1] ppc/smp: Replace unnecessary 'while' by 'if'
From: Leonardo Bras @ 2020-03-26 22:05 UTC (permalink / raw)
To: Paul Mackerras
Cc: linux-kernel, Greg Kroah-Hartman, Thomas Gleixner, linuxppc-dev,
Allison Randal
In-Reply-To: <20200326214005.GB9894@blackberry>
[-- Attachment #1: Type: text/plain, Size: 1088 bytes --]
On Fri, 2020-03-27 at 08:40 +1100, Paul Mackerras wrote:
> On Thu, Mar 26, 2020 at 05:37:52PM -0300, Leonardo Bras wrote:
> > spin_until_cond() will wait until nmi_ipi_busy == false, and
> > nmi_ipi_lock_start() does not seem to change nmi_ipi_busy, so there is
> > no way this while will ever repeat.
> >
> > Replace this 'while' by an 'if', so it does not look like it can repeat.
>
> Nack, it can repeat. The scenario is that cpu A is in this code,
> inside spin_until_cond(); cpu B has previously set nmi_ipi_busy, and
> cpu C is also waiting for nmi_ipi_busy to be cleared, like cpu A.
> When cpu B clears nmi_ipi_busy, both cpu A and cpu C will see that and
> will race inside nmi_ipi_lock_start(). One of them, say cpu C, will
> take the lock and proceed to set nmi_ipi_busy and then call
> nmi_ipi_unlock(). Then the other cpu (cpu A) will then take the lock
> and return from nmi_ipi_lock_start() and find nmi_ipi_busy == true.
> At that point it needs to go through the while loop body once more.
>
> Paul.
Ok, got it.
Thanks for explaining Paul!
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 1/4] hugetlbfs: add arch_hugetlb_valid_size
From: Mike Kravetz @ 2020-03-26 21:56 UTC (permalink / raw)
To: Dave Hansen, linux-mm, linux-kernel, linux-arm-kernel,
linuxppc-dev, linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Albert Ou, Andrew Morton, Vasily Gorbik, Jonathan Corbet,
Catalin Marinas, Dave Hansen, Heiko Carstens,
Christian Borntraeger, Ingo Molnar, Palmer Dabbelt, Paul Walmsley,
Paul Mackerras, Thomas Gleixner, Longpeng, Will Deacon,
David S.Miller
In-Reply-To: <1c8b16c7-248e-b75f-96c6-eabc953c5066@intel.com>
On 3/18/20 4:36 PM, Dave Hansen wrote:
> On 3/18/20 3:52 PM, Mike Kravetz wrote:
>> Sounds good. I'll incorporate those changes into a v2, unless someone
>> else with has a different opinion.
>>
>> BTW, this patch should not really change the way the code works today.
>> It is mostly a movement of code. Unless I am missing something, the
>> existing code will always allow setup of PMD_SIZE hugetlb pages.
>
> Hah, I totally skipped over the old code in the diff.
>
> It looks like we'll disable hugetblfs *entirely* if PSE isn't supported.
> I think this is actually wrong, but nobody ever noticed. I think you'd
> have to be running as a guest under a hypervisor that's lying about PSE
> not being supported *and* care about 1GB pages. Nobody does that.
Actually, !PSE will disable hugetlbfs a little later in the boot process.
You are talking about hugepages_supported() correct?
I think something really bad could happen in this situation (!PSE and
X86_FEATURE_GBPAGES). When parsing 'hugepages=' for gigantic pages we
immediately allocate from bootmem. This happens before later checks in
hugetlb_init for hugepages_supported(). So, I think we would end up
allocating GB pages from bootmem and not be able to use or free them. :(
Perhaps it would be best to check hugepages_supported() when parsing
hugetlb command line options. If not enabled, throw an error. This
will be much easier to do after moving all command line parsing to
arch independent code.
Is that a sufficient way to address this concern? I think it is a good
change in any case.
--
Mike Kravetz
^ permalink raw reply
* Re: [RFC PATCH 1/1] ppc/smp: Replace unnecessary 'while' by 'if'
From: Paul Mackerras @ 2020-03-26 21:40 UTC (permalink / raw)
To: Leonardo Bras
Cc: linux-kernel, Greg Kroah-Hartman, Thomas Gleixner, linuxppc-dev,
Allison Randal
In-Reply-To: <20200326203752.497029-1-leonardo@linux.ibm.com>
On Thu, Mar 26, 2020 at 05:37:52PM -0300, Leonardo Bras wrote:
> spin_until_cond() will wait until nmi_ipi_busy == false, and
> nmi_ipi_lock_start() does not seem to change nmi_ipi_busy, so there is
> no way this while will ever repeat.
>
> Replace this 'while' by an 'if', so it does not look like it can repeat.
Nack, it can repeat. The scenario is that cpu A is in this code,
inside spin_until_cond(); cpu B has previously set nmi_ipi_busy, and
cpu C is also waiting for nmi_ipi_busy to be cleared, like cpu A.
When cpu B clears nmi_ipi_busy, both cpu A and cpu C will see that and
will race inside nmi_ipi_lock_start(). One of them, say cpu C, will
take the lock and proceed to set nmi_ipi_busy and then call
nmi_ipi_unlock(). Then the other cpu (cpu A) will then take the lock
and return from nmi_ipi_lock_start() and find nmi_ipi_busy == true.
At that point it needs to go through the while loop body once more.
Paul.
^ permalink raw reply
* [PATCH] x86: Alias memset to __builtin_memset.
From: Clement Courbet @ 2020-03-26 12:38 UTC (permalink / raw)
Cc: x86, Kees Cook, Borislav Petkov, Greg Kroah-Hartman,
H. Peter Anvin, Nick Desaulniers, linux-kernel, clang-built-linux,
Ingo Molnar, Paul Mackerras, Clement Courbet, Joe Perches,
Bernd Petrovitsch, Nathan Chancellor, linuxppc-dev,
Thomas Gleixner, Allison Randal
In-Reply-To: <20200323114207.222412-1-courbet@google.com>
I discussed with the original authors who added freestanding to our
build. It turns out that it was added globally but this was just to
to workaround powerpc not compiling under clang, but they felt the
fix was appropriate globally.
Now Nick has dug up https://lkml.org/lkml/2019/8/29/1300, which
advises against freestanding. Also, I've did some research and
discovered that the original reason for using freestanding for
powerpc has been fixed here:
https://lore.kernel.org/linuxppc-dev/20191119045712.39633-3-natechancellor@gmail.com/
I'm going to remove -ffreestanding from downstream, so we don't really need
this anymore, sorry for waisting people's time.
I wonder if the freestanding fix from the aforementioned patch is really needed
though. I think that clang is actually right to point out the issue.
I don't see any reason why setjmp()/longjmp() are declared as taking longs
rather than ints. The implementation looks like it only ever propagates the
value (in longjmp) or sets it to 1 (in setjmp), and we only ever call longjmp
with integer parameters. But I'm not a PowerPC expert, so I might
be misreading the code.
So it seems that we could just remove freestanding altogether and rewrite the
code to:
diff --git a/arch/powerpc/include/asm/setjmp.h b/arch/powerpc/include/asm/setjmp.h
index 279d03a1eec6..7941ae68fe21 100644
--- a/arch/powerpc/include/asm/setjmp.h
+++ b/arch/powerpc/include/asm/setjmp.h
@@ -12,7 +12,9 @@
#define JMP_BUF_LEN 23
-extern long setjmp(long *);
-extern void longjmp(long *, long);
+typedef long * jmp_buf;
+
+extern int setjmp(jmp_buf);
+extern void longjmp(jmp_buf, int);
I'm happy to send a patch for this, and get rid of more -ffreestanding.
Opinions ?
^ permalink raw reply related
* [RFC PATCH 1/1] ppc/smp: Replace unnecessary 'while' by 'if'
From: Leonardo Bras @ 2020-03-26 20:37 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Leonardo Bras, Allison Randal, Greg Kroah-Hartman,
Thomas Gleixner
Cc: linuxppc-dev, linux-kernel
spin_until_cond() will wait until nmi_ipi_busy == false, and
nmi_ipi_lock_start() does not seem to change nmi_ipi_busy, so there is
no way this while will ever repeat.
Replace this 'while' by an 'if', so it does not look like it can repeat.
Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
---
arch/powerpc/kernel/smp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index ea6adbf6a221..7c904d6fb4d2 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -473,7 +473,7 @@ static int __smp_send_nmi_ipi(int cpu, void (*fn)(struct pt_regs *),
return 0;
nmi_ipi_lock_start(&flags);
- while (nmi_ipi_busy) {
+ if (nmi_ipi_busy) {
nmi_ipi_unlock_end(&flags);
spin_until_cond(!nmi_ipi_busy);
nmi_ipi_lock_start(&flags);
--
2.24.1
^ permalink raw reply related
* Re: [RFC 00/11] perf: Enhancing perf to export processor hazard information
From: Kim Phillips @ 2020-03-26 19:48 UTC (permalink / raw)
To: maddy, Ravi Bangoria
Cc: Mark Rutland, Andi Kleen, Peter Zijlstra, Jiri Olsa, LKML,
Stephane Eranian, Adrian Hunter, Alexander Shishkin, yao.jin,
Ingo Molnar, Paul Mackerras, Arnaldo Carvalho de Melo,
Robert Richter, Namhyung Kim, linuxppc-dev, Alexey Budankov,
Liang, Kan
In-Reply-To: <965dba09-813a-59a7-9c10-97ed1c892245@linux.ibm.com>
On 3/26/20 5:19 AM, maddy wrote:
>
>
> On 3/18/20 11:05 PM, Kim Phillips wrote:
>> Hi Maddy,
>>
>> On 3/17/20 1:50 AM, maddy wrote:
>>> On 3/13/20 4:08 AM, Kim Phillips wrote:
>>>> On 3/11/20 11:00 AM, Ravi Bangoria wrote:
>>>>> On 3/6/20 3:36 AM, Kim Phillips wrote:
>>>>>>> On 3/3/20 3:55 AM, Kim Phillips wrote:
>>>>>>>> On 3/2/20 2:21 PM, Stephane Eranian wrote:
>>>>>>>>> On Mon, Mar 2, 2020 at 2:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
>>>>>>>>>> On Mon, Mar 02, 2020 at 10:53:44AM +0530, Ravi Bangoria wrote:
>>>>>>>>>>> Modern processors export such hazard data in Performance
>>>>>>>>>>> Monitoring Unit (PMU) registers. Ex, 'Sampled Instruction Event
>>>>>>>>>>> Register' on IBM PowerPC[1][2] and 'Instruction-Based Sampling' on
>>>>>>>>>>> AMD[3] provides similar information.
>>>>>>>>>>>
>>>>>>>>>>> Implementation detail:
>>>>>>>>>>>
>>>>>>>>>>> A new sample_type called PERF_SAMPLE_PIPELINE_HAZ is introduced.
>>>>>>>>>>> If it's set, kernel converts arch specific hazard information
>>>>>>>>>>> into generic format:
>>>>>>>>>>>
>>>>>>>>>>> struct perf_pipeline_haz_data {
>>>>>>>>>>> /* Instruction/Opcode type: Load, Store, Branch .... */
>>>>>>>>>>> __u8 itype;
>>>>>>>>>>> /* Instruction Cache source */
>>>>>>>>>>> __u8 icache;
>>>>>>>>>>> /* Instruction suffered hazard in pipeline stage */
>>>>>>>>>>> __u8 hazard_stage;
>>>>>>>>>>> /* Hazard reason */
>>>>>>>>>>> __u8 hazard_reason;
>>>>>>>>>>> /* Instruction suffered stall in pipeline stage */
>>>>>>>>>>> __u8 stall_stage;
>>>>>>>>>>> /* Stall reason */
>>>>>>>>>>> __u8 stall_reason;
>>>>>>>>>>> __u16 pad;
>>>>>>>>>>> };
>>>>>>>>>> Kim, does this format indeed work for AMD IBS?
>>>>>>>> It's not really 1:1, we don't have these separations of stages
>>>>>>>> and reasons, for example: we have missed in L2 cache, for example.
>>>>>>>> So IBS output is flatter, with more cycle latency figures than
>>>>>>>> IBM's AFAICT.
>>>>>>> AMD IBS captures pipeline latency data incase Fetch sampling like the
>>>>>>> Fetch latency, tag to retire latency, completion to retire latency and
>>>>>>> so on. Yes, Ops sampling do provide more data on load/store centric
>>>>>>> information. But it also captures more detailed data for Branch instructions.
>>>>>>> And we also looked at ARM SPE, which also captures more details pipeline
>>>>>>> data and latency information.
>>>>>>>
>>>>>>>>> Personally, I don't like the term hazard. This is too IBM Power
>>>>>>>>> specific. We need to find a better term, maybe stall or penalty.
>>>>>>>> Right, IBS doesn't have a filter to only count stalled or otherwise
>>>>>>>> bad events. IBS' PPR descriptions has one occurrence of the
>>>>>>>> word stall, and no penalty. The way I read IBS is it's just
>>>>>>>> reporting more sample data than just the precise IP: things like
>>>>>>>> hits, misses, cycle latencies, addresses, types, etc., so words
>>>>>>>> like 'extended', or the 'auxiliary' already used today even
>>>>>>>> are more appropriate for IBS, although I'm the last person to
>>>>>>>> bikeshed.
>>>>>>> We are thinking of using "pipeline" word instead of Hazard.
>>>>>> Hm, the word 'pipeline' occurs 0 times in IBS documentation.
>>>>> NP. We thought pipeline is generic hw term so we proposed "pipeline"
>>>>> word. We are open to term which can be generic enough.
>>>>>
>>>>>> I realize there are a couple of core pipeline-specific pieces
>>>>>> of information coming out of it, but the vast majority
>>>>>> are addresses, latencies of various components in the memory
>>>>>> hierarchy, and various component hit/miss bits.
>>>>> Yes. we should capture core pipeline specific details. For example,
>>>>> IBS generates Branch unit information(IbsOpData1) and Icahce related
>>>>> data(IbsFetchCtl) which is something that shouldn't be extended as
>>>>> part of perf-mem, IMO.
>>>> Sure, IBS Op-side output is more 'perf mem' friendly, and so it
>>>> should populate perf_mem_data_src fields, just like POWER9 can:
>>>>
>>>> union perf_mem_data_src {
>>>> ...
>>>> __u64 mem_rsvd:24,
>>>> mem_snoopx:2, /* snoop mode, ext */
>>>> mem_remote:1, /* remote */
>>>> mem_lvl_num:4, /* memory hierarchy level number */
>>>> mem_dtlb:7, /* tlb access */
>>>> mem_lock:2, /* lock instr */
>>>> mem_snoop:5, /* snoop mode */
>>>> mem_lvl:14, /* memory hierarchy level */
>>>> mem_op:5; /* type of opcode */
>>>>
>>>>
>>>> E.g., SIER[LDST] SIER[A_XLATE_SRC] can be used to populate
>>>> mem_lvl[_num], SIER_TYPE can be used to populate 'mem_op',
>>>> 'mem_lock', and the Reload Bus Source Encoding bits can
>>>> be used to populate mem_snoop, right?
>>> Hi Kim,
>>>
>>> Yes. We do expose these data as part of perf-mem for POWER.
>> OK, I see relevant PERF_MEM_S bits in arch/powerpc/perf/isa207-common.c:
>> isa207_find_source now, thanks.
>>
>>>> For IBS, I see PERF_SAMPLE_ADDR and PERF_SAMPLE_PHYS_ADDR can be
>>>> used for the ld/st target addresses, too.
>>>>
>>>>>> What's needed here is a vendor-specific extended
>>>>>> sample information that all these technologies gather,
>>>>>> of which things like e.g., 'L1 TLB cycle latency' we
>>>>>> all should have in common.
>>>>> Yes. We will include fields to capture the latency cycles (like Issue
>>>>> latency, Instruction completion latency etc..) along with other pipeline
>>>>> details in the proposed structure.
>>>> Latency figures are just an example, and from what I
>>>> can tell, struct perf_sample_data already has a 'weight' member,
>>>> used with PERF_SAMPLE_WEIGHT, that is used by intel-pt to
>>>> transfer memory access latency figures. Granted, that's
>>>> a bad name given all other vendors don't call latency
>>>> 'weight'.
>>>>
>>>> I didn't see any latency figures coming out of POWER9,
>>>> and do not expect this patchseries to implement those
>>>> of other vendors, e.g., AMD's IBS; leave each vendor
>>>> to amend perf to suit their own h/w output please.
>>> Reference structure proposed in this patchset did not have members
>>> to capture latency info for that exact reason. But idea here is to
>>> abstract as vendor specific as possible. So if we include u16 array,
>>> then this format can also capture data from IBS since it provides
>>> few latency details.
>> OK, that sounds a bit different from the 6 x u8's + 1 u16 padded
>> struct presented in this patchset.
>>
>> IBS Ops can report e.g.:
>>
>> 15 tag-to-retire cycles bits,
>> 15 completion to retire count bits,
>> 15 L1 DTLB refill latency bits,
>> 15 DC miss latency bits,
>> 5 outstanding memory requests on mem refill bits, and so on.
>>
>> IBS Fetch reports 15 bits of fetch latency, and another 16
>> for iTLB latency, among others.
>>
>> Some of these may/may not be valid simultaneously, and
>> there are IBS specific rules to establish validity.
>>
>>>> My main point there, however, was that each vendor should
>>>> use streamlined record-level code to just copy the data
>>>> in the proprietary format that their hardware produces,
>>>> and then then perf tooling can synthesize the events
>>>> from the raw data at report/script/etc. time.
>>>>
>>>>>> I'm not sure why a new PERF_SAMPLE_PIPELINE_HAZ is needed
>>>>>> either. Can we use PERF_SAMPLE_AUX instead?
>>>>> We took a look at PERF_SAMPLE_AUX. IIUC, PERF_SAMPLE_AUX is intended when
>>>>> large volume of data needs to be captured as part of perf.data without
>>>>> frequent PMIs. But proposed type is to address the capture of pipeline
>>>> SAMPLE_AUX shouldn't care whether the volume is large, or how frequent
>>>> PMIs are, even though it may be used in those environments.
>>>>
>>>>> information on each sample using PMI at periodic intervals. Hence proposing
>>>>> PERF_SAMPLE_PIPELINE_HAZ.
>>>> And that's fine for any extra bits that POWER9 has to convey
>>>> to its users beyond things already represented by other sample
>>>> types like PERF_SAMPLE_DATA_SRC, but the capturing of both POWER9
>>>> and other vendor e.g., AMD IBS data can be made vendor-independent
>>>> at record time by using SAMPLE_AUX, or SAMPLE_RAW even, which is
>>>> what IBS currently uses.
>>> My bad. Not sure what you mean by this. We are trying to abstract
>>> as much vendor specific data as possible with this (like perf-mem).
>> Perhaps if I say it this way: instead of doing all the
>> isa207_get_phazard_data() work past the mfspr(SPRN_SIER)
>> in patch 4/11, rather/instead just put the raw sier value in a
>> PERF_SAMPLE_RAW or _AUX event, and call perf_event_update_userpage.
>> Specific SIER capabilities can be written as part of the perf.data
>> header. Then synthesize the true pipe events from the raw SIER
>> values later, and in userspace.
>
> Hi Kim,
>
> Would like to stay away from SAMPLE_RAW type for these comments in perf_events.h
>
> * #
> * # The RAW record below is opaque data wrt the ABI
> * #
> * # That is, the ABI doesn't make any promises wrt to
> * # the stability of its content, it may vary depending
> * # on event, hardware, kernel version and phase of
> * # the moon.
> * #
> * # In other words, PERF_SAMPLE_RAW contents are not an ABI.
> * #
The "it may vary depending on ... hardware" clause makes it sound
appropriate for the use-case where the raw hardware register contents
are copied directly into the user buffer.
> Secondly, sorry I didn't understand your suggestion about using PERF_SAMPLE_AUX.
> IIUC, SAMPLE_AUX will go to AUX ring buffer, which is more memory and more
> challenging when correlating and presenting the pipeline details for each IP.
> IMO, having a new sample type can be useful to capture the pipeline data
> both in perf_sample_data and if _AUX is enabled, can be made to push to
> AUX buffer.
OK, I didn't think SAMPLE_AUX and the aux ring buffer were
interdependent, sorry.
Thanks,
Kim
^ permalink raw reply
* Re: [PATCH] powerpc/64: Fix section mismatch warnings.
From: Michal Suchánek @ 2020-03-26 19:33 UTC (permalink / raw)
To: Naveen N. Rao
Cc: linux-kernel, Paul Mackerras, Greg Kroah-Hartman, Thomas Gleixner,
linuxppc-dev, Allison Randal
In-Reply-To: <1585245057.hh0vutz28g.naveen@linux.ibm.com>
On Thu, Mar 26, 2020 at 11:22:03PM +0530, Naveen N. Rao wrote:
> Michal Suchanek wrote:
> > Fixes the following warnings:
> >
> > WARNING: vmlinux.o(.text+0x2d24): Section mismatch in reference from the variable __boot_from_prom to the function .init.text:prom_init()
> > The function __boot_from_prom() references
> > the function __init prom_init().
> > This is often because __boot_from_prom lacks a __init
> > annotation or the annotation of prom_init is wrong.
> >
> > WARNING: vmlinux.o(.text+0x2fd0): Section mismatch in reference from the variable start_here_common to the function .init.text:start_kernel()
> > The function start_here_common() references
> > the function __init start_kernel().
> > This is often because start_here_common lacks a __init
> > annotation or the annotation of start_kernel is wrong.
> >
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> > arch/powerpc/kernel/head_64.S | 4 ++++
> > 1 file changed, 4 insertions(+)
>
> Michael committed a similar patch just earlier today:
> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=6eeb9b3b9ce588f14a697737a30d0702b5a20293
Missed it because it did not reach master yet.
Thanks
Michal
^ permalink raw reply
* [PATCH] Fix 4 "[v3, 28/32] powerpc/64s: interrupt implement exit logic in C"
From: Nicholas Piggin @ 2020-03-26 19:10 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
The return-to-kernel path has to replay any soft-pending interrupts if it is
returning to a context that had interrupts soft-enabled. It has to do this
carefully and avoid plain enabling interrupts if this is an irq context,
which can cause multiple nesting of interrupts on the stack, and other
unexpected issues.
The code which avoided this case got the soft-mask state wrong, and
marked interrupts as enabled before going around again to retry. This
seems to be mostly harmless except when PREEMPT=y, this calls
preempt_schedule_irq with irqs apparently enabled and runs into a BUG
in kernel/sched/core.c
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/syscall_64.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/syscall_64.c b/arch/powerpc/kernel/syscall_64.c
index 049608d811c7..cf06eb443a80 100644
--- a/arch/powerpc/kernel/syscall_64.c
+++ b/arch/powerpc/kernel/syscall_64.c
@@ -342,11 +342,15 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
trace_hardirqs_off();
local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
/*
- * Can't local_irq_enable in case we are in interrupt
- * context. Must replay directly.
+ * Can't local_irq_restore to replay if we were in
+ * interrupt context. Must replay directly.
*/
- replay_soft_interrupts();
- irq_soft_mask_set(flags);
+ if (irqs_disabled_flags(flags)) {
+ replay_soft_interrupts();
+ } else {
+ local_irq_restore(flags);
+ local_irq_save(flags);
+ }
/* Took an interrupt, may have more exit work to do. */
goto again;
}
--
2.23.0
^ permalink raw reply related
* [PATCH v4] powerpc/pseries: Handle UE event for memcpy_mcsafe
From: Ganesh Goudar @ 2020-03-26 18:49 UTC (permalink / raw)
To: mpe, linuxppc-dev; +Cc: mahesh, santosh, Ganesh Goudar
memcpy_mcsafe has been implemented for power machines which is used
by pmem infrastructure, so that an UE encountered during memcpy from
pmem devices would not result in panic instead a right error code
is returned. The implementation expects machine check handler to ignore
the event and set nip to continue the execution from fixup code.
Appropriate changes are already made to powernv machine check handler,
make similar changes to pseries machine check handler to ignore the
the event and set nip to continue execution at the fixup entry if we
hit UE at an instruction with a fixup entry.
while we are at it, have a common function which searches the exception
table entry and updates nip with fixup address, and any future common
changes can be made in this function that are valid for both architectures.
powernv changes are made by
commit 895e3dceeb97 ("powerpc/mce: Handle UE event for memcpy_mcsafe")
Reviewed-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Reviewed-by: Santosh S <santosh@fossix.org>
Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
---
V2: Fixes a trivial checkpatch error in commit msg.
V3: Use proper subject prefix.
V4: Rephrase the commit message.
Define a common function to update nip with fixup address.
---
arch/powerpc/include/asm/mce.h | 2 ++
arch/powerpc/kernel/mce.c | 14 ++++++++++++++
arch/powerpc/kernel/mce_power.c | 8 ++------
arch/powerpc/platforms/pseries/ras.c | 3 +++
4 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
index 6a6ddaabdb34..376a395daf32 100644
--- a/arch/powerpc/include/asm/mce.h
+++ b/arch/powerpc/include/asm/mce.h
@@ -218,6 +218,8 @@ extern void machine_check_queue_event(void);
extern void machine_check_print_event_info(struct machine_check_event *evt,
bool user_mode, bool in_guest);
unsigned long addr_to_pfn(struct pt_regs *regs, unsigned long addr);
+extern void mce_common_process_ue(struct pt_regs *regs,
+ struct mce_error_info *mce_err);
#ifdef CONFIG_PPC_BOOK3S_64
void flush_and_reload_slb(void);
#endif /* CONFIG_PPC_BOOK3S_64 */
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index 34c1001e9e8b..8077b5fb18a7 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -15,6 +15,7 @@
#include <linux/percpu.h>
#include <linux/export.h>
#include <linux/irq_work.h>
+#include <linux/extable.h>
#include <asm/machdep.h>
#include <asm/mce.h>
@@ -251,6 +252,19 @@ void machine_check_queue_event(void)
/* Queue irq work to process this event later. */
irq_work_queue(&mce_event_process_work);
}
+
+void mce_common_process_ue(struct pt_regs *regs,
+ struct mce_error_info *mce_err)
+{
+ const struct exception_table_entry *entry;
+
+ entry = search_kernel_exception_table(regs->nip);
+ if (entry) {
+ mce_err->ignore_event = true;
+ regs->nip = extable_fixup(entry);
+ }
+}
+
/*
* process pending MCE event from the mce event queue. This function will be
* called during syscall exit.
diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
index 1cbf7f1a4e3d..067b094bfeff 100644
--- a/arch/powerpc/kernel/mce_power.c
+++ b/arch/powerpc/kernel/mce_power.c
@@ -579,14 +579,10 @@ static long mce_handle_ue_error(struct pt_regs *regs,
struct mce_error_info *mce_err)
{
long handled = 0;
- const struct exception_table_entry *entry;
- entry = search_kernel_exception_table(regs->nip);
- if (entry) {
- mce_err->ignore_event = true;
- regs->nip = extable_fixup(entry);
+ mce_common_process_ue(regs, mce_err);
+ if (mce_err->ignore_event)
return 1;
- }
/*
* On specific SCOM read via MMIO we may get a machine check
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 43710b69e09e..1d1da639b8b7 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -558,6 +558,9 @@ static int mce_handle_error(struct pt_regs *regs, struct rtas_error_log *errp)
switch (mce_log->error_type) {
case MC_ERROR_TYPE_UE:
mce_err.error_type = MCE_ERROR_TYPE_UE;
+ mce_common_process_ue(regs, &mce_err);
+ if (mce_err.ignore_event)
+ disposition = RTAS_DISP_FULLY_RECOVERED;
switch (err_sub_type) {
case MC_ERROR_UE_IFETCH:
mce_err.u.ue_error_type = MCE_UE_ERROR_IFETCH;
--
2.17.2
^ permalink raw reply related
* Re: [PATCH] powerpc/64: Fix section mismatch warnings.
From: Naveen N. Rao @ 2020-03-26 17:52 UTC (permalink / raw)
To: Allison Randal, Benjamin Herrenschmidt, Christophe Leroy,
Greg Kroah-Hartman, linux-kernel, linuxppc-dev, Michael Ellerman,
Michal Suchanek, Paul Mackerras, Russell Currey, Thomas Gleixner
In-Reply-To: <20200326163742.28990-1-msuchanek@suse.de>
Michal Suchanek wrote:
> Fixes the following warnings:
>
> WARNING: vmlinux.o(.text+0x2d24): Section mismatch in reference from the variable __boot_from_prom to the function .init.text:prom_init()
> The function __boot_from_prom() references
> the function __init prom_init().
> This is often because __boot_from_prom lacks a __init
> annotation or the annotation of prom_init is wrong.
>
> WARNING: vmlinux.o(.text+0x2fd0): Section mismatch in reference from the variable start_here_common to the function .init.text:start_kernel()
> The function start_here_common() references
> the function __init start_kernel().
> This is often because start_here_common lacks a __init
> annotation or the annotation of start_kernel is wrong.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
> arch/powerpc/kernel/head_64.S | 4 ++++
> 1 file changed, 4 insertions(+)
Michael committed a similar patch just earlier today:
https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=6eeb9b3b9ce588f14a697737a30d0702b5a20293
- Naveen
^ permalink raw reply
* Re: [PATCH v2 12/12] powerpc/64s: system reset do not trace
From: Naveen N. Rao @ 2020-03-26 17:23 UTC (permalink / raw)
To: linuxppc-dev, Nicholas Piggin; +Cc: Mahesh Salgaonkar, Ganesh Goudar
In-Reply-To: <20200325103410.157573-13-npiggin@gmail.com>
Nicholas Piggin wrote:
> Similarly to the previous patch, do not trace system reset. This code
> is used when there is a crash or hang, and tracing disturbs the system
> more and has been known to crash in the crash handling path.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/kernel/traps.c | 5 +++++
> 1 file changed, 5 insertions(+)
For the ftrace bits:
Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 1845fd7e161a..ed7b7a6e2dc0 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -443,6 +443,9 @@ void system_reset_exception(struct pt_regs *regs)
> unsigned long hsrr0, hsrr1;
> bool nested = in_nmi();
> bool saved_hsrrs = false;
> + u8 ftrace_enabled = local_paca->ftrace_enabled;
> +
> + local_paca->ftrace_enabled = 0;
>
> /*
> * Avoid crashes in case of nested NMI exceptions. Recoverability
> @@ -524,6 +527,8 @@ void system_reset_exception(struct pt_regs *regs)
> if (!nested)
> nmi_exit();
>
> + local_paca->ftrace_enabled = ftrace_enabled;
> +
I suppose we could add helpers to save/restore the state. But, since we
only have these two users as of now, we can revisit that if the need
arises.
- Naveen
^ permalink raw reply
* Re: [PATCH] x86: Alias memset to __builtin_memset.
From: Nick Desaulniers @ 2020-03-26 17:18 UTC (permalink / raw)
To: Clement Courbet
Cc: maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), Kees Cook,
Borislav Petkov, Greg Kroah-Hartman, H. Peter Anvin, LKML,
clang-built-linux, Ingo Molnar, Paul Mackerras, Joe Perches,
Bernd Petrovitsch, Nathan Chancellor, linuxppc-dev,
Thomas Gleixner, Allison Randal
In-Reply-To: <20200326123841.134068-1-courbet@google.com>
On Thu, Mar 26, 2020 at 5:38 AM Clement Courbet <courbet@google.com> wrote:
>
> I discussed with the original authors who added freestanding to our
> build. It turns out that it was added globally but this was just to
> to workaround powerpc not compiling under clang, but they felt the
> fix was appropriate globally.
>
> Now Nick has dug up https://lkml.org/lkml/2019/8/29/1300, which
> advises against freestanding. Also, I've did some research and
> discovered that the original reason for using freestanding for
> powerpc has been fixed here:
> https://lore.kernel.org/linuxppc-dev/20191119045712.39633-3-natechancellor@gmail.com/
>
> I'm going to remove -ffreestanding from downstream, so we don't really need
> this anymore, sorry for waisting people's time.
>
> I wonder if the freestanding fix from the aforementioned patch is really needed
> though. I think that clang is actually right to point out the issue.
> I don't see any reason why setjmp()/longjmp() are declared as taking longs
> rather than ints. The implementation looks like it only ever propagates the
> value (in longjmp) or sets it to 1 (in setjmp), and we only ever call longjmp
> with integer parameters. But I'm not a PowerPC expert, so I might
> be misreading the code.
>
>
> So it seems that we could just remove freestanding altogether and rewrite the
> code to:
>
> diff --git a/arch/powerpc/include/asm/setjmp.h b/arch/powerpc/include/asm/setjmp.h
> index 279d03a1eec6..7941ae68fe21 100644
> --- a/arch/powerpc/include/asm/setjmp.h
> +++ b/arch/powerpc/include/asm/setjmp.h
> @@ -12,7 +12,9 @@
>
> #define JMP_BUF_LEN 23
> -extern long setjmp(long *);
> -extern void longjmp(long *, long);
> +typedef long * jmp_buf;
> +
> +extern int setjmp(jmp_buf);
> +extern void longjmp(jmp_buf, int);
>
> I'm happy to send a patch for this, and get rid of more -ffreestanding.
> Opinions ?
Yes, I think the above diff and additionally cleaning up
-ffreestanding from arch/powerpc/kernel/Makefile and
arch/powerpc/xmon/Makefile would be a much better approach. If that's
good enough to fix the warning, I kind of can't believe we didn't spot
that in the original code review!
Actually, the god awful warning was:
./arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
built-in function 'setjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header <setjmp.h>.
[-Werror,-Wincomplete-setjmp-declaration]
extern long setjmp(long *) __attribute__((returns_twice));
^
So jmp_buf was missing, wasn't used, but also the long vs int confusion.
I tested the above diff, all calls to setjmp under arch/powerpc/ just
compare the return value against 0. Callers of longjmp just pass 1
for the final parameter. So the above changes should be no functional
change.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
* Re: [PATCH V2 1/3] mm/debug: Add tests validating arch page table helpers for core features
From: Zi Yan @ 2020-03-26 17:08 UTC (permalink / raw)
To: Anshuman Khandual
Cc: Heiko Carstens, linux-mm, Paul Mackerras, H. Peter Anvin,
linux-riscv, Will Deacon, linux-arch, linux-s390, x86,
Mike Rapoport, Christian Borntraeger, Ingo Molnar,
Catalin Marinas, linux-snps-arc, Vasily Gorbik, Borislav Petkov,
Paul Walmsley, Kirill A . Shutemov, Thomas Gleixner,
linux-arm-kernel, Vineet Gupta, linux-kernel, Palmer Dabbelt,
Andrew Morton, linuxppc-dev
In-Reply-To: <5b188e44-73d5-673c-8df1-f2c42b556cf9@arm.com>
[-- Attachment #1: Type: text/plain, Size: 12046 bytes --]
On 25 Mar 2020, at 22:18, Anshuman Khandual wrote:
> External email: Use caution opening links or attachments
>
>
> On 03/24/2020 06:59 PM, Zi Yan wrote:
>> On 24 Mar 2020, at 1:22, Anshuman Khandual wrote:
>>
>>> This adds new tests validating arch page table helpers for these following
>>> core memory features. These tests create and test specific mapping types at
>>> various page table levels.
>>>
>>> 1. SPECIAL mapping
>>> 2. PROTNONE mapping
>>> 3. DEVMAP mapping
>>> 4. SOFTDIRTY mapping
>>> 5. SWAP mapping
>>> 6. MIGRATION mapping
>>> 7. HUGETLB mapping
>>> 8. THP mapping
>>>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: Mike Rapoport <rppt@linux.ibm.com>
>>> Cc: Vineet Gupta <vgupta@synopsys.com>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>> Cc: Paul Mackerras <paulus@samba.org>
>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
>>> Cc: Vasily Gorbik <gor@linux.ibm.com>
>>> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: Ingo Molnar <mingo@redhat.com>
>>> Cc: Borislav Petkov <bp@alien8.de>
>>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>>> Cc: Kirill A. Shutemov <kirill@shutemov.name>
>>> Cc: Paul Walmsley <paul.walmsley@sifive.com>
>>> Cc: Palmer Dabbelt <palmer@dabbelt.com>
>>> Cc: linux-snps-arc@lists.infradead.org
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linuxppc-dev@lists.ozlabs.org
>>> Cc: linux-s390@vger.kernel.org
>>> Cc: linux-riscv@lists.infradead.org
>>> Cc: x86@kernel.org
>>> Cc: linux-arch@vger.kernel.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> ---
>>> mm/debug_vm_pgtable.c | 291 +++++++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 290 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
>>> index 98990a515268..15055a8f6478 100644
>>> --- a/mm/debug_vm_pgtable.c
>>> +++ b/mm/debug_vm_pgtable.c
>>> @@ -289,6 +289,267 @@ static void __init pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp,
>>> WARN_ON(pmd_bad(pmd));
>>> }
>>>
>>> +static void __init pte_special_tests(unsigned long pfn, pgprot_t prot)
>>> +{
>>> + pte_t pte = pfn_pte(pfn, prot);
>>> +
>>> + if (!IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL))
>>> + return;
>>> +
>>> + WARN_ON(!pte_special(pte_mkspecial(pte)));
>>> +}
>>> +
>>> +static void __init pte_protnone_tests(unsigned long pfn, pgprot_t prot)
>>> +{
>>> + pte_t pte = pfn_pte(pfn, prot);
>>> +
>>> + if (!IS_ENABLED(CONFIG_NUMA_BALANCING))
>>> + return;
>>> +
>>> + WARN_ON(!pte_protnone(pte));
>>> + WARN_ON(!pte_present(pte));
>>> +}
>>> +
>>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>> +static void __init pmd_protnone_tests(unsigned long pfn, pgprot_t prot)
>>> +{
>>> + pmd_t pmd = pfn_pmd(pfn, prot);
>>> +
>>> + if (!IS_ENABLED(CONFIG_NUMA_BALANCING))
>>> + return;
>>> +
>>> + WARN_ON(!pmd_protnone(pmd));
>>> + WARN_ON(!pmd_present(pmd));
>>> +}
>>> +#else
>>> +static void __init pmd_protnone_tests(unsigned long pfn, pgprot_t prot) { }
>>> +#endif
>>> +
>>> +#ifdef CONFIG_ARCH_HAS_PTE_DEVMAP
>>> +static void __init pte_devmap_tests(unsigned long pfn, pgprot_t prot)
>>> +{
>>> + pte_t pte = pfn_pte(pfn, prot);
>>> +
>>> + WARN_ON(!pte_devmap(pte_mkdevmap(pte)));
>>> +}
>>> +
>>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>> +static void __init pmd_devmap_tests(unsigned long pfn, pgprot_t prot)
>>> +{
>>> + pmd_t pmd = pfn_pmd(pfn, prot);
>>> +
>>> + WARN_ON(!pmd_devmap(pmd_mkdevmap(pmd)));
>>> +}
>>> +
>>> +#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
>>> +static void __init pud_devmap_tests(unsigned long pfn, pgprot_t prot)
>>> +{
>>> + pud_t pud = pfn_pud(pfn, prot);
>>> +
>>> + WARN_ON(!pud_devmap(pud_mkdevmap(pud)));
>>> +}
>>> +#else
>>> +static void __init pud_devmap_tests(unsigned long pfn, pgprot_t prot) { }
>>> +#endif
>>> +#else
>>> +static void __init pmd_devmap_tests(unsigned long pfn, pgprot_t prot) { }
>>> +static void __init pud_devmap_tests(unsigned long pfn, pgprot_t prot) { }
>>> +#endif
>>> +#else
>>> +static void __init pte_devmap_tests(unsigned long pfn, pgprot_t prot) { }
>>> +static void __init pmd_devmap_tests(unsigned long pfn, pgprot_t prot) { }
>>> +static void __init pud_devmap_tests(unsigned long pfn, pgprot_t prot) { }
>>> +#endif
>>> +
>>> +static void __init pte_soft_dirty_tests(unsigned long pfn, pgprot_t prot)
>>> +{
>>> + pte_t pte = pfn_pte(pfn, prot);
>>> +
>>> + if (!IS_ENABLED(CONFIG_HAVE_ARCH_SOFT_DIRTY))
>>> + return;
>>> +
>>> + WARN_ON(!pte_soft_dirty(pte_mksoft_dirty(pte)));
>>> + WARN_ON(pte_soft_dirty(pte_clear_soft_dirty(pte)));
>>> +}
>>> +
>>> +static void __init pte_swap_soft_dirty_tests(unsigned long pfn, pgprot_t prot)
>>> +{
>>> + pte_t pte = pfn_pte(pfn, prot);
>>> +
>>> + if (!IS_ENABLED(CONFIG_HAVE_ARCH_SOFT_DIRTY))
>>> + return;
>>> +
>>> + WARN_ON(!pte_swp_soft_dirty(pte_swp_mksoft_dirty(pte)));
>>> + WARN_ON(pte_swp_soft_dirty(pte_swp_clear_soft_dirty(pte)));
>>> +}
>>> +
>>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>> +static void __init pmd_soft_dirty_tests(unsigned long pfn, pgprot_t prot)
>>> +{
>>> + pmd_t pmd = pfn_pmd(pfn, prot);
>>> +
>>> + if (!IS_ENABLED(CONFIG_HAVE_ARCH_SOFT_DIRTY))
>>> + return;
>>> +
>>> + WARN_ON(!pmd_soft_dirty(pmd_mksoft_dirty(pmd)));
>>> + WARN_ON(pmd_soft_dirty(pmd_clear_soft_dirty(pmd)));
>>> +}
>>> +
>>> +static void __init pmd_swap_soft_dirty_tests(unsigned long pfn, pgprot_t prot)
>>> +{
>>> + pmd_t pmd = pfn_pmd(pfn, prot);
>>> +
>>> + if (!IS_ENABLED(CONFIG_HAVE_ARCH_SOFT_DIRTY) ||
>>> + !IS_ENABLED(CONFIG_ARCH_ENABLE_THP_MIGRATION))
>>> + return;
>>> +
>>> + WARN_ON(!pmd_swp_soft_dirty(pmd_swp_mksoft_dirty(pmd)));
>>> + WARN_ON(pmd_swp_soft_dirty(pmd_swp_clear_soft_dirty(pmd)));
>>> +}
>>> +#else
>>> +static void __init pmd_soft_dirty_tests(unsigned long pfn, pgprot_t prot) { }
>>> +static void __init pmd_swap_soft_dirty_tests(unsigned long pfn, pgprot_t prot)
>>> +{
>>> +}
>>> +#endif
>>> +
>>> +static void __init pte_swap_tests(unsigned long pfn, pgprot_t prot)
>>> +{
>>> + swp_entry_t swp;
>>> + pte_t pte;
>>> +
>>> + pte = pfn_pte(pfn, prot);
>>> + swp = __pte_to_swp_entry(pte);
>>> + WARN_ON(!pte_same(pte, __swp_entry_to_pte(swp)));
>>> +}
>>> +
>>> +#ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
>>> +static void __init pmd_swap_tests(unsigned long pfn, pgprot_t prot)
>>> +{
>>> + swp_entry_t swp;
>>> + pmd_t pmd;
>>> +
>>> + pmd = pfn_pmd(pfn, prot);
>>> + swp = __pmd_to_swp_entry(pmd);
>>> + WARN_ON(!pmd_same(pmd, __swp_entry_to_pmd(swp)));
>>> +}
>>> +#else
>>> +static void __init pmd_swap_tests(unsigned long pfn, pgprot_t prot) { }
>>> +#endif
>>> +
>>> +static void __init swap_migration_tests(void)
>>> +{
>>> + struct page *page;
>>> + swp_entry_t swp;
>>> +
>>> + if (!IS_ENABLED(CONFIG_MIGRATION))
>>> + return;
>>> + /*
>>> + * swap_migration_tests() requires a dedicated page as it needs to
>>> + * be locked before creating a migration entry from it. Locking the
>>> + * page that actually maps kernel text ('start_kernel') can be real
>>> + * problematic. Lets allocate a dedicated page explicitly for this
>>> + * purpose that will be freed subsequently.
>>> + */
>>> + page = alloc_page(GFP_KERNEL);
>>> + if (!page) {
>>> + pr_err("page allocation failed\n");
>>> + return;
>>> + }
>>> +
>>> + /*
>>> + * make_migration_entry() expects given page to be
>>> + * locked, otherwise it stumbles upon a BUG_ON().
>>> + */
>>> + __SetPageLocked(page);
>>> + swp = make_migration_entry(page, 1);
>>> + WARN_ON(!is_migration_entry(swp));
>>> + WARN_ON(!is_write_migration_entry(swp));
>>> +
>>> + make_migration_entry_read(&swp);
>>> + WARN_ON(!is_migration_entry(swp));
>>> + WARN_ON(is_write_migration_entry(swp));
>>> +
>>> + swp = make_migration_entry(page, 0);
>>> + WARN_ON(!is_migration_entry(swp));
>>> + WARN_ON(is_write_migration_entry(swp));
>>> + __ClearPageLocked(page);
>>> + __free_page(page);
>>> +}
>>> +
>>> +#ifdef CONFIG_HUGETLB_PAGE
>>> +static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot)
>>> +{
>>> + struct page *page;
>>> + pte_t pte;
>>> +
>>> + /*
>>> + * Accessing the page associated with the pfn is safe here,
>>> + * as it was previously derived from a real kernel symbol.
>>> + */
>>> + page = pfn_to_page(pfn);
>>> + pte = mk_huge_pte(page, prot);
>>> +
>>> + WARN_ON(!huge_pte_dirty(huge_pte_mkdirty(pte)));
>>> + WARN_ON(!huge_pte_write(huge_pte_mkwrite(huge_pte_wrprotect(pte))));
>>> + WARN_ON(huge_pte_write(huge_pte_wrprotect(huge_pte_mkwrite(pte))));
>>> +
>>> +#ifdef CONFIG_ARCH_WANT_GENERAL_HUGETLB
>>> + pte = pfn_pte(pfn, prot);
>>> +
>>> + WARN_ON(!pte_huge(pte_mkhuge(pte)));
>>> +#endif
>>> +}
>>> +#else
>>> +static void __init hugetlb_basic_tests(unsigned long pfn, pgprot_t prot) { }
>>> +#endif
>>> +
>>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>>> +static void __init pmd_thp_tests(unsigned long pfn, pgprot_t prot)
>>> +{
>>> + pmd_t pmd;
>>> +
>>> + /*
>>> + * pmd_trans_huge() and pmd_present() must return positive
>>> + * after MMU invalidation with pmd_mknotpresent().
>>> + */
>>> + pmd = pfn_pmd(pfn, prot);
>>> + WARN_ON(!pmd_trans_huge(pmd_mkhuge(pmd)));
>>> +
>>> +#ifndef __HAVE_ARCH_PMDP_INVALIDATE
>>> + WARN_ON(!pmd_trans_huge(pmd_mknotpresent(pmd_mkhuge(pmd))));
>>> + WARN_ON(!pmd_present(pmd_mknotpresent(pmd_mkhuge(pmd))));
>>> +#endif
>>
>> I think we need a better comment here, because requiring pmd_trans_huge() and
>> pmd_present() returning true after pmd_mknotpresent() is not straightforward.
>
> Thats right.
>
>>
>> According to Andrea Arcangeli’s email (https://lore.kernel.org/linux-mm/20181017020930.GN30832@redhat.com/),
>> This behavior is an optimization for transparent huge page.
>> pmd_trans_huge() must be true if pmd_page() returns you a valid THP to avoid
>> taking the pmd_lock when others walk over non transhuge pmds (i.e. there are no
>> THP allocated). Especially when we split a THP, removing the present bit from
>> the pmd, pmd_trans_huge() still needs to return true. pmd_present() should
>> be true whenever pmd_trans_huge() returns true.
>
> Sure, will modify the existing comment here like this.
>
> /*
> * pmd_trans_huge() and pmd_present() must return positive after
> * MMU invalidation with pmd_mknotpresent(). This behavior is an
> * optimization for transparent huge page. pmd_trans_huge() must
> * be true if pmd_page() returns a valid THP to avoid taking the
> * pmd_lock when others walk over non transhuge pmds (i.e. there
> * are no THP allocated). Especially when splitting a THP and
> * removing the present bit from the pmd, pmd_trans_huge() still
> * needs to return true. pmd_present() should be true whenever
> * pmd_trans_huge() returns true.
> */
>
>>
>> I think it is also worth either putting Andres’s email or the link to it
>> in the rst file in your 3rd patch. It is a good documentation for this special
>> case.
>
> Makes sense. Will update Andrea's email link in the .rst file as well.
>
Look good to me. Thanks.
Reviewed-by: Zi Yan <ziy@nvidia.com>
—
Best Regards,
Yan Zi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]
^ permalink raw reply
* [PATCH] powerpc/64: Fix section mismatch warnings.
From: Michal Suchanek @ 2020-03-26 16:37 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Christophe Leroy, Greg Kroah-Hartman, Allison Randal,
Thomas Gleixner, Russell Currey, Michal Suchanek, linuxppc-dev,
linux-kernel
Fixes the following warnings:
WARNING: vmlinux.o(.text+0x2d24): Section mismatch in reference from the variable __boot_from_prom to the function .init.text:prom_init()
The function __boot_from_prom() references
the function __init prom_init().
This is often because __boot_from_prom lacks a __init
annotation or the annotation of prom_init is wrong.
WARNING: vmlinux.o(.text+0x2fd0): Section mismatch in reference from the variable start_here_common to the function .init.text:start_kernel()
The function start_here_common() references
the function __init start_kernel().
This is often because start_here_common lacks a __init
annotation or the annotation of start_kernel is wrong.
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
arch/powerpc/kernel/head_64.S | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
index ad79fddb974d..4afb82f1b7c7 100644
--- a/arch/powerpc/kernel/head_64.S
+++ b/arch/powerpc/kernel/head_64.S
@@ -537,6 +537,7 @@ __start_initialization_multiplatform:
b __after_prom_start
#endif /* CONFIG_PPC_BOOK3E */
+__REF
__boot_from_prom:
#ifdef CONFIG_PPC_OF_BOOT_TRAMPOLINE
/* Save parameters */
@@ -574,6 +575,7 @@ __boot_from_prom:
/* We never return. We also hit that trap if trying to boot
* from OF while CONFIG_PPC_OF_BOOT_TRAMPOLINE isn't selected */
trap
+ .previous
__after_prom_start:
#ifdef CONFIG_RELOCATABLE
@@ -980,6 +982,7 @@ start_here_multiplatform:
.previous
/* This is where all platforms converge execution */
+__REF
start_here_common:
/* relocation is on at this point */
std r1,PACAKSAVE(r13)
@@ -1001,6 +1004,7 @@ start_here_common:
/* Not reached */
trap
EMIT_BUG_ENTRY 0b, __FILE__, __LINE__, 0
+ .previous
/*
* We put a few things here that have to be page-aligned.
--
2.16.4
^ permalink raw reply related
* Re: [powerpc] Intermittent crashes ( link_path_walk) with linux-next
From: Sachin Sant @ 2020-03-26 16:24 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, Linux Next Mailing List, linuxppc-dev
In-Reply-To: <20200326134925.GP23230@ZenIV.linux.org.uk>
> On 26-Mar-2020, at 7:19 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Thu, Mar 26, 2020 at 10:40:06PM +1100, Michael Ellerman wrote:
>
>>> The code in question (link_path_walk() in fs/namei.c ) was recently changed by
>>> following commit:
>>>
>>> commit 881386f7e46a:
>>> link_path_walk(): sample parent's i_uid and i_mode for the last component
>>
>> That and about 10 other commits.
>>
>> Unless Al can give us a clue we'll need to bisect.
>
> Already fixed yesterday. It's not link_path_walk(), it's handle_dots()
> ignoring an error returned by step_into().
>
> commit 5e3c3570ec97 is the broken one; commit 20971012f63e is its variant with the
> fix folded in. So next-20200325 has the bug and next-20200326 should have it
> fixed. Could you check the current -next and see if you still observe that crap?
Thanks Al for the information.
I confirm that today’s next tree (20200326) work for me. I can no longer recreate this
problem.
Thanks
-Sachin
^ permalink raw reply
* Re: [PATCH v2] mm/sparse: Fix kernel crash with pfn_section_valid check
From: Pankaj Gupta @ 2020-03-26 16:04 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Sachin Sant, Michal Hocko, Baoquan He, David Hildenbrand,
linux-kernel, stable, Mike Rapoport, linux-mm, Wei Yang,
Andrew Morton, linuxppc-dev, Dan Williams, Oscar Salvador
In-Reply-To: <20200326133235.343616-1-aneesh.kumar@linux.ibm.com>
>
> Fixes the below crash
>
> BUG: Kernel NULL pointer dereference on read at 0x00000000
> Faulting instruction address: 0xc000000000c3447c
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> CPU: 11 PID: 7519 Comm: lt-ndctl Not tainted 5.6.0-rc7-autotest #1
> ...
> NIP [c000000000c3447c] vmemmap_populated+0x98/0xc0
> LR [c000000000088354] vmemmap_free+0x144/0x320
> Call Trace:
> section_deactivate+0x220/0x240
> __remove_pages+0x118/0x170
> arch_remove_memory+0x3c/0x150
> memunmap_pages+0x1cc/0x2f0
> devm_action_release+0x30/0x50
> release_nodes+0x2f8/0x3e0
> device_release_driver_internal+0x168/0x270
> unbind_store+0x130/0x170
> drv_attr_store+0x44/0x60
> sysfs_kf_write+0x68/0x80
> kernfs_fop_write+0x100/0x290
> __vfs_write+0x3c/0x70
> vfs_write+0xcc/0x240
> ksys_write+0x7c/0x140
> system_call+0x5c/0x68
>
> The crash is due to NULL dereference at
>
> test_bit(idx, ms->usage->subsection_map); due to ms->usage = NULL; in pfn_section_valid()
>
> With commit: d41e2f3bd546 ("mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case")
> section_mem_map is set to NULL after depopulate_section_mem(). This
> was done so that pfn_page() can work correctly with kernel config that disables
> SPARSEMEM_VMEMMAP. With that config pfn_to_page does
>
> __section_mem_map_addr(__sec) + __pfn;
> where
>
> static inline struct page *__section_mem_map_addr(struct mem_section *section)
> {
> unsigned long map = section->section_mem_map;
> map &= SECTION_MAP_MASK;
> return (struct page *)map;
> }
>
> Now with SPASEMEM_VMEMAP enabled, mem_section->usage->subsection_map is used to
> check the pfn validity (pfn_valid()). Since section_deactivate release
> mem_section->usage if a section is fully deactivated, pfn_valid() check after
> a subsection_deactivate cause a kernel crash.
>
> static inline int pfn_valid(unsigned long pfn)
> {
> ...
> return early_section(ms) || pfn_section_valid(ms, pfn);
> }
>
> where
>
> static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
> {
> int idx = subsection_map_index(pfn);
>
> return test_bit(idx, ms->usage->subsection_map);
> }
>
> Avoid this by clearing SECTION_HAS_MEM_MAP when mem_section->usage is freed.
> For architectures like ppc64 where large pages are used for vmmemap mapping (16MB),
> a specific vmemmap mapping can cover multiple sections. Hence before a vmemmap
> mapping page can be freed, the kernel needs to make sure there are no valid sections
> within that mapping. Clearing the section valid bit before
> depopulate_section_memap enables this.
>
> Fixes: d41e2f3bd546 ("mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case")
> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Wei Yang <richardw.yang@linux.intel.com>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: Mike Rapoport <rppt@linux.ibm.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
> mm/sparse.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/mm/sparse.c b/mm/sparse.c
> index aadb7298dcef..65599e8bd636 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -781,6 +781,12 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
> ms->usage = NULL;
> }
> memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
> + /*
> + * Mark the section invalid so that valid_section()
> + * return false. This prevents code from dereferencing
> + * ms->usage array.
> + */
> + ms->section_mem_map &= ~SECTION_HAS_MEM_MAP;
> }
>
> if (section_is_early && memmap)
> --
Agree with Michal, comment for clearing the section would be nicer.
Fix looks good to me.
Acked-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH V2 0/3] mm/debug: Add more arch page table helper tests
From: Christophe Leroy @ 2020-03-26 15:23 UTC (permalink / raw)
To: Anshuman Khandual, linux-mm
Cc: linux-doc, Heiko Carstens, Paul Mackerras, H. Peter Anvin,
linux-riscv, Will Deacon, linux-arch, linux-s390, Jonathan Corbet,
x86, Mike Rapoport, Christian Borntraeger, Ingo Molnar,
Catalin Marinas, linux-snps-arc, Vasily Gorbik, Borislav Petkov,
Paul Walmsley, Kirill A . Shutemov, Thomas Gleixner,
linux-arm-kernel, Vineet Gupta, linux-kernel, Palmer Dabbelt,
Andrew Morton, linuxppc-dev
In-Reply-To: <2bb4badc-2b7a-e15d-a99b-b1bd38c9d9bf@arm.com>
Le 26/03/2020 à 03:23, Anshuman Khandual a écrit :
>
>
> On 03/24/2020 10:52 AM, Anshuman Khandual wrote:
>> This series adds more arch page table helper tests. The new tests here are
>> either related to core memory functions and advanced arch pgtable helpers.
>> This also creates a documentation file enlisting all expected semantics as
>> suggested by Mike Rapoport (https://lkml.org/lkml/2020/1/30/40).
>>
>> This series has been tested on arm64 and x86 platforms.
>
> If folks can test these patches out on remaining ARCH_HAS_DEBUG_VM_PGTABLE
> enabled platforms i.e s390, arc, powerpc (32 and 64), that will be really
> appreciated. Thank you.
>
On powerpc 8xx (PPC32), I get:
[ 53.338368] debug_vm_pgtable: debug_vm_pgtable: Validating
architecture page table helpers
[ 53.347403] ------------[ cut here ]------------
[ 53.351832] WARNING: CPU: 0 PID: 1 at mm/debug_vm_pgtable.c:647
debug_vm_pgtable+0x280/0x3f4
[ 53.360140] CPU: 0 PID: 1 Comm: swapper Not tainted
5.6.0-rc7-s3k-dev-01090-g92710e99881f #3544
[ 53.368718] NIP: c0777c04 LR: c0777bb8 CTR: 00000000
[ 53.373720] REGS: c9023df0 TRAP: 0700 Not tainted
(5.6.0-rc7-s3k-dev-01090-g92710e99881f)
[ 53.382042] MSR: 00029032 <EE,ME,IR,DR,RI> CR: 22000222 XER: 20000000
[ 53.388667]
[ 53.388667] GPR00: c0777bb8 c9023ea8 c6120000 00000001 1e410000
00000000 00000000 007641c9
[ 53.388667] GPR08: 00000000 00000001 00000000 ffffffff 82000222
00000000 c00039b8 00000000
[ 53.388667] GPR16: 00000000 00000000 00000000 fffffff0 065fc000
1e410000 c6600000 000001e4
[ 53.388667] GPR24: 000001d9 c062d14c c65fc000 c642d448 000006c9
00000000 c65f8000 c65fc040
[ 53.423400] NIP [c0777c04] debug_vm_pgtable+0x280/0x3f4
[ 53.428559] LR [c0777bb8] debug_vm_pgtable+0x234/0x3f4
[ 53.433593] Call Trace:
[ 53.436048] [c9023ea8] [c0777bb8] debug_vm_pgtable+0x234/0x3f4
(unreliable)
[ 53.442936] [c9023f28] [c00039e0] kernel_init+0x28/0x124
[ 53.448184] [c9023f38] [c000f174] ret_from_kernel_thread+0x14/0x1c
[ 53.454245] Instruction dump:
[ 53.457180] 41a20008 4bea3ed9 62890021 7d36b92e 7d36b82e 71290fd0
3149ffff 7d2a4910
[ 53.464838] 0f090000 5789077e 3149ffff 7d2a4910 <0f090000> 38c00000
38a00000 38800000
[ 53.472671] ---[ end trace fd5dd92744dc0065 ]---
[ 53.519778] Freeing unused kernel memory: 608K
^ 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