* [PATCH] PCI/AER: Move pci_uevent_ers() out of pci.h
From: Michael Ellerman @ 2018-02-08 12:20 UTC (permalink / raw)
To: bhelgaas; +Cc: linux-pci, linuxppc-dev, torvalds, ruscur, bryantly, jjalvare
There's no reason pci_uevent_ers() needs to be inline in pci.h, so
move it out to a C file.
Given it's used by AER the obvious location would be somewhere in
drivers/pci/pcie/aer, but because it's also used by powerpc EEH code
unfortunately that doesn't work in the case where EEH is enabled but
PCIEPORTBUS is not.
So for now put it in pci-driver.c, next to pci_uevent(), with an
appropriate #ifdef so it's not built if AER and EEH are both disabled.
While we're moving it also fix up the kernel doc comment for @pdev to
be accurate.
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
drivers/pci/pci-driver.c | 36 ++++++++++++++++++++++++++++++++++++
include/linux/pci.h | 38 +++-----------------------------------
2 files changed, 39 insertions(+), 35 deletions(-)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 3bed6beda051..f21e8b1bef80 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -1517,6 +1517,42 @@ static int pci_uevent(struct device *dev, struct kobj_uevent_env *env)
return 0;
}
+#if defined(CONFIG_PCIEAER) || defined(CONFIG_EEH)
+/**
+ * pci_uevent_ers - emit a uevent during recovery path of pci device
+ * @pdev: pci device undergoing error recovery
+ * @err_type: type of error event
+ */
+void pci_uevent_ers(struct pci_dev *pdev, enum pci_ers_result err_type)
+{
+ int idx = 0;
+ char *envp[3];
+
+ switch (err_type) {
+ case PCI_ERS_RESULT_NONE:
+ case PCI_ERS_RESULT_CAN_RECOVER:
+ envp[idx++] = "ERROR_EVENT=BEGIN_RECOVERY";
+ envp[idx++] = "DEVICE_ONLINE=0";
+ break;
+ case PCI_ERS_RESULT_RECOVERED:
+ envp[idx++] = "ERROR_EVENT=SUCCESSFUL_RECOVERY";
+ envp[idx++] = "DEVICE_ONLINE=1";
+ break;
+ case PCI_ERS_RESULT_DISCONNECT:
+ envp[idx++] = "ERROR_EVENT=FAILED_RECOVERY";
+ envp[idx++] = "DEVICE_ONLINE=0";
+ break;
+ default:
+ break;
+ }
+
+ if (idx > 0) {
+ envp[idx++] = NULL;
+ kobject_uevent_env(&pdev->dev.kobj, KOBJ_CHANGE, envp);
+ }
+}
+#endif
+
static int pci_bus_num_vf(struct device *dev)
{
return pci_num_vf(to_pci_dev(dev));
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 024a1beda008..19c1dbcff0c6 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2280,41 +2280,9 @@ static inline bool pci_is_thunderbolt_attached(struct pci_dev *pdev)
return false;
}
-/**
- * pci_uevent_ers - emit a uevent during recovery path of pci device
- * @pdev: pci device to check
- * @err_type: type of error event
- *
- */
-static inline void pci_uevent_ers(struct pci_dev *pdev,
- enum pci_ers_result err_type)
-{
- int idx = 0;
- char *envp[3];
-
- switch (err_type) {
- case PCI_ERS_RESULT_NONE:
- case PCI_ERS_RESULT_CAN_RECOVER:
- envp[idx++] = "ERROR_EVENT=BEGIN_RECOVERY";
- envp[idx++] = "DEVICE_ONLINE=0";
- break;
- case PCI_ERS_RESULT_RECOVERED:
- envp[idx++] = "ERROR_EVENT=SUCCESSFUL_RECOVERY";
- envp[idx++] = "DEVICE_ONLINE=1";
- break;
- case PCI_ERS_RESULT_DISCONNECT:
- envp[idx++] = "ERROR_EVENT=FAILED_RECOVERY";
- envp[idx++] = "DEVICE_ONLINE=0";
- break;
- default:
- break;
- }
-
- if (idx > 0) {
- envp[idx++] = NULL;
- kobject_uevent_env(&pdev->dev.kobj, KOBJ_CHANGE, envp);
- }
-}
+#if defined(CONFIG_PCIEAER) || defined(CONFIG_EEH)
+void pci_uevent_ers(struct pci_dev *pdev, enum pci_ers_result err_type);
+#endif
/* Provide the legacy pci_dma_* API */
#include <linux/pci-dma-compat.h>
--
2.14.1
^ permalink raw reply related
* Re: [PATCH v7 04/24] mm: Dont assume page-table invariance during faults
From: Laurent Dufour @ 2018-02-08 14:35 UTC (permalink / raw)
To: Matthew Wilcox
Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack, benh, mpe,
paulus, Thomas Gleixner, Ingo Molnar, hpa, Will Deacon,
Sergey Senozhatsky, Andrea Arcangeli, Alexei Starovoitov,
kemi.wang, sergey.senozhatsky.work, Daniel Jordan, linux-kernel,
linux-mm, haren, khandual, npiggin, bsingharora, Tim Chen,
linuxppc-dev, x86
In-Reply-To: <20180206202831.GB16511@bombadil.infradead.org>
On 06/02/2018 21:28, Matthew Wilcox wrote:
> On Tue, Feb 06, 2018 at 05:49:50PM +0100, Laurent Dufour wrote:
>> From: Peter Zijlstra <peterz@infradead.org>
>>
>> One of the side effects of speculating on faults (without holding
>> mmap_sem) is that we can race with free_pgtables() and therefore we
>> cannot assume the page-tables will stick around.
>>
>> Remove the reliance on the pte pointer.
>>
>> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>>
>> In most of the case pte_unmap_same() was returning 1, which meaning that
>> do_swap_page() should do its processing. So in most of the case there will
>> be no impact.
>>
>> Now regarding the case where pte_unmap_safe() was returning 0, and thus
>> do_swap_page return 0 too, this happens when the page has already been
>> swapped back. This may happen before do_swap_page() get called or while in
>> the call to do_swap_page(). In that later case, the check done when
>> swapin_readahead() returns will detect that case.
>>
>> The worst case would be that a page fault is occuring on 2 threads at the
>> same time on the same swapped out page. In that case one thread will take
>> much time looping in __read_swap_cache_async(). But in the regular page
>> fault path, this is even worse since the thread would wait for semaphore to
>> be released before starting anything.
>>
>> [Remove only if !CONFIG_SPECULATIVE_PAGE_FAULT]
>> Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
>
> I have a great deal of trouble connecting all of the words above to the
> contents of the patch.
Thanks for pushing forward here, this raised some doubts on my side.
I reviewed that part of code, and I think I could now change the way
pte_unmap_safe() is checking for the pte's value. Since we now have all the
needed details in the vm_fault structure, I will pass it to
pte_unamp_same() and deal with the VMA checks when locking for the pte as
it is done in the other part of the page fault handler by calling
pte_spinlock().
This means that this patch will be dropped, and pte_unmap_same() will become :
static inline int pte_unmap_same(struct vm_fault *vmf, int *same)
{
int ret = 0;
*same = 1;
#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)
if (sizeof(pte_t) > sizeof(unsigned long)) {
if (pte_spinlock(vmf)) {
*same = pte_same(*vmf->pte, vmf->orig_pte);
spin_unlock(vmf->ptl);
}
else
ret = VM_FAULT_RETRY;
}
#endif
pte_unmap(vmf->pte);
return ret;
}
Laurent.
>
>>
>> +#ifndef CONFIG_SPECULATIVE_PAGE_FAULT
>> /*
>> * handle_pte_fault chooses page fault handler according to an entry which was
>> * read non-atomically. Before making any commitment, on those architectures
>> @@ -2311,6 +2312,7 @@ static inline int pte_unmap_same(struct mm_struct *mm, pmd_t *pmd,
>> pte_unmap(page_table);
>> return same;
>> }
>> +#endif /* CONFIG_SPECULATIVE_PAGE_FAULT */
>>
>> static inline void cow_user_page(struct page *dst, struct page *src, unsigned long va, struct vm_area_struct *vma)
>> {
>> @@ -2898,11 +2900,13 @@ int do_swap_page(struct vm_fault *vmf)
>> swapcache = page;
>> }
>>
>> +#ifndef CONFIG_SPECULATIVE_PAGE_FAULT
>> if (!pte_unmap_same(vma->vm_mm, vmf->pmd, vmf->pte, vmf->orig_pte)) {
>> if (page)
>> put_page(page);
>> goto out;
>> }
>> +#endif
>>
>
> This feels to me like we want:
>
> #ifdef CONFIG_SPECULATIVE_PAGE_FAULT
> [current code]
> #else
> /*
> * Some words here which explains why we always want to return this
> * value if we support speculative page faults.
> */
> static inline int pte_unmap_same(struct mm_struct *mm, pmd_t *pmd,
> pte_t *page_table, pte_t orig_pte)
> {
> return 1;
> }
> #endif
>
> instead of cluttering do_swap_page with an ifdef.
>
^ permalink raw reply
* Re: [PATCH v7 04/24] mm: Dont assume page-table invariance during faults
From: Matthew Wilcox @ 2018-02-08 15:00 UTC (permalink / raw)
To: Laurent Dufour
Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack, benh, mpe,
paulus, Thomas Gleixner, Ingo Molnar, hpa, Will Deacon,
Sergey Senozhatsky, Andrea Arcangeli, Alexei Starovoitov,
kemi.wang, sergey.senozhatsky.work, Daniel Jordan, linux-kernel,
linux-mm, haren, khandual, npiggin, bsingharora, Tim Chen,
linuxppc-dev, x86
In-Reply-To: <484242d8-e632-9e39-5c99-2e1b4b3b69a5@linux.vnet.ibm.com>
On Thu, Feb 08, 2018 at 03:35:58PM +0100, Laurent Dufour wrote:
> I reviewed that part of code, and I think I could now change the way
> pte_unmap_safe() is checking for the pte's value. Since we now have all the
> needed details in the vm_fault structure, I will pass it to
> pte_unamp_same() and deal with the VMA checks when locking for the pte as
> it is done in the other part of the page fault handler by calling
> pte_spinlock().
This does indeed look much better! Thank you!
> This means that this patch will be dropped, and pte_unmap_same() will become :
>
> static inline int pte_unmap_same(struct vm_fault *vmf, int *same)
> {
> int ret = 0;
>
> *same = 1;
> #if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)
> if (sizeof(pte_t) > sizeof(unsigned long)) {
> if (pte_spinlock(vmf)) {
> *same = pte_same(*vmf->pte, vmf->orig_pte);
> spin_unlock(vmf->ptl);
> }
> else
> ret = VM_FAULT_RETRY;
> }
> #endif
> pte_unmap(vmf->pte);
> return ret;
> }
I'm not a huge fan of auxiliary return values. Perhaps we could do this
instead:
ret = pte_unmap_same(vmf);
if (ret != VM_FAULT_NOTSAME) {
if (page)
put_page(page);
goto out;
}
ret = 0;
(we have a lot of unused bits in VM_FAULT_, so adding a new one shouldn't
be a big deal)
^ permalink raw reply
* Re: [PATCH] PCI/AER: Move pci_uevent_ers() out of pci.h
From: Bryant G. Ly @ 2018-02-08 15:05 UTC (permalink / raw)
To: Michael Ellerman, bhelgaas
Cc: linux-pci, linuxppc-dev, torvalds, ruscur, jjalvare
In-Reply-To: <20180208122035.9305-1-mpe@ellerman.id.au>
On 2/8/18 6:20 AM, Michael Ellerman wrote:
> There's no reason pci_uevent_ers() needs to be inline in pci.h, so
> move it out to a C file.
>
> Given it's used by AER the obvious location would be somewhere in
> drivers/pci/pcie/aer, but because it's also used by powerpc EEH code
> unfortunately that doesn't work in the case where EEH is enabled but
> PCIEPORTBUS is not.
>
> So for now put it in pci-driver.c, next to pci_uevent(), with an
> appropriate #ifdef so it's not built if AER and EEH are both disabled.
>
> While we're moving it also fix up the kernel doc comment for @pdev to
> be accurate.
>
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> drivers/pci/pci-driver.c | 36 ++++++++++++++++++++++++++++++++++++
> include/linux/pci.h | 38 +++-----------------------------------
> 2 files changed, 39 insertions(+), 35 deletions(-)
Looks good, thanks for fixing it!
Signed-off-by: Bryant G. Ly <bryantly@linux.vnet.ibm.com>
-Bryant
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/mm: Fix crashes with PUD level hugetlb config
From: Aneesh Kumar K.V @ 2018-02-08 15:16 UTC (permalink / raw)
To: benh, paulus, mpe, linuxram; +Cc: linuxppc-dev
In-Reply-To: <20180208103442.22045-1-aneesh.kumar@linux.vnet.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:
> To support memory keys, we moved the hash pte slot information to the second
> half of the page table. This was ok with PTE entries at level 4 and level 3.
> We already allocate larger page table pages at those level to accomodate extra
> details. For level 4 we already have the extra space which was used to track
> 4k hash page table entry details and at pmd level the extra space was allocated
> to track the THP details.
>
> With hugetlbfs PTE, we used this extra space at the PMD level to store the
> slot details. But we also support hugetlbfs PTE at PUD leve and PUD level page
> didn't allocate extra space. This resulted in memory corruption.
>
> Fix this by allocating extra space at PUD level when HUGETLB is enabled. We
> may need further changes to allocate larger space at PMD level when we enable
> HUGETLB. That will be done in next patch.
>
> Fixes:bf9a95f9a6481bc6e(" powerpc: Free up four 64K PTE bits in 64K backed HPTE pages")
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Another fix, I still get random memory corruption with hugetlb test with
16G hugepage config.
commit f9484ac6b06bfacfeb82f1116bfc95e396fe7453
Author: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Date: Thu Feb 8 19:36:22 2018 +0530
powerpc/mm/hash64: Store the slot information at the right offset.
The hugetlb pte entries are at the PMD and PUD level. Use the right offset
for them to get the second half of the table.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
diff --git a/arch/powerpc/include/asm/book3s/64/hash-4k.h b/arch/powerpc/include/asm/book3s/64/hash-4k.h
index 949d691094a4..67c5475311ee 100644
--- a/arch/powerpc/include/asm/book3s/64/hash-4k.h
+++ b/arch/powerpc/include/asm/book3s/64/hash-4k.h
@@ -63,7 +63,8 @@ static inline int hash__hugepd_ok(hugepd_t hpd)
* keeping the prototype consistent across the two formats.
*/
static inline unsigned long pte_set_hidx(pte_t *ptep, real_pte_t rpte,
- unsigned int subpg_index, unsigned long hidx)
+ unsigned int subpg_index, unsigned long hidx,
+ int offset)
{
return (hidx << H_PAGE_F_GIX_SHIFT) &
(H_PAGE_F_SECOND | H_PAGE_F_GIX);
diff --git a/arch/powerpc/include/asm/book3s/64/hash-64k.h b/arch/powerpc/include/asm/book3s/64/hash-64k.h
index ee440fb3d240..3bcf269f8f55 100644
--- a/arch/powerpc/include/asm/book3s/64/hash-64k.h
+++ b/arch/powerpc/include/asm/book3s/64/hash-64k.h
@@ -45,7 +45,7 @@
* generic accessors and iterators here
*/
#define __real_pte __real_pte
-static inline real_pte_t __real_pte(pte_t pte, pte_t *ptep)
+static inline real_pte_t __real_pte(pte_t pte, pte_t *ptep, int offset)
{
real_pte_t rpte;
unsigned long *hidxp;
@@ -59,7 +59,7 @@ static inline real_pte_t __real_pte(pte_t pte, pte_t *ptep)
*/
smp_rmb();
- hidxp = (unsigned long *)(ptep + PTRS_PER_PTE);
+ hidxp = (unsigned long *)(ptep + offset);
rpte.hidx = *hidxp;
return rpte;
}
@@ -86,9 +86,10 @@ static inline unsigned long __rpte_to_hidx(real_pte_t rpte, unsigned long index)
* expected to modify the PTE bits accordingly and commit the PTE to memory.
*/
static inline unsigned long pte_set_hidx(pte_t *ptep, real_pte_t rpte,
- unsigned int subpg_index, unsigned long hidx)
+ unsigned int subpg_index,
+ unsigned long hidx, int offset)
{
- unsigned long *hidxp = (unsigned long *)(ptep + PTRS_PER_PTE);
+ unsigned long *hidxp = (unsigned long *)(ptep + offset);
rpte.hidx &= ~HIDX_BITS(0xfUL, subpg_index);
*hidxp = rpte.hidx | HIDX_BITS(HIDX_SHIFT_BY_ONE(hidx), subpg_index);
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 3c14663d457d..d92707aff762 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -350,7 +350,7 @@ extern unsigned long pci_io_base;
*/
#ifndef __real_pte
-#define __real_pte(e,p) ((real_pte_t){(e)})
+#define __real_pte(e, p, o) ((real_pte_t){(e)})
#define __rpte_to_pte(r) ((r).pte)
#define __rpte_to_hidx(r,index) (pte_val(__rpte_to_pte(r)) >> H_PAGE_F_GIX_SHIFT)
diff --git a/arch/powerpc/mm/hash64_4k.c b/arch/powerpc/mm/hash64_4k.c
index 5a69b51d08a3..d573d7d07f25 100644
--- a/arch/powerpc/mm/hash64_4k.c
+++ b/arch/powerpc/mm/hash64_4k.c
@@ -55,7 +55,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
* need to add in 0x1 if it's a read-only user page
*/
rflags = htab_convert_pte_flags(new_pte);
- rpte = __real_pte(__pte(old_pte), ptep);
+ rpte = __real_pte(__pte(old_pte), ptep, PTRS_PER_PTE);
if (cpu_has_feature(CPU_FTR_NOEXECUTE) &&
!cpu_has_feature(CPU_FTR_COHERENT_ICACHE))
@@ -117,7 +117,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
return -1;
}
new_pte = (new_pte & ~_PAGE_HPTEFLAGS) | H_PAGE_HASHPTE;
- new_pte |= pte_set_hidx(ptep, rpte, 0, slot);
+ new_pte |= pte_set_hidx(ptep, rpte, 0, slot, PTRS_PER_PTE);
}
*ptep = __pte(new_pte & ~H_PAGE_BUSY);
return 0;
diff --git a/arch/powerpc/mm/hash64_64k.c b/arch/powerpc/mm/hash64_64k.c
index 2253bbc6a599..e601d95c3b20 100644
--- a/arch/powerpc/mm/hash64_64k.c
+++ b/arch/powerpc/mm/hash64_64k.c
@@ -86,7 +86,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
subpg_index = (ea & (PAGE_SIZE - 1)) >> shift;
vpn = hpt_vpn(ea, vsid, ssize);
- rpte = __real_pte(__pte(old_pte), ptep);
+ rpte = __real_pte(__pte(old_pte), ptep, PTRS_PER_PTE);
/*
*None of the sub 4k page is hashed
*/
@@ -214,7 +214,7 @@ int __hash_page_4K(unsigned long ea, unsigned long access, unsigned long vsid,
return -1;
}
- new_pte |= pte_set_hidx(ptep, rpte, subpg_index, slot);
+ new_pte |= pte_set_hidx(ptep, rpte, subpg_index, slot, PTRS_PER_PTE);
new_pte |= H_PAGE_HASHPTE;
*ptep = __pte(new_pte & ~H_PAGE_BUSY);
@@ -262,7 +262,7 @@ int __hash_page_64K(unsigned long ea, unsigned long access,
} while (!pte_xchg(ptep, __pte(old_pte), __pte(new_pte)));
rflags = htab_convert_pte_flags(new_pte);
- rpte = __real_pte(__pte(old_pte), ptep);
+ rpte = __real_pte(__pte(old_pte), ptep, PTRS_PER_PTE);
if (cpu_has_feature(CPU_FTR_NOEXECUTE) &&
!cpu_has_feature(CPU_FTR_COHERENT_ICACHE))
@@ -327,7 +327,7 @@ int __hash_page_64K(unsigned long ea, unsigned long access,
}
new_pte = (new_pte & ~_PAGE_HPTEFLAGS) | H_PAGE_HASHPTE;
- new_pte |= pte_set_hidx(ptep, rpte, 0, slot);
+ new_pte |= pte_set_hidx(ptep, rpte, 0, slot, PTRS_PER_PTE);
}
*ptep = __pte(new_pte & ~H_PAGE_BUSY);
return 0;
diff --git a/arch/powerpc/mm/hugetlbpage-hash64.c b/arch/powerpc/mm/hugetlbpage-hash64.c
index 12511f5a015f..b320f5097a06 100644
--- a/arch/powerpc/mm/hugetlbpage-hash64.c
+++ b/arch/powerpc/mm/hugetlbpage-hash64.c
@@ -27,7 +27,7 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,
unsigned long vpn;
unsigned long old_pte, new_pte;
unsigned long rflags, pa, sz;
- long slot;
+ long slot, offset;
BUG_ON(shift != mmu_psize_defs[mmu_psize].shift);
@@ -63,7 +63,11 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,
} while(!pte_xchg(ptep, __pte(old_pte), __pte(new_pte)));
rflags = htab_convert_pte_flags(new_pte);
- rpte = __real_pte(__pte(old_pte), ptep);
+ if (unlikely(mmu_psize == MMU_PAGE_16G))
+ offset = PTRS_PER_PUD;
+ else
+ offset = PTRS_PER_PMD;
+ rpte = __real_pte(__pte(old_pte), ptep, offset);
sz = ((1UL) << shift);
if (!cpu_has_feature(CPU_FTR_COHERENT_ICACHE))
@@ -104,7 +108,7 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,
return -1;
}
- new_pte |= pte_set_hidx(ptep, rpte, 0, slot);
+ new_pte |= pte_set_hidx(ptep, rpte, 0, slot, offset);
}
/*
diff --git a/arch/powerpc/mm/tlb_hash64.c b/arch/powerpc/mm/tlb_hash64.c
index 881ebd53ffc2..9b23f12e863c 100644
--- a/arch/powerpc/mm/tlb_hash64.c
+++ b/arch/powerpc/mm/tlb_hash64.c
@@ -51,7 +51,7 @@ void hpte_need_flush(struct mm_struct *mm, unsigned long addr,
unsigned int psize;
int ssize;
real_pte_t rpte;
- int i;
+ int i, offset;
i = batch->index;
@@ -67,6 +67,10 @@ void hpte_need_flush(struct mm_struct *mm, unsigned long addr,
psize = get_slice_psize(mm, addr);
/* Mask the address for the correct page size */
addr &= ~((1UL << mmu_psize_defs[psize].shift) - 1);
+ if (unlikely(psize == MMU_PAGE_16G))
+ offset = PTRS_PER_PUD;
+ else
+ offset = PTRS_PER_PMD;
#else
BUG();
psize = pte_pagesize_index(mm, addr, pte); /* shutup gcc */
@@ -78,6 +82,7 @@ void hpte_need_flush(struct mm_struct *mm, unsigned long addr,
* support 64k pages, this might be different from the
* hardware page size encoded in the slice table. */
addr &= PAGE_MASK;
+ offset = PTRS_PER_PTE;
}
@@ -91,7 +96,7 @@ void hpte_need_flush(struct mm_struct *mm, unsigned long addr,
}
WARN_ON(vsid == 0);
vpn = hpt_vpn(addr, vsid, ssize);
- rpte = __real_pte(__pte(pte), ptep);
+ rpte = __real_pte(__pte(pte), ptep, offset);
/*
* Check if we have an active batch on this CPU. If not, just
^ permalink raw reply related
* Re: [PATCH v7 04/24] mm: Dont assume page-table invariance during faults
From: Laurent Dufour @ 2018-02-08 17:14 UTC (permalink / raw)
To: Matthew Wilcox
Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack, benh, mpe,
paulus, Thomas Gleixner, Ingo Molnar, hpa, Will Deacon,
Sergey Senozhatsky, Andrea Arcangeli, Alexei Starovoitov,
kemi.wang, sergey.senozhatsky.work, Daniel Jordan, linux-kernel,
linux-mm, haren, khandual, npiggin, bsingharora, Tim Chen,
linuxppc-dev, x86
In-Reply-To: <20180208150025.GD15846@bombadil.infradead.org>
On 08/02/2018 16:00, Matthew Wilcox wrote:
> On Thu, Feb 08, 2018 at 03:35:58PM +0100, Laurent Dufour wrote:
>> I reviewed that part of code, and I think I could now change the way
>> pte_unmap_safe() is checking for the pte's value. Since we now have all the
>> needed details in the vm_fault structure, I will pass it to
>> pte_unamp_same() and deal with the VMA checks when locking for the pte as
>> it is done in the other part of the page fault handler by calling
>> pte_spinlock().
>
> This does indeed look much better! Thank you!
>
>> This means that this patch will be dropped, and pte_unmap_same() will become :
>>
>> static inline int pte_unmap_same(struct vm_fault *vmf, int *same)
>> {
>> int ret = 0;
>>
>> *same = 1;
>> #if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)
>> if (sizeof(pte_t) > sizeof(unsigned long)) {
>> if (pte_spinlock(vmf)) {
>> *same = pte_same(*vmf->pte, vmf->orig_pte);
>> spin_unlock(vmf->ptl);
>> }
>> else
>> ret = VM_FAULT_RETRY;
>> }
>> #endif
>> pte_unmap(vmf->pte);
>> return ret;
>> }
>
> I'm not a huge fan of auxiliary return values. Perhaps we could do this
> instead:
>
> ret = pte_unmap_same(vmf);
> if (ret != VM_FAULT_NOTSAME) {
> if (page)
> put_page(page);
> goto out;
> }
> ret = 0;
>
> (we have a lot of unused bits in VM_FAULT_, so adding a new one shouldn't
> be a big deal)
I do agree, using an auxiliary return value is not a good idea.
What about the following changes based on your suggestion ?
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 7de4323b9e89..0cd31a37bb3d 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1212,6 +1212,7 @@ static inline void clear_page_pfmemalloc(struct page *page)
#define VM_FAULT_NEEDDSYNC 0x2000 /* ->fault did not modify page tables
* and needs fsync() to complete (for
* synchronous page faults in DAX) */
+#define VM_FAULT_PTNOTSAME 0x4000 /* Page table entries have changed */
#define VM_FAULT_ERROR (VM_FAULT_OOM | VM_FAULT_SIGBUS | VM_FAULT_SIGSEGV | \
VM_FAULT_HWPOISON | VM_FAULT_HWPOISON_LARGE | \
diff --git a/mm/memory.c b/mm/memory.c
index b7da99c74fef..c9b419f8e4c5 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2433,21 +2433,30 @@ static inline bool pte_map_lock(struct vm_fault *vmf)
* parts, do_swap_page must check under lock before unmapping the pte and
* proceeding (but do_wp_page is only called after already making such a check;
* and do_anonymous_page can safely check later on).
+ *
+ * pte_unmap_same() returns:
+ * 0 if the PTE are the same
+ * VM_FAULT_PTNOTSAME if the PTE are different
+ * VM_FAULT_RETRY if the VMA has changed in our back during
+ * a speculative page fault handling.
*/
-static inline int pte_unmap_same(struct mm_struct *mm, pmd_t *pmd,
- pte_t *page_table, pte_t orig_pte)
+static inline int pte_unmap_same(struct vm_fault *vmf)
{
- int same = 1;
+ int ret = 0;
+
#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT)
if (sizeof(pte_t) > sizeof(unsigned long)) {
- spinlock_t *ptl = pte_lockptr(mm, pmd);
- spin_lock(ptl);
- same = pte_same(*page_table, orig_pte);
- spin_unlock(ptl);
+ if (pte_spinlock(vmf)) {
+ if (!pte_same(*vmf->pte, vmf->orig_pte))
+ ret = VM_FAULT_PTNOTSAME;
+ spin_unlock(vmf->ptl);
+ }
+ else
+ ret = VM_FAULT_RETRY;
}
#endif
- pte_unmap(page_table);
- return same;
+ pte_unmap(vmf->pte);
+ return ret;
}
static inline void cow_user_page(struct page *dst, struct page *src, unsigned long va, struct vm_area_struct *vma)
@@ -3037,7 +3046,7 @@ int do_swap_page(struct vm_fault *vmf)
pte_t pte;
int locked;
int exclusive = 0;
- int ret = 0;
+ int ret;
bool vma_readahead = swap_use_vma_readahead();
if (vma_readahead) {
@@ -3045,9 +3054,16 @@ int do_swap_page(struct vm_fault *vmf)
swapcache = page;
}
- if (!pte_unmap_same(vma->vm_mm, vmf->pmd, vmf->pte, vmf->orig_pte)) {
+ ret = pte_unmap_same(vmf);
+ if (ret) {
if (page)
put_page(page);
+ /*
+ * In the case the PTE are different, meaning that the
+ * page has already been processed by another CPU, we return 0.
+ */
+ if (ret == VM_FAULT_PTNOTSAME)
+ ret = 0;
goto out;
}
Thanks,
Laurent.
^ permalink raw reply related
* Re: [RFC][PATCH bpf 1/2] bpf: allow 64-bit offsets for bpf function calls
From: Alexei Starovoitov @ 2018-02-08 17:08 UTC (permalink / raw)
To: Sandipan Das, daniel; +Cc: netdev, linuxppc-dev, mpe, naveen.n.rao
In-Reply-To: <20180208120306.2568-1-sandipan@linux.vnet.ibm.com>
On 2/8/18 4:03 AM, Sandipan Das wrote:
> The imm field of a bpf_insn is a signed 32-bit integer. For
> JIT-ed bpf-to-bpf function calls, it stores the offset from
> __bpf_call_base to the start of the callee function.
>
> For some architectures, such as powerpc64, it was found that
> this offset may be as large as 64 bits because of which this
> cannot be accomodated in the imm field without truncation.
>
> To resolve this, we additionally use the aux data within each
> bpf_prog associated with the caller functions to store the
> addresses of their respective callees.
>
> Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com>
> ---
> kernel/bpf/verifier.c | 39 ++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 5fb69a85d967..52088b4ca02f 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5282,6 +5282,19 @@ static int jit_subprogs(struct bpf_verifier_env *env)
> * run last pass of JIT
> */
> for (i = 0; i <= env->subprog_cnt; i++) {
> + u32 flen = func[i]->len, callee_cnt = 0;
> + struct bpf_prog **callee;
> +
> + /* for now assume that the maximum number of bpf function
> + * calls that can be made by a caller must be at most the
> + * number of bpf instructions in that function
> + */
> + callee = kzalloc(sizeof(func[i]) * flen, GFP_KERNEL);
> + if (!callee) {
> + err = -ENOMEM;
> + goto out_free;
> + }
> +
> insn = func[i]->insnsi;
> for (j = 0; j < func[i]->len; j++, insn++) {
> if (insn->code != (BPF_JMP | BPF_CALL) ||
> @@ -5292,6 +5305,26 @@ static int jit_subprogs(struct bpf_verifier_env *env)
> insn->imm = (u64 (*)(u64, u64, u64, u64, u64))
> func[subprog]->bpf_func -
> __bpf_call_base;
> +
> + /* the offset to the callee from __bpf_call_base
> + * may be larger than what the 32 bit integer imm
> + * can accomodate which will truncate the higher
> + * order bits
> + *
> + * to avoid this, we additionally utilize the aux
> + * data of each caller function for storing the
> + * addresses of every callee associated with it
> + */
> + callee[callee_cnt++] = func[subprog];
can you share typical /proc/kallsyms ?
Are you saying that kernel and kernel modules are allocated from
address spaces that are always more than 32-bit apart?
That would mean that all kernel calls into modules are far calls
and the other way around form .ko into kernel?
Performance is probably suffering because every call needs to be built
with full 64-bit offset. No ?
^ permalink raw reply
* [PATCH] KVM: PPC: Book3S HV: Branch inside feature section
From: Alexander Graf @ 2018-02-08 17:38 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Paul Mackerras
We ended up with code that did a conditional branch inside a feature
section to code outside of the feature section. Depending on how the
object file gets organized, that might mean we exceed the 14bit
relocation limit for conditional branches:
arch/powerpc/kvm/built-in.o:arch/powerpc/kvm/book3s_hv_rmhandlers.S:416:(__ftr_alt_97+0x8): relocation truncated to fit: R_PPC64_REL14 against `.text'+1ca4
So instead of doing a conditional branch outside of the feature section,
let's just jump at the end of the same, making the branch very short.
Signed-off-by: Alexander Graf <agraf@suse.de>
---
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 9c61f736c75b..1e037326897b 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -413,10 +413,11 @@ FTR_SECTION_ELSE
/* On P9 we use the split_info for coordinating LPCR changes */
lwz r4, KVM_SPLIT_DO_SET(r6)
cmpwi r4, 0
- beq 63f
+ beq 1f
mr r3, r6
bl kvmhv_p9_set_lpcr
nop
+1:
ALT_FTR_SECTION_END_IFCLR(CPU_FTR_ARCH_300)
63:
/* Order load of vcpu after load of vcore */
--
2.12.3
^ permalink raw reply related
* Re: [RFC][PATCH bpf 1/2] bpf: allow 64-bit offsets for bpf function calls
From: Naveen N. Rao @ 2018-02-08 17:59 UTC (permalink / raw)
To: Alexei Starovoitov, daniel, Sandipan Das; +Cc: linuxppc-dev, mpe, netdev
In-Reply-To: <4ce54c76-f8d5-e739-d9c2-e3318e398417@fb.com>
Alexei Starovoitov wrote:
> On 2/8/18 4:03 AM, Sandipan Das wrote:
>> The imm field of a bpf_insn is a signed 32-bit integer. For
>> JIT-ed bpf-to-bpf function calls, it stores the offset from
>> __bpf_call_base to the start of the callee function.
>>
>> For some architectures, such as powerpc64, it was found that
>> this offset may be as large as 64 bits because of which this
>> cannot be accomodated in the imm field without truncation.
>>
>> To resolve this, we additionally use the aux data within each
>> bpf_prog associated with the caller functions to store the
>> addresses of their respective callees.
>>
>> Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com>
>> ---
>> kernel/bpf/verifier.c | 39 ++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 38 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 5fb69a85d967..52088b4ca02f 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -5282,6 +5282,19 @@ static int jit_subprogs(struct bpf_verifier_env *=
env)
>> * run last pass of JIT
>> */
>> for (i =3D 0; i <=3D env->subprog_cnt; i++) {
>> + u32 flen =3D func[i]->len, callee_cnt =3D 0;
>> + struct bpf_prog **callee;
>> +
>> + /* for now assume that the maximum number of bpf function
>> + * calls that can be made by a caller must be at most the
>> + * number of bpf instructions in that function
>> + */
>> + callee =3D kzalloc(sizeof(func[i]) * flen, GFP_KERNEL);
>> + if (!callee) {
>> + err =3D -ENOMEM;
>> + goto out_free;
>> + }
>> +
>> insn =3D func[i]->insnsi;
>> for (j =3D 0; j < func[i]->len; j++, insn++) {
>> if (insn->code !=3D (BPF_JMP | BPF_CALL) ||
>> @@ -5292,6 +5305,26 @@ static int jit_subprogs(struct bpf_verifier_env *=
env)
>> insn->imm =3D (u64 (*)(u64, u64, u64, u64, u64))
>> func[subprog]->bpf_func -
>> __bpf_call_base;
>> +
>> + /* the offset to the callee from __bpf_call_base
>> + * may be larger than what the 32 bit integer imm
>> + * can accomodate which will truncate the higher
>> + * order bits
>> + *
>> + * to avoid this, we additionally utilize the aux
>> + * data of each caller function for storing the
>> + * addresses of every callee associated with it
>> + */
>> + callee[callee_cnt++] =3D func[subprog];
>=20
> can you share typical /proc/kallsyms ?
> Are you saying that kernel and kernel modules are allocated from
> address spaces that are always more than 32-bit apart?
Yes. On ppc64, kernel text is linearly mapped from 0xc000000000000000,=20
while vmalloc'ed area starts from 0xd000000000000000 (for radix, this is
different, but still beyond a 32-bit offset).
> That would mean that all kernel calls into modules are far calls
> and the other way around form .ko into kernel?
> Performance is probably suffering because every call needs to be built
> with full 64-bit offset. No ?
Possibly, and I think Michael can give a better perspective, but I think
this is due to our ABI. For inter-module calls, we need to setup the TOC
pointer (or the address of the function being called with ABIv2), which=20
would require us to load a full address regardless.
- Naveen
=
^ permalink raw reply
* Re: [PATCH kernel] powerpc/pci: Fix broken INTx configuration via OF
From: Bjorn Helgaas @ 2018-02-08 19:20 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: linuxppc-dev, Benjamin Herrenschmidt, Michael Ellerman,
Paul Mackerras, Rob Herring, Alistair Popple, linux-pci
In-Reply-To: <20180208053354.41725-1-aik@ozlabs.ru>
[+cc linux-pci]
The original commit was merged via PCI, and I think it's a good idea
to merge fixes to it the same way. I'll try to merge this in time for
v4.16-rc1.
On Wed, Feb 7, 2018 at 11:33 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> Commit 59f47eff03a0 ("powerpc/pci: Use of_irq_parse_and_map_pci() helper")
> correctly states that of_irq_parse_and_map_pci() does the same thing as
> of_irq_parse_pci() does as it simply calls
> of_irq_parse_pci() and irq_create_of_mapping().
>
> However of_irq_parse_and_map_pci() not only returns 0 for success and
> negative value for an error but also a positive virq value from
> irq_create_of_mapping() which the mentioned commit ignores and
> INTx config fails.
>
> This fixes the of_irq_parse_and_map_pci() return value handling.
>
> Fixes: 59f47eff03a0 "powerpc/pci: Use of_irq_parse_and_map_pci() helper"
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>
> Found it on POWER9 + powernv system - almost all devices suddenly lost
> INTx support.
> ---
> arch/powerpc/kernel/pci-common.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index ae2ede4..acbb44f2 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -370,7 +370,8 @@ static int pci_read_irq_line(struct pci_dev *pci_dev)
> memset(&oirq, 0xff, sizeof(oirq));
> #endif
> /* Try to get a mapping from the device-tree */
> - if (!of_irq_parse_and_map_pci(pci_dev, 0, 0)) {
> + virq = of_irq_parse_and_map_pci(pci_dev, 0, 0);
> + if (virq <= 0) {
> u8 line, pin;
>
> /* If that fails, lets fallback to what is in the config
> --
> 2.11.0
>
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/mm: Fix crashes with PUD level hugetlb config
From: Ram Pai @ 2018-02-08 19:22 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: benh, paulus, mpe, linuxppc-dev
In-Reply-To: <20180208103442.22045-1-aneesh.kumar@linux.vnet.ibm.com>
On Thu, Feb 08, 2018 at 04:04:41PM +0530, Aneesh Kumar K.V wrote:
> To support memory keys, we moved the hash pte slot information to the second
> half of the page table. This was ok with PTE entries at level 4 and level 3.
> We already allocate larger page table pages at those level to accomodate extra
> details. For level 4 we already have the extra space which was used to track
> 4k hash page table entry details and at pmd level the extra space was allocated
> to track the THP details.
>
> With hugetlbfs PTE, we used this extra space at the PMD level to store the
> slot details. But we also support hugetlbfs PTE at PUD leve and PUD level page
> didn't allocate extra space. This resulted in memory corruption.
>
> Fix this by allocating extra space at PUD level when HUGETLB is enabled. We
> may need further changes to allocate larger space at PMD level when we enable
> HUGETLB. That will be done in next patch.
>
> Fixes:bf9a95f9a6481bc6e(" powerpc: Free up four 64K PTE bits in 64K backed HPTE pages")
hmm.. did not know that hugetlbs operated at the PUD level. Thanks for
catching this.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
> NOTE: In the long run we may want to look at my patch series to remove slot
...snip...
> }
>
> static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long addr)
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index 51017726d495..3c14663d457d 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -232,11 +232,13 @@ extern unsigned long __pmd_index_size;
> extern unsigned long __pud_index_size;
> extern unsigned long __pgd_index_size;
> extern unsigned long __pmd_cache_index;
> +extern unsigned long __pud_cache_index;
> #define PTE_INDEX_SIZE __pte_index_size
> #define PMD_INDEX_SIZE __pmd_index_size
> #define PUD_INDEX_SIZE __pud_index_size
> #define PGD_INDEX_SIZE __pgd_index_size
> #define PMD_CACHE_INDEX __pmd_cache_index
> +#define PUD_CACHE_INDEX __pmd_cache_index
This is a typo. Should be 'pud' not a 'pmd'.
#define PUD_CACHE_INDEX __pud_cache_index
^
> /*
> * Because of use of pte fragments and THP, size of page table
..snip...
RP
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/mm: Fix crashes with PUD level hugetlb config
From: Ram Pai @ 2018-02-08 19:29 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: benh, paulus, mpe, linuxppc-dev
In-Reply-To: <87r2pvfr5g.fsf@linux.vnet.ibm.com>
On Thu, Feb 08, 2018 at 08:46:27PM +0530, Aneesh Kumar K.V wrote:
> "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:
>
> > To support memory keys, we moved the hash pte slot information to the second
> > half of the page table. This was ok with PTE entries at level 4 and level 3.
> > We already allocate larger page table pages at those level to accomodate extra
> > details. For level 4 we already have the extra space which was used to track
> > 4k hash page table entry details and at pmd level the extra space was allocated
> > to track the THP details.
> >
> > With hugetlbfs PTE, we used this extra space at the PMD level to store the
> > slot details. But we also support hugetlbfs PTE at PUD leve and PUD level page
> > didn't allocate extra space. This resulted in memory corruption.
> >
> > Fix this by allocating extra space at PUD level when HUGETLB is enabled. We
> > may need further changes to allocate larger space at PMD level when we enable
> > HUGETLB. That will be done in next patch.
> >
> > Fixes:bf9a95f9a6481bc6e(" powerpc: Free up four 64K PTE bits in 64K backed HPTE pages")
> >
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>
> Another fix, I still get random memory corruption with hugetlb test with
> 16G hugepage config.
this fix may not be needed. It random corruption may be artifact of the typo you
had in your first patch?
RP
^ permalink raw reply
* Re: [PATCH kernel] powerpc/pci: Fix broken INTx configuration via OF
From: Rob Herring @ 2018-02-08 19:46 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: linuxppc-dev, Benjamin Herrenschmidt, Bjorn Helgaas,
Michael Ellerman, Paul Mackerras, Alistair Popple
In-Reply-To: <20180208053354.41725-1-aik@ozlabs.ru>
On Wed, Feb 7, 2018 at 11:33 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> Commit 59f47eff03a0 ("powerpc/pci: Use of_irq_parse_and_map_pci() helper")
> correctly states that of_irq_parse_and_map_pci() does the same thing as
> of_irq_parse_pci() does as it simply calls
> of_irq_parse_pci() and irq_create_of_mapping().
>
> However of_irq_parse_and_map_pci() not only returns 0 for success and
> negative value for an error but also a positive virq value from
> irq_create_of_mapping() which the mentioned commit ignores and
> INTx config fails.
>
> This fixes the of_irq_parse_and_map_pci() return value handling.
>
> Fixes: 59f47eff03a0 "powerpc/pci: Use of_irq_parse_and_map_pci() helper"
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>
> Found it on POWER9 + powernv system - almost all devices suddenly lost
> INTx support.
> ---
> arch/powerpc/kernel/pci-common.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Thanks.
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v7 00/24] Speculative page faults
From: Andrew Morton @ 2018-02-08 20:53 UTC (permalink / raw)
To: Laurent Dufour
Cc: paulmck, peterz, kirill, ak, mhocko, dave, jack, Matthew Wilcox,
benh, mpe, paulus, Thomas Gleixner, Ingo Molnar, hpa, Will Deacon,
Sergey Senozhatsky, Andrea Arcangeli, Alexei Starovoitov,
kemi.wang, sergey.senozhatsky.work, Daniel Jordan, linux-kernel,
linux-mm, haren, khandual, npiggin, bsingharora, Tim Chen,
linuxppc-dev, x86
In-Reply-To: <1517935810-31177-1-git-send-email-ldufour@linux.vnet.ibm.com>
On Tue, 6 Feb 2018 17:49:46 +0100 Laurent Dufour <ldufour@linux.vnet.ibm.com> wrote:
> This is a port on kernel 4.15 of the work done by Peter Zijlstra to
> handle page fault without holding the mm semaphore [1].
>
> The idea is to try to handle user space page faults without holding the
> mmap_sem. This should allow better concurrency for massively threaded
> process since the page fault handler will not wait for other threads memory
> layout change to be done, assuming that this change is done in another part
> of the process's memory space. This type page fault is named speculative
> page fault. If the speculative page fault fails because of a concurrency is
> detected or because underlying PMD or PTE tables are not yet allocating, it
> is failing its processing and a classic page fault is then tried.
>
> The speculative page fault (SPF) has to look for the VMA matching the fault
> address without holding the mmap_sem, this is done by introducing a rwlock
> which protects the access to the mm_rb tree. Previously this was done using
> SRCU but it was introducing a lot of scheduling to process the VMA's
> freeing
> operation which was hitting the performance by 20% as reported by Kemi Wang
> [2].Using a rwlock to protect access to the mm_rb tree is limiting the
> locking contention to these operations which are expected to be in a O(log
> n)
> order. In addition to ensure that the VMA is not freed in our back a
> reference count is added and 2 services (get_vma() and put_vma()) are
> introduced to handle the reference count. When a VMA is fetch from the RB
> tree using get_vma() is must be later freeed using put_vma(). Furthermore,
> to allow the VMA to be used again by the classic page fault handler a
> service is introduced can_reuse_spf_vma(). This service is expected to be
> called with the mmap_sem hold. It checked that the VMA is still matching
> the specified address and is releasing its reference count as the mmap_sem
> is hold it is ensure that it will not be freed in our back. In general, the
> VMA's reference count could be decremented when holding the mmap_sem but it
> should not be increased as holding the mmap_sem is ensuring that the VMA is
> stable. I can't see anymore the overhead I got while will-it-scale
> benchmark anymore.
>
> The VMA's attributes checked during the speculative page fault processing
> have to be protected against parallel changes. This is done by using a per
> VMA sequence lock. This sequence lock allows the speculative page fault
> handler to fast check for parallel changes in progress and to abort the
> speculative page fault in that case.
>
> Once the VMA is found, the speculative page fault handler would check for
> the VMA's attributes to verify that the page fault has to be handled
> correctly or not. Thus the VMA is protected through a sequence lock which
> allows fast detection of concurrent VMA changes. If such a change is
> detected, the speculative page fault is aborted and a *classic* page fault
> is tried. VMA sequence lockings are added when VMA attributes which are
> checked during the page fault are modified.
>
> When the PTE is fetched, the VMA is checked to see if it has been changed,
> so once the page table is locked, the VMA is valid, so any other changes
> leading to touching this PTE will need to lock the page table, so no
> parallel change is possible at this time.
>
> The locking of the PTE is done with interrupts disabled, this allows to
> check for the PMD to ensure that there is not an ongoing collapsing
> operation. Since khugepaged is firstly set the PMD to pmd_none and then is
> waiting for the other CPU to have catch the IPI interrupt, if the pmd is
> valid at the time the PTE is locked, we have the guarantee that the
> collapsing opertion will have to wait on the PTE lock to move foward. This
> allows the SPF handler to map the PTE safely. If the PMD value is different
> than the one recorded at the beginning of the SPF operation, the classic
> page fault handler will be called to handle the operation while holding the
> mmap_sem. As the PTE lock is done with the interrupts disabled, the lock is
> done using spin_trylock() to avoid dead lock when handling a page fault
> while a TLB invalidate is requested by an other CPU holding the PTE.
>
> Support for THP is not done because when checking for the PMD, we can be
> confused by an in progress collapsing operation done by khugepaged. The
> issue is that pmd_none() could be true either if the PMD is not already
> populate or if the underlying PTE are in the way to be collapsed. So we
> cannot safely allocate a PMD if pmd_none() is true.
>
> This series builds on top of v4.15-mmotm-2018-01-31-16-51 and is
> functional on x86 and PowerPC.
One question which people will want to answer is "is this thing
working". ie, how frequently does the code fall back to the regular
heavyweight fault path.
I see that trace events have been added for this, but the overall
changelog doesn't describe them. I think this material is important
enough to justify including it here.
Also, a few words to help people figure out how to gather these stats
would be nice. And maybe helper scripts if appropriate?
I'm wondering if this info should even be presented via
/proc/self/something, dunno.
And it would be interesting to present the fallback frequency in the
benchmark results.
> ------------------
> Benchmarks results
>
> There is no functional change compared to the v6 so benchmark results are
> the same.
> Please see https://lkml.org/lkml/2018/1/12/515 for details.
Please include this vitally important info in the [0/n], don't make
people chase links.
And I'd really like to see some quantitative testing results for real
workloads, not just a bunch of microbenchmarks. Help us understand how
useful this patchset is to our users.
^ permalink raw reply
* Re: [PATCH kernel] powerpc/pci: Fix broken INTx configuration via OF
From: Bjorn Helgaas @ 2018-02-08 21:39 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Alexey Kardashevskiy, linuxppc-dev, Benjamin Herrenschmidt,
Michael Ellerman, Paul Mackerras, Rob Herring, Alistair Popple,
linux-pci
In-Reply-To: <CAErSpo7S7vJTKBrxuzH-G0y-U2Z15jKpPJDZEEuhh7vAs0hjOA@mail.gmail.com>
On Thu, Feb 08, 2018 at 01:20:04PM -0600, Bjorn Helgaas wrote:
> [+cc linux-pci]
>
> The original commit was merged via PCI, and I think it's a good idea
> to merge fixes to it the same way. I'll try to merge this in time for
> v4.16-rc1.
>
> On Wed, Feb 7, 2018 at 11:33 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> > Commit 59f47eff03a0 ("powerpc/pci: Use of_irq_parse_and_map_pci() helper")
> > correctly states that of_irq_parse_and_map_pci() does the same thing as
> > of_irq_parse_pci() does as it simply calls
> > of_irq_parse_pci() and irq_create_of_mapping().
> >
> > However of_irq_parse_and_map_pci() not only returns 0 for success and
> > negative value for an error but also a positive virq value from
> > irq_create_of_mapping() which the mentioned commit ignores and
> > INTx config fails.
> >
> > This fixes the of_irq_parse_and_map_pci() return value handling.
> >
> > Fixes: 59f47eff03a0 "powerpc/pci: Use of_irq_parse_and_map_pci() helper"
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > ---
> >
> > Found it on POWER9 + powernv system - almost all devices suddenly lost
> > INTx support.
> > ---
> > arch/powerpc/kernel/pci-common.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> > index ae2ede4..acbb44f2 100644
> > --- a/arch/powerpc/kernel/pci-common.c
> > +++ b/arch/powerpc/kernel/pci-common.c
> > @@ -370,7 +370,8 @@ static int pci_read_irq_line(struct pci_dev *pci_dev)
> > memset(&oirq, 0xff, sizeof(oirq));
> > #endif
> > /* Try to get a mapping from the device-tree */
> > - if (!of_irq_parse_and_map_pci(pci_dev, 0, 0)) {
> > + virq = of_irq_parse_and_map_pci(pci_dev, 0, 0);
> > + if (virq <= 0) {
I don't understand how this fix works. We used to check the result of
of_irq_parse_and_map_pci() and entered the block if it was zero.
Now you enter the block if it is zero or less than zero, but:
static int pci_read_irq_line(...)
{
unsigned int virq = 0; /* unnecessarily initialized, BTW */
virq = of_irq_parse_and_map_pci(pci_dev, 0, 0);
if (virq <= 0) {
...
virq is unsigned, so "virq < 0" can never be true. So how does this
change anything?
Bjorn
^ permalink raw reply
* Re: [PATCH kernel] powerpc/pci: Fix broken INTx configuration via OF
From: Benjamin Herrenschmidt @ 2018-02-08 22:21 UTC (permalink / raw)
To: Bjorn Helgaas, Bjorn Helgaas
Cc: Alexey Kardashevskiy, linuxppc-dev, Michael Ellerman,
Paul Mackerras, Rob Herring, Alistair Popple, linux-pci
In-Reply-To: <20180208213956.GC98765@bhelgaas-glaptop.roam.corp.google.com>
On Thu, 2018-02-08 at 15:39 -0600, Bjorn Helgaas wrote:
> I don't understand how this fix works. We used to check the result of
> of_irq_parse_and_map_pci() and entered the block if it was zero.
>
> Now you enter the block if it is zero or less than zero, but:
>
> static int pci_read_irq_line(...)
> {
> unsigned int virq = 0; /* unnecessarily initialized, BTW */
>
> virq = of_irq_parse_and_map_pci(pci_dev, 0, 0);
> if (virq <= 0) {
> ...
>
> virq is unsigned, so "virq < 0" can never be true. So how does this
> change anything?
Yes it does:
So the unsigned thing is a second bug in the original patch that Alexey
isn't fixing, we need to fix it too.
However, the actual bug Alexey is fixing is that we lost the actual
value of virq. IE, without his fix, we test it for 0 but we don't
actually return it if it's positive.
So he fixes the normal case but there's still a bug in the error case,
we need to make virq signed.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH kernel] powerpc/pci: Fix broken INTx configuration via OF
From: Bjorn Helgaas @ 2018-02-08 22:42 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Bjorn Helgaas, Alexey Kardashevskiy, linuxppc-dev,
Michael Ellerman, Paul Mackerras, Rob Herring, Alistair Popple,
linux-pci
In-Reply-To: <1518128503.2312.219.camel@kernel.crashing.org>
On Fri, Feb 09, 2018 at 09:21:43AM +1100, Benjamin Herrenschmidt wrote:
> On Thu, 2018-02-08 at 15:39 -0600, Bjorn Helgaas wrote:
> > I don't understand how this fix works. We used to check the result of
> > of_irq_parse_and_map_pci() and entered the block if it was zero.
> >
> > Now you enter the block if it is zero or less than zero, but:
> >
> > static int pci_read_irq_line(...)
> > {
> > unsigned int virq = 0; /* unnecessarily initialized, BTW */
> >
> > virq = of_irq_parse_and_map_pci(pci_dev, 0, 0);
> > if (virq <= 0) {
> > ...
> >
> > virq is unsigned, so "virq < 0" can never be true. So how does this
> > change anything?
>
> Yes it does:
>
> So the unsigned thing is a second bug in the original patch that Alexey
> isn't fixing, we need to fix it too.
>
> However, the actual bug Alexey is fixing is that we lost the actual
> value of virq. IE, without his fix, we test it for 0 but we don't
> actually return it if it's positive.
Ah, I see, the bug is that we discarded the non-zero virq value when
we actually need it. I'm going to wait for a new patch with a
changelog that says that and doesn't test an unsigned value for < 0.
> So he fixes the normal case but there's still a bug in the error case,
> we need to make virq signed.
I looked through the of_irq_parse_and_map_pci() path and I do not see
a case where it can return a negative value. It either returns zero
or one of these:
virq = irq_find_mapping(...)
virq = irq_create_mapping(...)
Both of these functions return unsigned values.
^ permalink raw reply
* Re: [PATCH kernel] powerpc/pci: Fix broken INTx configuration via OF
From: Benjamin Herrenschmidt @ 2018-02-08 22:50 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Bjorn Helgaas, Alexey Kardashevskiy, linuxppc-dev,
Michael Ellerman, Paul Mackerras, Rob Herring, Alistair Popple,
linux-pci
In-Reply-To: <20180208224205.GA206223@bhelgaas-glaptop.roam.corp.google.com>
On Thu, 2018-02-08 at 16:42 -0600, Bjorn Helgaas wrote:
> On Fri, Feb 09, 2018 at 09:21:43AM +1100, Benjamin Herrenschmidt wrote:
> > On Thu, 2018-02-08 at 15:39 -0600, Bjorn Helgaas wrote:
> > > I don't understand how this fix works. We used to check the result of
> > > of_irq_parse_and_map_pci() and entered the block if it was zero.
> > >
> > > Now you enter the block if it is zero or less than zero, but:
> > >
> > > static int pci_read_irq_line(...)
> > > {
> > > unsigned int virq = 0; /* unnecessarily initialized, BTW */
> > >
> > > virq = of_irq_parse_and_map_pci(pci_dev, 0, 0);
> > > if (virq <= 0) {
> > > ...
> > >
> > > virq is unsigned, so "virq < 0" can never be true. So how does this
> > > change anything?
> >
> > Yes it does:
> >
> > So the unsigned thing is a second bug in the original patch that Alexey
> > isn't fixing, we need to fix it too.
> >
> > However, the actual bug Alexey is fixing is that we lost the actual
> > value of virq. IE, without his fix, we test it for 0 but we don't
> > actually return it if it's positive.
>
> Ah, I see, the bug is that we discarded the non-zero virq value when
> we actually need it. I'm going to wait for a new patch with a
> changelog that says that and doesn't test an unsigned value for < 0.
>
> > So he fixes the normal case but there's still a bug in the error case,
> > we need to make virq signed.
>
> I looked through the of_irq_parse_and_map_pci() path and I do not see
> a case where it can return a negative value. It either returns zero
> or one of these:
>
> virq = irq_find_mapping(...)
> virq = irq_create_mapping(...)
>
> Both of these functions return unsigned values.
Ok so the test is just wrong then. Aleey, can you respin ?
Cheers,
Ben.
^ permalink raw reply
* [PATCH kernel v2] powerpc/pci: Fix broken INTx configuration via OF
From: Alexey Kardashevskiy @ 2018-02-09 2:41 UTC (permalink / raw)
To: linuxppc-dev
Cc: Alexey Kardashevskiy, Benjamin Herrenschmidt, Bjorn Helgaas,
Michael Ellerman, Rob Herring, linux-pci
Commit 59f47eff03a0 ("powerpc/pci: Use of_irq_parse_and_map_pci() helper")
replaced of_irq_parse_pci() + irq_create_of_mapping() with
of_irq_parse_and_map_pci() but this change lost virq returned by
irq_create_of_mapping() so virq remained zero causing INTx
misconfiguration.
This fixes pci_read_irq_line() not to loose a virq returned by
of_irq_parse_and_map_pci().
Fixes: 59f47eff03a0 "powerpc/pci: Use of_irq_parse_and_map_pci() helper"
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v2:
* changed the condition from <=0 to !=0 as by design
of_irq_parse_and_map_pci() can only return 0 for an error and virq>0.
---
arch/powerpc/kernel/pci-common.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index ae2ede4..33580a9 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -362,7 +362,7 @@ struct pci_controller* pci_find_hose_for_OF_device(struct device_node* node)
*/
static int pci_read_irq_line(struct pci_dev *pci_dev)
{
- unsigned int virq = 0;
+ unsigned int virq;
pr_debug("PCI: Try to map irq for %s...\n", pci_name(pci_dev));
@@ -370,7 +370,8 @@ static int pci_read_irq_line(struct pci_dev *pci_dev)
memset(&oirq, 0xff, sizeof(oirq));
#endif
/* Try to get a mapping from the device-tree */
- if (!of_irq_parse_and_map_pci(pci_dev, 0, 0)) {
+ virq = of_irq_parse_and_map_pci(pci_dev, 0, 0);
+ if (!virq) {
u8 line, pin;
/* If that fails, lets fallback to what is in the config
--
2.11.0
^ permalink raw reply related
* Re: [RESEND, V2] powerpc/numa: Invalidate numa_cpu_lookup_table on cpu remove
From: Michael Ellerman @ 2018-02-09 4:00 UTC (permalink / raw)
To: Nathan Fontenot, linuxppc-dev
In-Reply-To: <151699561194.20320.12952014708209986173.stgit@ltcalpine2-lp14.aus.stglabs.ibm.com>
On Fri, 2018-01-26 at 19:41:59 UTC, Nathan Fontenot wrote:
> When DLPAR removing a CPU, the unmapping of the cpu from a node in
> unmap_cpu_from_node() should also invalidate the CPUs entry in the
> numa_cpu_lookup_table. There is not a guarantee that on a subsequent
> DLPAR add of the CPU the associativity will be the same and thus
> could be in a different node. Invalidating the entry in the
> numa_cpu_lookup_table causes the associativity to be read from the
> device tree at the time of the add.
>
> The current behavior of not invalidating the CPUs entry in the
> numa_cpu_lookup_table can result in scenarios where the the topology
> layout of CPUs in the partition does not match the device tree
> or the topology reported by the HMC.
>
> Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> Reviewed-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/1d9a090783bef19fe8cdec878620d2
cheers
^ permalink raw reply
* Re: ocxl: fix signed comparison with less than zero
From: Michael Ellerman @ 2018-02-09 4:00 UTC (permalink / raw)
To: Colin King, Frederic Barrat, Andrew Donnellan, Arnd Bergmann,
Greg Kroah-Hartman, linuxppc-dev
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20180130151144.32544-1-colin.king@canonical.com>
On Tue, 2018-01-30 at 15:11:44 UTC, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently the comparison of used < 0 is always false because
> uses is a size_t. Fix this by making used a ssize_t type.
>
> Detected by Coccinelle:
> drivers/misc/ocxl/file.c:320:6-10: WARNING: Unsigned expression
> compared with zero: used < 0
>
> Fixes: 5ef3166e8a32 ("ocxl: Driver code for 'generic' opencapi devices")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/dedab7f0d3137441a97fe7cf9b9ca5
cheers
^ permalink raw reply
* Re: powerpc/64s: fix may_hard_irq_enable for PMI soft masking
From: Michael Ellerman @ 2018-02-09 4:00 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Madhavan Srinivasan, Nicholas Piggin
In-Reply-To: <20180203071750.6469-1-npiggin@gmail.com>
On Sat, 2018-02-03 at 07:17:50 UTC, Nicholas Piggin wrote:
> The soft IRQ masking code has to hard-disable interrupts in cases
> where the exception is not cleared by the masked handler. External
> interrupts used this approach for soft masking. Now recently PMU
> interrupts do the same thing.
>
> The soft IRQ masking code additionally allowed for interrupt handlers
> to hard-enable interrupts after soft-disabling them. The idea is to
> allow PMU interrupts through to profile interrupt handlers.
>
> So when interrupts are being replayed when there is a pending
> interrupt that requires hard-disabling, there is a test to prevent
> those handlers from hard-enabling them if there is a pending external
> interrupt. may_hard_irq_enable() handles this.
>
> After f442d00480 ("powerpc/64s: Add support to mask perf interrupts
> and replay them"), may_hard_irq_enable() could prematurely enable
> MSR[EE] when a PMU exception exists, which would result in the
> interrupt firing again while masked, and MSR[EE] being disabled again.
>
> I haven't seen that this could cause a serious problem, but it's
> more consistent to handle these soft-masked interrupts in the same
> way. So introduce a define for all types of interrupts that require
> MSR[EE] masking in their soft-disable handlers, and use that in
> may_hard_irq_enable().
>
> Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> Reviewed-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/6cc3f91bf69fc8c1719704607474f9
cheers
^ permalink raw reply
* Re: powerpc/64s: Fix MASKABLE_RELON_EXCEPTION_HV_OOL macro
From: Michael Ellerman @ 2018-02-09 4:00 UTC (permalink / raw)
To: Madhavan Srinivasan; +Cc: Madhavan Srinivasan, linuxppc-dev
In-Reply-To: <1517920597-14588-1-git-send-email-maddy@linux.vnet.ibm.com>
On Tue, 2018-02-06 at 12:36:37 UTC, Madhavan Srinivasan wrote:
> Commit f14e953b191f ("powerpc/64s: Add support to take additional parameter in MASKABLE_* macro")
> messed up MASKABLE_RELON_EXCEPTION_HV_OOL macro by adding the wrong
> __SOFTEN__ test which caused guest kernel trash at boot. Patch to fix
> the macro to use SOFTEN_TEST_HV instead of SOFTEN_NOTEST_HV.
>
> Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Fix-Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/5c11d1e52d996749897a8616860b18
cheers
^ permalink raw reply
* Re: powerpc/64s/radix: kernel boot-time NULL pointer protection using a guard-PID
From: Michael Ellerman @ 2018-02-09 4:00 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: Benjamin Herrenschmidt, Aneesh Kumar K . V, Nicholas Piggin
In-Reply-To: <20180207012002.4452-1-npiggin@gmail.com>
On Wed, 2018-02-07 at 01:20:02 UTC, Nicholas Piggin wrote:
> This change restores and formalises the behaviour that access to NULL or
> other user addresses by the kernel during boot should fault rather than
> succeed and modify memory. This was inadvertently broken when fixing
> another bug, because it was previously not well defined and only worked
> by chance.
>
> powerpc/64s/radix uses high address bits to select an address space
> "quadrant", which determines which PID and LPID are used to translate
> the rest of the address (effective PID, effective LPID). The kernel
> mapping at 0xC... selects quadrant 3, which uses PID=0 and LPID=0. So
> the kernel page tables are installed in the PID 0 process table entry.
>
> An address at 0x0... selects quadrant 0, which uses PID=PIDR for
> translating the rest of the address (that is, it uses the value of the
> PIDR register as the effective PID). If PIDR=0, then the translation is
> performed with the PID 0 process table entry page tables. This is the
> kernel mapping, so we effectively get another copy of the kernel address
> space at 0. A NULL pointer access will access physical memory address 0.
>
> To prevent duplicating the kernel address space in quadrant 0, this
> patch allocates a guard PID containing no translations, and initializes
> PIDR with this during boot, before the MMU is switched on. Any kernel
> access to quadrant 0 will use this guard PID for translation and find no
> valid mappings, and therefore fault.
>
> After boot, this PID will be switchd away to user context PIDs, but
> those contain user mappings (and usually NULL pointer protection) rather
> than kernel mapping, which is much safer (and by design). It may be in
> future this is tightened further, which the guard PID could be used for.
>
> Commit 371b8044 ("powerpc/64s: Initialize ISAv3 MMU registers before
> setting partition table"), introduced this problem because it zeroes
> PIDR at boot. However previously the value was inherited from firmware
> or kexec, which is not robust and can be zero (e.g., mambo).
>
> Fixes: 371b8044 ("powerpc/64s: Initialize ISAv3 MMU registers before setting partition table")
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Reported-by: Florian Weimer <fweimer@redhat.com>
> Tested-by: Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/eeb715c3e995fbdda0cc05e61216c6
Added:
Cc: stable@vger.kernel.org # v4.15+
cheers
^ permalink raw reply
* Re: powerpc/mm/pgtable: Split mappings on hot-unplug
From: Michael Ellerman @ 2018-02-09 4:00 UTC (permalink / raw)
To: Balbir Singh, linuxppc-dev
In-Reply-To: <20180207063551.8991-1-bsingharora@gmail.com>
On Wed, 2018-02-07 at 06:35:51 UTC, Balbir Singh wrote:
> This patch splits the a linear mapping if the hot-unplug range
> is smaller than the mapping size. The code detects if the mapping
> needs to be split into a smaller size and if so, uses the stop
> machine infrastructure to map the current linear mapping with
> a smaller size mapping. Then the requested area is unmapped.
>
> The code will skip any region of the mapping that overlaps with
> kernel text and warn about it once. We don't want to remove
> a mapping where the kernel text and the LMB we intend to remove
> overlap in the same TLB mapping as it may affect the currently
> executing code (being cautious, we might be able to remove this
> restriction later, see TODOs below)
>
> I've tested these changes under a kvm guest with 2 vcpus, from
> a split mapping point of view, some of the caveats mentioned
> above applied to the testing I did.
>
> TODOs:
> 1. Refine the stop machine with an atomic replacement of the
> PMD. The code needs to be refactored to support those changes.
> I intend to publish those changes, but this is a minimal set
> that should be also probably backported to stable
>
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/4dd5f8a99e791a8c6500e3592f3ce8
cheers
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox