* Re: [PATCH v3 4/7] powerpc/fadump: exclude memory holes while reserving memory in second kernel.
From: Hari Bathini @ 2018-04-03 9:51 UTC (permalink / raw)
To: Mahesh J Salgaonkar, linuxppc-dev
Cc: Srikar Dronamraju, kernelfans, Aneesh Kumar K.V, Ananth Narayan,
Nathan Fontenot, Anshuman Khandual
In-Reply-To: <152265062142.23251.2863873644423472277.stgit@jupiter.in.ibm.com>
On Monday 02 April 2018 12:00 PM, Mahesh J Salgaonkar wrote:
> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>
> The second kernel, during early boot after the crash, reserves rest of
> the memory above boot memory size to make sure it does not touch any of the
> dump memory area. It uses memblock_reserve() that reserves the specified
> memory region irrespective of memory holes present within that region.
> There are chances where previous kernel would have hot removed some of
> its memory leaving memory holes behind. In such cases fadump kernel reports
> incorrect number of reserved pages through arch_reserved_kernel_pages()
> hook causing kernel to hang or panic.
>
> Fix this by excluding memory holes while reserving rest of the memory
> above boot memory size during second kernel boot after crash.
>
> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> ---
> arch/powerpc/kernel/fadump.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 011f1aa7abab..a497e9fb93fe 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -433,6 +433,21 @@ static inline unsigned long get_fadump_metadata_base(
> return be64_to_cpu(fdm->metadata_region.source_address);
> }
>
> +static void fadump_memblock_reserve(unsigned long base, unsigned long size)
> +{
> + struct memblock_region *reg;
> + unsigned long start, end;
> +
> + for_each_memblock(memory, reg) {
> + start = max(base, (unsigned long)reg->base);
> + end = reg->base + reg->size;
> + end = min(base + size, end);
> +
> + if (start < end)
> + memblock_reserve(start, end - start);
> + }
> +}
> +
> int __init fadump_reserve_mem(void)
> {
> unsigned long base, size, memory_boundary;
> @@ -487,7 +502,7 @@ int __init fadump_reserve_mem(void)
> */
> base = fw_dump.boot_memory_size;
> size = memory_boundary - base;
> - memblock_reserve(base, size);
> + fadump_memblock_reserve(base, size);
> printk(KERN_INFO "Reserved %ldMB of memory at %ldMB "
Mahesh, you may want to change this print as well as it would be
misleading in case of
holes in the memory.
Thanks
Hari
> "for saving crash dump\n",
> (unsigned long)(size >> 20),
>
^ permalink raw reply
* Re: [PATCH] cxl: Fix possible deadlock when processing page faults from cxllib
From: Laurent Dufour @ 2018-04-03 9:56 UTC (permalink / raw)
To: Frederic Barrat, linuxppc-dev, mpe; +Cc: clombard, andrew.donnellan, vaibhav
In-Reply-To: <20180403094311.331-1-fbarrat@linux.vnet.ibm.com>
On 03/04/2018 11:43, Frederic Barrat wrote:
> cxllib_handle_fault() is called by an external driver when it needs to
> have the host process page faults for a buffer which may cover several
> pages. Currently the function holds the mm->mmap_sem semaphore with
> read access while iterating over the buffer, since it could spawn
> several VMAs. When calling a lower-level function to handle the page
> fault for a single page, the semaphore is accessed again in read
> mode. That is wrong and can lead to deadlocks if a writer tries to
> sneak in while a buffer of several pages is being processed.
>
> The fix is to release the semaphore once cxllib_handle_fault() got the
> information it needs from the current vma. The address space/VMAs
> could evolve while we iterate over the full buffer, but in the
> unlikely case where we miss a page, the driver will raise a new page
> fault when retrying.
>
> Fixes: 3ced8d730063 ("cxl: Export library to support IBM XSL")
> Cc: stable@vger.kernel.org # 4.13+
> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
FWIW,
Reviewed-by : Laurent Dufour <ldufour@linux.vnet.ibm.com>
> ---
> drivers/misc/cxl/cxllib.c | 85 ++++++++++++++++++++++++++++++-----------------
> 1 file changed, 55 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/misc/cxl/cxllib.c b/drivers/misc/cxl/cxllib.c
> index 30ccba436b3b..55cd35d1a9cc 100644
> --- a/drivers/misc/cxl/cxllib.c
> +++ b/drivers/misc/cxl/cxllib.c
> @@ -208,49 +208,74 @@ int cxllib_get_PE_attributes(struct task_struct *task,
> }
> EXPORT_SYMBOL_GPL(cxllib_get_PE_attributes);
>
> -int cxllib_handle_fault(struct mm_struct *mm, u64 addr, u64 size, u64 flags)
> +static int get_vma_info(struct mm_struct *mm, u64 addr,
> + u64 *vma_start, u64 *vma_end,
> + unsigned long *page_size)
> {
> - int rc;
> - u64 dar;
> struct vm_area_struct *vma = NULL;
> - unsigned long page_size;
> -
> - if (mm == NULL)
> - return -EFAULT;
> + int rc = 0;
>
> down_read(&mm->mmap_sem);
>
> vma = find_vma(mm, addr);
> if (!vma) {
> - pr_err("Can't find vma for addr %016llx\n", addr);
> rc = -EFAULT;
> goto out;
> }
> - /* get the size of the pages allocated */
> - page_size = vma_kernel_pagesize(vma);
> -
> - for (dar = (addr & ~(page_size - 1)); dar < (addr + size); dar += page_size) {
> - if (dar < vma->vm_start || dar >= vma->vm_end) {
> - vma = find_vma(mm, addr);
> - if (!vma) {
> - pr_err("Can't find vma for addr %016llx\n", addr);
> - rc = -EFAULT;
> - goto out;
> - }
> - /* get the size of the pages allocated */
> - page_size = vma_kernel_pagesize(vma);
> + *page_size = vma_kernel_pagesize(vma);
> + *vma_start = vma->vm_start;
> + *vma_end = vma->vm_end;
> +out:
> + up_read(&mm->mmap_sem);
> + return rc;
> +}
> +
> +int cxllib_handle_fault(struct mm_struct *mm, u64 addr, u64 size, u64 flags)
> +{
> + int rc;
> + u64 dar, vma_start, vma_end;
> + unsigned long page_size;
> +
> + if (mm == NULL)
> + return -EFAULT;
> +
> + /*
> + * The buffer we have to process can extend over several pages
> + * and may also cover several VMAs.
> + * We iterate over all the pages. The page size could vary
> + * between VMAs.
> + */
> + rc = get_vma_info(mm, addr, &vma_start, &vma_end, &page_size);
> + if (rc)
> + return rc;
> +
> + for (dar = (addr & ~(page_size - 1)); dar < (addr + size);
> + dar += page_size) {
> + if (dar < vma_start || dar >= vma_end) {
> + /*
> + * We don't hold the mm->mmap_sem semaphore
> + * while iterating, since the semaphore is
> + * required by one of the lower-level page
> + * fault processing functions and it could
> + * create a deadlock.
> + *
> + * It means the VMAs can be altered between 2
> + * loop iterations and we could theoretically
> + * miss a page (however unlikely). But that's
> + * not really a problem, as the driver will
> + * retry access, get another page fault on the
> + * missing page and call us again.
> + */
> + rc = get_vma_info(mm, dar, &vma_start, &vma_end,
> + &page_size);
> + if (rc)
> + return rc;
> }
>
> rc = cxl_handle_mm_fault(mm, flags, dar);
> - if (rc) {
> - pr_err("cxl_handle_mm_fault failed %d", rc);
> - rc = -EFAULT;
> - goto out;
> - }
> + if (rc)
> + return -EFAULT;
> }
> - rc = 0;
> -out:
> - up_read(&mm->mmap_sem);
> - return rc;
> + return 0;
> }
> EXPORT_SYMBOL_GPL(cxllib_handle_fault);
>
^ permalink raw reply
* Re: [PATCH v3 6/7] powerpc/fadump: Do not allow hot-remove memory from fadump reserved area.
From: Mahesh Jagannath Salgaonkar @ 2018-04-03 11:37 UTC (permalink / raw)
To: Pingfan Liu
Cc: linuxppc-dev, Srikar Dronamraju, Aneesh Kumar K.V, Ananth Narayan,
Hari Bathini, Nathan Fontenot, Anshuman Khandual
In-Reply-To: <CAFgQCTu0qLrHuuiaCpPPEoFesz+19V+0CCQY-rjPz2chcNL1gw@mail.gmail.com>
On 04/03/2018 08:48 AM, Pingfan Liu wrote:
> I think CMA has protected us from hot-remove, so this patch is not necessary.
Yes, but only if the memory from declared CMA region is allocated using
cma_alloc(). The rest of the memory inside CMA region which hasn't been
cma_allocat-ed can still be hot-removed. Hence this patch is necessary.
fadump allocates only 1 page from fadump CMA region for metadata region.
Rest all memory is free to use by applications and vulnerable to hot-remove.
Thanks,
-Mahesh.
>
> Regards,
> Pingfan
>
> On Mon, Apr 2, 2018 at 2:30 PM, Mahesh J Salgaonkar
> <mahesh@linux.vnet.ibm.com> wrote:
>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>
>> For fadump to work successfully there should not be any holes in reserved
>> memory ranges where kernel has asked firmware to move the content of old
>> kernel memory in event of crash. But this memory area is currently not
>> protected from hot-remove operations. Hence, fadump service can fail to
>> re-register after the hot-remove operation, if hot-removed memory belongs
>> to fadump reserved region. To avoid this make sure that memory from fadump
>> reserved area is not hot-removable if fadump is registered.
>>
>> However, if user still wants to remove that memory, he can do so by
>> manually stopping fadump service before hot-remove operation.
>>
>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/include/asm/fadump.h | 2 +-
>> arch/powerpc/kernel/fadump.c | 10 ++++++++--
>> arch/powerpc/platforms/pseries/hotplug-memory.c | 7 +++++--
>> 3 files changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h
>> index 44b6ebfe9be6..d16dc77107a8 100644
>> --- a/arch/powerpc/include/asm/fadump.h
>> +++ b/arch/powerpc/include/asm/fadump.h
>> @@ -207,7 +207,7 @@ struct fad_crash_memory_ranges {
>> unsigned long long size;
>> };
>>
>> -extern int is_fadump_boot_memory_area(u64 addr, ulong size);
>> +extern int is_fadump_memory_area(u64 addr, ulong size);
>> extern int early_init_dt_scan_fw_dump(unsigned long node,
>> const char *uname, int depth, void *data);
>> extern int fadump_reserve_mem(void);
>> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
>> index 59aaf0357a52..2c3c7e655eec 100644
>> --- a/arch/powerpc/kernel/fadump.c
>> +++ b/arch/powerpc/kernel/fadump.c
>> @@ -162,13 +162,19 @@ int __init early_init_dt_scan_fw_dump(unsigned long node,
>>
>> /*
>> * If fadump is registered, check if the memory provided
>> - * falls within boot memory area.
>> + * falls within boot memory area and reserved memory area.
>> */
>> -int is_fadump_boot_memory_area(u64 addr, ulong size)
>> +int is_fadump_memory_area(u64 addr, ulong size)
>> {
>> + u64 d_start = fw_dump.reserve_dump_area_start;
>> + u64 d_end = d_start + fw_dump.reserve_dump_area_size;
>> +
>> if (!fw_dump.dump_registered)
>> return 0;
>>
>> + if (((addr + size) > d_start) && (addr <= d_end))
>> + return 1;
>> +
>> return (addr + size) > RMA_START && addr <= fw_dump.boot_memory_size;
>> }
>>
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> index c1578f54c626..e4c658cda3a7 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> @@ -389,8 +389,11 @@ static bool lmb_is_removable(struct drmem_lmb *lmb)
>> phys_addr = lmb->base_addr;
>>
>> #ifdef CONFIG_FA_DUMP
>> - /* Don't hot-remove memory that falls in fadump boot memory area */
>> - if (is_fadump_boot_memory_area(phys_addr, block_sz))
>> + /*
>> + * Don't hot-remove memory that falls in fadump boot memory area
>> + * and memory that is reserved for capturing old kernel memory.
>> + */
>> + if (is_fadump_memory_area(phys_addr, block_sz))
>> return false;
>> #endif
>>
>>
>
^ permalink raw reply
* Re: [PATCH v3 4/7] powerpc/fadump: exclude memory holes while reserving memory in second kernel.
From: Mahesh Jagannath Salgaonkar @ 2018-04-03 11:39 UTC (permalink / raw)
To: Hari Bathini, linuxppc-dev
Cc: Srikar Dronamraju, kernelfans, Aneesh Kumar K.V, Ananth Narayan,
Nathan Fontenot, Anshuman Khandual
In-Reply-To: <04e18a87-2882-b656-8fed-800c6d0bc835@linux.vnet.ibm.com>
On 04/03/2018 03:21 PM, Hari Bathini wrote:
>
>
> On Monday 02 April 2018 12:00 PM, Mahesh J Salgaonkar wrote:
>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>
>> The second kernel, during early boot after the crash, reserves rest of
>> the memory above boot memory size to make sure it does not touch any
>> of the
>> dump memory area. It uses memblock_reserve() that reserves the specified
>> memory region irrespective of memory holes present within that region.
>> There are chances where previous kernel would have hot removed some of
>> its memory leaving memory holes behind. In such cases fadump kernel
>> reports
>> incorrect number of reserved pages through arch_reserved_kernel_pages()
>> hook causing kernel to hang or panic.
>>
>> Fix this by excluding memory holes while reserving rest of the memory
>> above boot memory size during second kernel boot after crash.
>>
>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/kernel/fadump.c | 17 ++++++++++++++++-
>> 1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
>> index 011f1aa7abab..a497e9fb93fe 100644
>> --- a/arch/powerpc/kernel/fadump.c
>> +++ b/arch/powerpc/kernel/fadump.c
>> @@ -433,6 +433,21 @@ static inline unsigned long
>> get_fadump_metadata_base(
>> return be64_to_cpu(fdm->metadata_region.source_address);
>> }
>>
>> +static void fadump_memblock_reserve(unsigned long base, unsigned long
>> size)
>> +{
>> + struct memblock_region *reg;
>> + unsigned long start, end;
>> +
>> + for_each_memblock(memory, reg) {
>> + start = max(base, (unsigned long)reg->base);
>> + end = reg->base + reg->size;
>> + end = min(base + size, end);
>> +
>> + if (start < end)
>> + memblock_reserve(start, end - start);
>> + }
>> +}
>> +
>> int __init fadump_reserve_mem(void)
>> {
>> unsigned long base, size, memory_boundary;
>> @@ -487,7 +502,7 @@ int __init fadump_reserve_mem(void)
>> */
>> base = fw_dump.boot_memory_size;
>> size = memory_boundary - base;
>> - memblock_reserve(base, size);
>> + fadump_memblock_reserve(base, size);
>> printk(KERN_INFO "Reserved %ldMB of memory at %ldMB "
>
> Mahesh, you may want to change this print as well as it would be
> misleading in case of
> holes in the memory.
Yeah, may be we can just move that printk also inside
fadump_memblock_reserve().
Thanks,
-Mahesh.
>
> Thanks
> Hari
>
>> "for saving crash dump\n",
>> (unsigned long)(size >> 20),
>>
>
^ permalink raw reply
* Re: [PATCH] cxl: Fix possible deadlock when processing page faults from cxllib
From: Vaibhav Jain @ 2018-04-03 11:43 UTC (permalink / raw)
To: Frederic Barrat, linuxppc-dev, mpe; +Cc: ldufour, clombard, andrew.donnellan
In-Reply-To: <20180403094311.331-1-fbarrat@linux.vnet.ibm.com>
Frederic Barrat <fbarrat@linux.vnet.ibm.com> writes:
> cxllib_handle_fault() is called by an external driver when it needs to
> have the host process page faults for a buffer which may cover several
> pages. Currently the function holds the mm->mmap_sem semaphore with
> read access while iterating over the buffer, since it could spawn
> several VMAs. When calling a lower-level function to handle the page
> fault for a single page, the semaphore is accessed again in read
> mode. That is wrong and can lead to deadlocks if a writer tries to
> sneak in while a buffer of several pages is being processed.
>
> The fix is to release the semaphore once cxllib_handle_fault() got the
> information it needs from the current vma. The address space/VMAs
> could evolve while we iterate over the full buffer, but in the
> unlikely case where we miss a page, the driver will raise a new page
> fault when retrying.
>
> Fixes: 3ced8d730063 ("cxl: Export library to support IBM XSL")
> Cc: stable@vger.kernel.org # 4.13+
> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
Reviewed-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
^ permalink raw reply
* Re: [PATCH 2/2] smp: introduce kick_active_cpus_sync()
From: Mark Rutland @ 2018-04-03 13:48 UTC (permalink / raw)
To: Yury Norov
Cc: Will Deacon, Paul E. McKenney, Chris Metcalf, Christopher Lameter,
Russell King - ARM Linux, Steven Rostedt, Mathieu Desnoyers,
Catalin Marinas, Pekka Enberg, David Rientjes, Joonsoo Kim,
Andrew Morton, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, linux-arm-kernel, linuxppc-dev, kvm-ppc,
linux-mm, linux-kernel
In-Reply-To: <20180401111108.mudkiewzn33sifvk@yury-thinkpad>
Hi Yury,
On Sun, Apr 01, 2018 at 02:11:08PM +0300, Yury Norov wrote:
> +/*
> + * Flush I-cache if CPU is in extended quiescent state
> + */
This comment is misleading. An ISB doesn't touch the I-cache; it forces
a context synchronization event.
> + .macro isb_if_eqs
> +#ifndef CONFIG_TINY_RCU
> + bl rcu_is_watching
> + tst w0, #0xff
> + b.ne 1f
The TST+B.NE can be a CBNZ:
bl rcu_is_watching
cbnz x0, 1f
isb
1:
> + /* Pairs with aarch64_insn_patch_text for EQS CPUs. */
> + isb
> +1:
> +#endif
> + .endm
> +
> el0_sync_invalid:
> inv_entry 0, BAD_SYNC
> ENDPROC(el0_sync_invalid)
> @@ -840,8 +861,10 @@ el0_svc:
> mov wsc_nr, #__NR_syscalls
> el0_svc_naked: // compat entry point
> stp x0, xscno, [sp, #S_ORIG_X0] // save the original x0 and syscall number
> + isb_if_eqs
> enable_dbg_and_irq
> - ct_user_exit 1
> + ct_user_exit
I don't think this is safe. here we issue the ISB *before* exiting a
quiesecent state, so I think we can race with another CPU that calls
kick_all_active_cpus_sync, e.g.
CPU0 CPU1
ISB
patch_some_text()
kick_all_active_cpus_sync()
ct_user_exit
// not synchronized!
use_of_patched_text()
... and therefore the ISB has no effect, which could be disasterous.
I believe we need the ISB *after* we transition into a non-quiescent
state, so that we can't possibly miss a context synchronization event.
Thanks,
Mark.
^ permalink raw reply
* [PATCH v2] cxl: Fix possible deadlock when processing page faults from cxllib
From: Frederic Barrat @ 2018-04-03 13:54 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: ldufour, clombard, andrew.donnellan, vaibhav
cxllib_handle_fault() is called by an external driver when it needs to
have the host resolve page faults for a buffer. The buffer can cover
several pages and VMAs. The function iterates over all the pages used
by the buffer, based on the page size of the VMA.
To ensure some stability while processing the faults, the thread T1
grabs the mm->mmap_sem semaphore with read access (R1). However, when
processing a page fault for a single page, one of the underlying
functions, copro_handle_mm_fault(), also grabs the same semaphore with
read access (R2). So the thread T1 takes the semaphore twice.
If another thread T2 tries to access the semaphore in write mode W1
(say, because it wants to allocate memory and calls 'brk'), then that
thread T2 will have to wait because there's a reader (R1). If the
thread T1 is processing a new page at that time, it won't get an
automatic grant at R2, because there's now a writer thread
waiting (T2). And we have a deadlock.
The timeline is:
1. thread T1 owns the semaphore with read access R1
2. thread T2 requests write access W1 and waits
3. thread T1 requests read access R2 and waits
The fix is for the thread T1 to release the semaphore R1 once it got
the information it needs from the current VMA. The address space/VMAs
could evolve while T1 iterates over the full buffer, but in the
unlikely case where T1 misses a page, the external driver will raise a
new page fault when retrying the memory access.
Fixes: 3ced8d730063 ("cxl: Export library to support IBM XSL")
Cc: stable@vger.kernel.org # 4.13+
Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
---
drivers/misc/cxl/cxllib.c | 85 ++++++++++++++++++++++++++++++-----------------
1 file changed, 55 insertions(+), 30 deletions(-)
diff --git a/drivers/misc/cxl/cxllib.c b/drivers/misc/cxl/cxllib.c
index 30ccba436b3b..55cd35d1a9cc 100644
--- a/drivers/misc/cxl/cxllib.c
+++ b/drivers/misc/cxl/cxllib.c
@@ -208,49 +208,74 @@ int cxllib_get_PE_attributes(struct task_struct *task,
}
EXPORT_SYMBOL_GPL(cxllib_get_PE_attributes);
-int cxllib_handle_fault(struct mm_struct *mm, u64 addr, u64 size, u64 flags)
+static int get_vma_info(struct mm_struct *mm, u64 addr,
+ u64 *vma_start, u64 *vma_end,
+ unsigned long *page_size)
{
- int rc;
- u64 dar;
struct vm_area_struct *vma = NULL;
- unsigned long page_size;
-
- if (mm == NULL)
- return -EFAULT;
+ int rc = 0;
down_read(&mm->mmap_sem);
vma = find_vma(mm, addr);
if (!vma) {
- pr_err("Can't find vma for addr %016llx\n", addr);
rc = -EFAULT;
goto out;
}
- /* get the size of the pages allocated */
- page_size = vma_kernel_pagesize(vma);
-
- for (dar = (addr & ~(page_size - 1)); dar < (addr + size); dar += page_size) {
- if (dar < vma->vm_start || dar >= vma->vm_end) {
- vma = find_vma(mm, addr);
- if (!vma) {
- pr_err("Can't find vma for addr %016llx\n", addr);
- rc = -EFAULT;
- goto out;
- }
- /* get the size of the pages allocated */
- page_size = vma_kernel_pagesize(vma);
+ *page_size = vma_kernel_pagesize(vma);
+ *vma_start = vma->vm_start;
+ *vma_end = vma->vm_end;
+out:
+ up_read(&mm->mmap_sem);
+ return rc;
+}
+
+int cxllib_handle_fault(struct mm_struct *mm, u64 addr, u64 size, u64 flags)
+{
+ int rc;
+ u64 dar, vma_start, vma_end;
+ unsigned long page_size;
+
+ if (mm == NULL)
+ return -EFAULT;
+
+ /*
+ * The buffer we have to process can extend over several pages
+ * and may also cover several VMAs.
+ * We iterate over all the pages. The page size could vary
+ * between VMAs.
+ */
+ rc = get_vma_info(mm, addr, &vma_start, &vma_end, &page_size);
+ if (rc)
+ return rc;
+
+ for (dar = (addr & ~(page_size - 1)); dar < (addr + size);
+ dar += page_size) {
+ if (dar < vma_start || dar >= vma_end) {
+ /*
+ * We don't hold the mm->mmap_sem semaphore
+ * while iterating, since the semaphore is
+ * required by one of the lower-level page
+ * fault processing functions and it could
+ * create a deadlock.
+ *
+ * It means the VMAs can be altered between 2
+ * loop iterations and we could theoretically
+ * miss a page (however unlikely). But that's
+ * not really a problem, as the driver will
+ * retry access, get another page fault on the
+ * missing page and call us again.
+ */
+ rc = get_vma_info(mm, dar, &vma_start, &vma_end,
+ &page_size);
+ if (rc)
+ return rc;
}
rc = cxl_handle_mm_fault(mm, flags, dar);
- if (rc) {
- pr_err("cxl_handle_mm_fault failed %d", rc);
- rc = -EFAULT;
- goto out;
- }
+ if (rc)
+ return -EFAULT;
}
- rc = 0;
-out:
- up_read(&mm->mmap_sem);
- return rc;
+ return 0;
}
EXPORT_SYMBOL_GPL(cxllib_handle_fault);
--
2.14.1
^ permalink raw reply related
* Re: [PATCH v2] cxl: Fix possible deadlock when processing page faults from cxllib
From: Laurent Dufour @ 2018-04-03 14:06 UTC (permalink / raw)
To: Frederic Barrat, linuxppc-dev, mpe; +Cc: clombard, andrew.donnellan, vaibhav
In-Reply-To: <20180403135402.30374-1-fbarrat@linux.vnet.ibm.com>
On 03/04/2018 15:54, Frederic Barrat wrote:
> cxllib_handle_fault() is called by an external driver when it needs to
> have the host resolve page faults for a buffer. The buffer can cover
> several pages and VMAs. The function iterates over all the pages used
> by the buffer, based on the page size of the VMA.
>
> To ensure some stability while processing the faults, the thread T1
> grabs the mm->mmap_sem semaphore with read access (R1). However, when
> processing a page fault for a single page, one of the underlying
> functions, copro_handle_mm_fault(), also grabs the same semaphore with
> read access (R2). So the thread T1 takes the semaphore twice.
>
> If another thread T2 tries to access the semaphore in write mode W1
> (say, because it wants to allocate memory and calls 'brk'), then that
> thread T2 will have to wait because there's a reader (R1). If the
> thread T1 is processing a new page at that time, it won't get an
> automatic grant at R2, because there's now a writer thread
> waiting (T2). And we have a deadlock.
>
> The timeline is:
> 1. thread T1 owns the semaphore with read access R1
> 2. thread T2 requests write access W1 and waits
> 3. thread T1 requests read access R2 and waits
>
> The fix is for the thread T1 to release the semaphore R1 once it got
> the information it needs from the current VMA. The address space/VMAs
> could evolve while T1 iterates over the full buffer, but in the
> unlikely case where T1 misses a page, the external driver will raise a
> new page fault when retrying the memory access.
>
> Fixes: 3ced8d730063 ("cxl: Export library to support IBM XSL")
> Cc: stable@vger.kernel.org # 4.13+
> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
What are the changes introduced in this v2 ?
> ---
> drivers/misc/cxl/cxllib.c | 85 ++++++++++++++++++++++++++++++-----------------
> 1 file changed, 55 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/misc/cxl/cxllib.c b/drivers/misc/cxl/cxllib.c
> index 30ccba436b3b..55cd35d1a9cc 100644
> --- a/drivers/misc/cxl/cxllib.c
> +++ b/drivers/misc/cxl/cxllib.c
> @@ -208,49 +208,74 @@ int cxllib_get_PE_attributes(struct task_struct *task,
> }
> EXPORT_SYMBOL_GPL(cxllib_get_PE_attributes);
>
> -int cxllib_handle_fault(struct mm_struct *mm, u64 addr, u64 size, u64 flags)
> +static int get_vma_info(struct mm_struct *mm, u64 addr,
> + u64 *vma_start, u64 *vma_end,
> + unsigned long *page_size)
> {
> - int rc;
> - u64 dar;
> struct vm_area_struct *vma = NULL;
> - unsigned long page_size;
> -
> - if (mm == NULL)
> - return -EFAULT;
> + int rc = 0;
>
> down_read(&mm->mmap_sem);
>
> vma = find_vma(mm, addr);
> if (!vma) {
> - pr_err("Can't find vma for addr %016llx\n", addr);
> rc = -EFAULT;
> goto out;
> }
> - /* get the size of the pages allocated */
> - page_size = vma_kernel_pagesize(vma);
> -
> - for (dar = (addr & ~(page_size - 1)); dar < (addr + size); dar += page_size) {
> - if (dar < vma->vm_start || dar >= vma->vm_end) {
> - vma = find_vma(mm, addr);
> - if (!vma) {
> - pr_err("Can't find vma for addr %016llx\n", addr);
> - rc = -EFAULT;
> - goto out;
> - }
> - /* get the size of the pages allocated */
> - page_size = vma_kernel_pagesize(vma);
> + *page_size = vma_kernel_pagesize(vma);
> + *vma_start = vma->vm_start;
> + *vma_end = vma->vm_end;
> +out:
> + up_read(&mm->mmap_sem);
> + return rc;
> +}
> +
> +int cxllib_handle_fault(struct mm_struct *mm, u64 addr, u64 size, u64 flags)
> +{
> + int rc;
> + u64 dar, vma_start, vma_end;
> + unsigned long page_size;
> +
> + if (mm == NULL)
> + return -EFAULT;
> +
> + /*
> + * The buffer we have to process can extend over several pages
> + * and may also cover several VMAs.
> + * We iterate over all the pages. The page size could vary
> + * between VMAs.
> + */
> + rc = get_vma_info(mm, addr, &vma_start, &vma_end, &page_size);
> + if (rc)
> + return rc;
> +
> + for (dar = (addr & ~(page_size - 1)); dar < (addr + size);
> + dar += page_size) {
> + if (dar < vma_start || dar >= vma_end) {
> + /*
> + * We don't hold the mm->mmap_sem semaphore
> + * while iterating, since the semaphore is
> + * required by one of the lower-level page
> + * fault processing functions and it could
> + * create a deadlock.
> + *
> + * It means the VMAs can be altered between 2
> + * loop iterations and we could theoretically
> + * miss a page (however unlikely). But that's
> + * not really a problem, as the driver will
> + * retry access, get another page fault on the
> + * missing page and call us again.
> + */
> + rc = get_vma_info(mm, dar, &vma_start, &vma_end,
> + &page_size);
> + if (rc)
> + return rc;
> }
>
> rc = cxl_handle_mm_fault(mm, flags, dar);
> - if (rc) {
> - pr_err("cxl_handle_mm_fault failed %d", rc);
> - rc = -EFAULT;
> - goto out;
> - }
> + if (rc)
> + return -EFAULT;
> }
> - rc = 0;
> -out:
> - up_read(&mm->mmap_sem);
> - return rc;
> + return 0;
> }
> EXPORT_SYMBOL_GPL(cxllib_handle_fault);
>
^ permalink raw reply
* Re: [PATCH v2] cxl: Fix possible deadlock when processing page faults from cxllib
From: Frederic Barrat @ 2018-04-03 14:15 UTC (permalink / raw)
To: Laurent Dufour, Frederic Barrat, linuxppc-dev, mpe
Cc: clombard, andrew.donnellan, vaibhav
In-Reply-To: <904fbfd8-5224-78b5-ca74-4c4dec1125e7@linux.vnet.ibm.com>
Le 03/04/2018 à 16:06, Laurent Dufour a écrit :
> On 03/04/2018 15:54, Frederic Barrat wrote:
>> cxllib_handle_fault() is called by an external driver when it needs to
>> have the host resolve page faults for a buffer. The buffer can cover
>> several pages and VMAs. The function iterates over all the pages used
>> by the buffer, based on the page size of the VMA.
>>
>> To ensure some stability while processing the faults, the thread T1
>> grabs the mm->mmap_sem semaphore with read access (R1). However, when
>> processing a page fault for a single page, one of the underlying
>> functions, copro_handle_mm_fault(), also grabs the same semaphore with
>> read access (R2). So the thread T1 takes the semaphore twice.
>>
>> If another thread T2 tries to access the semaphore in write mode W1
>> (say, because it wants to allocate memory and calls 'brk'), then that
>> thread T2 will have to wait because there's a reader (R1). If the
>> thread T1 is processing a new page at that time, it won't get an
>> automatic grant at R2, because there's now a writer thread
>> waiting (T2). And we have a deadlock.
>>
>> The timeline is:
>> 1. thread T1 owns the semaphore with read access R1
>> 2. thread T2 requests write access W1 and waits
>> 3. thread T1 requests read access R2 and waits
>>
>> The fix is for the thread T1 to release the semaphore R1 once it got
>> the information it needs from the current VMA. The address space/VMAs
>> could evolve while T1 iterates over the full buffer, but in the
>> unlikely case where T1 misses a page, the external driver will raise a
>> new page fault when retrying the memory access.
>>
>> Fixes: 3ced8d730063 ("cxl: Export library to support IBM XSL")
>> Cc: stable@vger.kernel.org # 4.13+
>> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
>
> What are the changes introduced in this v2 ?
How did that happen! I would have sworn to have added the changelog!
Code is the same. I rewrote the commit message to hopefully make it more
readable.
Fred
>> ---
>> drivers/misc/cxl/cxllib.c | 85 ++++++++++++++++++++++++++++++-----------------
>> 1 file changed, 55 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/misc/cxl/cxllib.c b/drivers/misc/cxl/cxllib.c
>> index 30ccba436b3b..55cd35d1a9cc 100644
>> --- a/drivers/misc/cxl/cxllib.c
>> +++ b/drivers/misc/cxl/cxllib.c
>> @@ -208,49 +208,74 @@ int cxllib_get_PE_attributes(struct task_struct *task,
>> }
>> EXPORT_SYMBOL_GPL(cxllib_get_PE_attributes);
>>
>> -int cxllib_handle_fault(struct mm_struct *mm, u64 addr, u64 size, u64 flags)
>> +static int get_vma_info(struct mm_struct *mm, u64 addr,
>> + u64 *vma_start, u64 *vma_end,
>> + unsigned long *page_size)
>> {
>> - int rc;
>> - u64 dar;
>> struct vm_area_struct *vma = NULL;
>> - unsigned long page_size;
>> -
>> - if (mm == NULL)
>> - return -EFAULT;
>> + int rc = 0;
>>
>> down_read(&mm->mmap_sem);
>>
>> vma = find_vma(mm, addr);
>> if (!vma) {
>> - pr_err("Can't find vma for addr %016llx\n", addr);
>> rc = -EFAULT;
>> goto out;
>> }
>> - /* get the size of the pages allocated */
>> - page_size = vma_kernel_pagesize(vma);
>> -
>> - for (dar = (addr & ~(page_size - 1)); dar < (addr + size); dar += page_size) {
>> - if (dar < vma->vm_start || dar >= vma->vm_end) {
>> - vma = find_vma(mm, addr);
>> - if (!vma) {
>> - pr_err("Can't find vma for addr %016llx\n", addr);
>> - rc = -EFAULT;
>> - goto out;
>> - }
>> - /* get the size of the pages allocated */
>> - page_size = vma_kernel_pagesize(vma);
>> + *page_size = vma_kernel_pagesize(vma);
>> + *vma_start = vma->vm_start;
>> + *vma_end = vma->vm_end;
>> +out:
>> + up_read(&mm->mmap_sem);
>> + return rc;
>> +}
>> +
>> +int cxllib_handle_fault(struct mm_struct *mm, u64 addr, u64 size, u64 flags)
>> +{
>> + int rc;
>> + u64 dar, vma_start, vma_end;
>> + unsigned long page_size;
>> +
>> + if (mm == NULL)
>> + return -EFAULT;
>> +
>> + /*
>> + * The buffer we have to process can extend over several pages
>> + * and may also cover several VMAs.
>> + * We iterate over all the pages. The page size could vary
>> + * between VMAs.
>> + */
>> + rc = get_vma_info(mm, addr, &vma_start, &vma_end, &page_size);
>> + if (rc)
>> + return rc;
>> +
>> + for (dar = (addr & ~(page_size - 1)); dar < (addr + size);
>> + dar += page_size) {
>> + if (dar < vma_start || dar >= vma_end) {
>> + /*
>> + * We don't hold the mm->mmap_sem semaphore
>> + * while iterating, since the semaphore is
>> + * required by one of the lower-level page
>> + * fault processing functions and it could
>> + * create a deadlock.
>> + *
>> + * It means the VMAs can be altered between 2
>> + * loop iterations and we could theoretically
>> + * miss a page (however unlikely). But that's
>> + * not really a problem, as the driver will
>> + * retry access, get another page fault on the
>> + * missing page and call us again.
>> + */
>> + rc = get_vma_info(mm, dar, &vma_start, &vma_end,
>> + &page_size);
>> + if (rc)
>> + return rc;
>> }
>>
>> rc = cxl_handle_mm_fault(mm, flags, dar);
>> - if (rc) {
>> - pr_err("cxl_handle_mm_fault failed %d", rc);
>> - rc = -EFAULT;
>> - goto out;
>> - }
>> + if (rc)
>> + return -EFAULT;
>> }
>> - rc = 0;
>> -out:
>> - up_read(&mm->mmap_sem);
>> - return rc;
>> + return 0;
>> }
>> EXPORT_SYMBOL_GPL(cxllib_handle_fault);
>>
^ permalink raw reply
* [PATCH 1/4] libnvdimm: Add of_node to region and bus descriptors
From: Oliver O'Halloran @ 2018-04-03 14:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: 000linux-nvdimm, Oliver O'Halloran
We want to be able to cross reference the region and bus devices
with the device tree node that they were spawned from. libNVDIMM
handles creating the actual devices for these internally, so we
need to pass in a pointer to the relevant node in the descriptor.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Acked-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/nvdimm/bus.c | 1 +
drivers/nvdimm/region_devs.c | 1 +
include/linux/libnvdimm.h | 3 +++
3 files changed, 5 insertions(+)
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index 78eabc3a1ab1..c6106914f396 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -358,6 +358,7 @@ struct nvdimm_bus *nvdimm_bus_register(struct device *parent,
nvdimm_bus->dev.release = nvdimm_bus_release;
nvdimm_bus->dev.groups = nd_desc->attr_groups;
nvdimm_bus->dev.bus = &nvdimm_bus_type;
+ nvdimm_bus->dev.of_node = nd_desc->of_node;
dev_set_name(&nvdimm_bus->dev, "ndbus%d", nvdimm_bus->id);
rc = device_register(&nvdimm_bus->dev);
if (rc) {
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index e6d01911e092..2f1d5771100e 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -1005,6 +1005,7 @@ static struct nd_region *nd_region_create(struct nvdimm_bus *nvdimm_bus,
dev->parent = &nvdimm_bus->dev;
dev->type = dev_type;
dev->groups = ndr_desc->attr_groups;
+ dev->of_node = ndr_desc->of_node;
nd_region->ndr_size = resource_size(ndr_desc->res);
nd_region->ndr_start = ndr_desc->res->start;
nd_device_register(dev);
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index ff855ed965fb..f61cb5050297 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -76,12 +76,14 @@ typedef int (*ndctl_fn)(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm, unsigned int cmd, void *buf,
unsigned int buf_len, int *cmd_rc);
+struct device_node;
struct nvdimm_bus_descriptor {
const struct attribute_group **attr_groups;
unsigned long bus_dsm_mask;
unsigned long cmd_mask;
struct module *module;
char *provider_name;
+ struct device_node *of_node;
ndctl_fn ndctl;
int (*flush_probe)(struct nvdimm_bus_descriptor *nd_desc);
int (*clear_to_send)(struct nvdimm_bus_descriptor *nd_desc,
@@ -123,6 +125,7 @@ struct nd_region_desc {
int num_lanes;
int numa_node;
unsigned long flags;
+ struct device_node *of_node;
};
struct device;
--
2.9.5
^ permalink raw reply related
* [PATCH 2/4] libnvdimm: Add device-tree based driver
From: Oliver O'Halloran @ 2018-04-03 14:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: 000linux-nvdimm, Oliver O'Halloran
In-Reply-To: <20180403142031.28629-1-oohall@gmail.com>
This patch adds peliminary device-tree bindings for persistent memory
regions. The driver registers a libnvdimm bus for each pmem-region
node and each address range under the node is converted to a region
within that bus.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
v2: Made each bus have a separate node rather having a shared bus.
Renamed to of_pmem rather than of_nvdimm.
Changed log level of happy-path messages to debug.
---
MAINTAINERS | 7 +++
drivers/nvdimm/Kconfig | 10 ++++
drivers/nvdimm/Makefile | 1 +
drivers/nvdimm/of_pmem.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 137 insertions(+)
create mode 100644 drivers/nvdimm/of_pmem.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 4e62756936fa..6ef38be700e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8035,6 +8035,13 @@ Q: https://patchwork.kernel.org/project/linux-nvdimm/list/
S: Supported
F: drivers/nvdimm/pmem*
+LIBNVDIMM: DEVICETREE BINDINGS
+M: Oliver O'Halloran <oohall@gmail.com>
+L: linux-nvdimm@lists.01.org
+Q: https://patchwork.kernel.org/project/linux-nvdimm/list/
+S: Supported
+F: drivers/nvdimm/of_pmem.c
+
LIBNVDIMM: NON-VOLATILE MEMORY DEVICE SUBSYSTEM
M: Dan Williams <dan.j.williams@intel.com>
L: linux-nvdimm@lists.01.org
diff --git a/drivers/nvdimm/Kconfig b/drivers/nvdimm/Kconfig
index a65f2e1d9f53..2d6862bf7436 100644
--- a/drivers/nvdimm/Kconfig
+++ b/drivers/nvdimm/Kconfig
@@ -102,4 +102,14 @@ config NVDIMM_DAX
Select Y if unsure
+config OF_PMEM
+ tristate "Device-tree support for persistent memory regions"
+ depends on OF
+ default LIBNVDIMM
+ help
+ Allows regions of persistent memory to be described in the
+ device-tree.
+
+ Select Y if unsure.
+
endif
diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
index 70d5f3ad9909..e8847045dac0 100644
--- a/drivers/nvdimm/Makefile
+++ b/drivers/nvdimm/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_BLK_DEV_PMEM) += nd_pmem.o
obj-$(CONFIG_ND_BTT) += nd_btt.o
obj-$(CONFIG_ND_BLK) += nd_blk.o
obj-$(CONFIG_X86_PMEM_LEGACY) += nd_e820.o
+obj-$(CONFIG_OF_PMEM) += of_pmem.o
nd_pmem-y := pmem.o
diff --git a/drivers/nvdimm/of_pmem.c b/drivers/nvdimm/of_pmem.c
new file mode 100644
index 000000000000..374c796ea1de
--- /dev/null
+++ b/drivers/nvdimm/of_pmem.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#define pr_fmt(fmt) "of_pmem: " fmt
+
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/libnvdimm.h>
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/slab.h>
+
+static const struct attribute_group *region_attr_groups[] = {
+ &nd_region_attribute_group,
+ &nd_device_attribute_group,
+ NULL,
+};
+
+static const struct attribute_group *bus_attr_groups[] = {
+ &nvdimm_bus_attribute_group,
+ NULL,
+};
+
+struct of_nd_private {
+ struct nvdimm_bus_descriptor bus_desc;
+ struct nvdimm_bus *bus;
+};
+
+static int of_nd_region_probe(struct platform_device *pdev)
+{
+ struct of_nd_private *priv;
+ struct device_node *np;
+ struct nvdimm_bus *bus;
+ bool is_volatile;
+ int i;
+
+ np = dev_of_node(&pdev->dev);
+ if (!np)
+ return -ENXIO;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->bus_desc.attr_groups = bus_attr_groups;
+ priv->bus_desc.provider_name = "of_pmem";
+ priv->bus_desc.module = THIS_MODULE;
+ priv->bus_desc.of_node = np;
+
+ priv->bus = bus = nvdimm_bus_register(&pdev->dev, &priv->bus_desc);
+ if (!bus) {
+ kfree(priv);
+ return -ENODEV;
+ }
+ platform_set_drvdata(pdev, priv);
+
+ is_volatile = !!of_find_property(np, "volatile", NULL);
+ dev_dbg(&pdev->dev, "Registering %s regions from %pOF\n",
+ is_volatile ? "volatile" : "non-volatile", np);
+
+ for (i = 0; i < pdev->num_resources; i++) {
+ struct nd_region_desc ndr_desc;
+ struct nd_region *region;
+
+ /*
+ * NB: libnvdimm copies the data from ndr_desc into it's own
+ * structures so passing a stack pointer is fine.
+ */
+ memset(&ndr_desc, 0, sizeof(ndr_desc));
+ ndr_desc.attr_groups = region_attr_groups;
+ ndr_desc.numa_node = of_node_to_nid(np);
+ ndr_desc.res = &pdev->resource[i];
+ ndr_desc.of_node = np;
+ set_bit(ND_REGION_PAGEMAP, &ndr_desc.flags);
+
+ if (is_volatile)
+ region = nvdimm_volatile_region_create(bus, &ndr_desc);
+ else
+ region = nvdimm_pmem_region_create(bus, &ndr_desc);
+
+ if (!region)
+ dev_warn(&pdev->dev, "Unable to register region %pR from %pOF\n",
+ ndr_desc.res, np);
+ else
+ dev_dbg(&pdev->dev, "Registered region %pR from %pOF\n",
+ ndr_desc.res, np);
+ }
+
+ return 0;
+}
+
+static int of_nd_region_remove(struct platform_device *pdev)
+{
+ struct of_nd_private *priv = platform_get_drvdata(pdev);
+
+ nvdimm_bus_unregister(priv->bus);
+ kfree(priv);
+
+ return 0;
+}
+
+static const struct of_device_id of_nd_region_match[] = {
+ { .compatible = "pmem-region" },
+ { },
+};
+
+static struct platform_driver of_nd_region_driver = {
+ .probe = of_nd_region_probe,
+ .remove = of_nd_region_remove,
+ .driver = {
+ .name = "of_pmem",
+ .owner = THIS_MODULE,
+ .of_match_table = of_nd_region_match,
+ },
+};
+
+module_platform_driver(of_nd_region_driver);
+MODULE_DEVICE_TABLE(of, of_nd_region_match);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("IBM Corporation");
--
2.9.5
^ permalink raw reply related
* [PATCH 3/4] doc/devicetree: Persistent memory region bindings
From: Oliver O'Halloran @ 2018-04-03 14:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: 000linux-nvdimm, Oliver O'Halloran, devicetree
In-Reply-To: <20180403142031.28629-1-oohall@gmail.com>
Add device-tree binding documentation for the nvdimm region driver.
Cc: devicetree@vger.kernel.org
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
v2: Changed name from nvdimm-region to pmem-region.
Cleaned up the example binding and fixed the overlapping regions.
Added support for multiple regions in a single reg.
---
.../devicetree/bindings/pmem/pmem-region.txt | 80 ++++++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 81 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pmem/pmem-region.txt
diff --git a/Documentation/devicetree/bindings/pmem/pmem-region.txt b/Documentation/devicetree/bindings/pmem/pmem-region.txt
new file mode 100644
index 000000000000..de48dc8cd562
--- /dev/null
+++ b/Documentation/devicetree/bindings/pmem/pmem-region.txt
@@ -0,0 +1,80 @@
+Device-tree bindings for persistent memory regions
+-----------------------------------------------------
+
+Persistent memory refers to a class of memory devices that are:
+
+ a) Usable as main system memory (i.e. cacheable), and
+ b) Retain their contents across power failure.
+
+Given b) it is best to think of persistent memory as a kind of memory mapped
+storage device. To ensure data integrity the operating system needs to manage
+persistent regions separately to the normal memory pool. To aid with that this
+binding provides a standardised interface for discovering where persistent
+memory regions exist inside the physical address space.
+
+Bindings for the region nodes:
+-----------------------------
+
+Required properties:
+ - compatible = "pmem-region"
+
+ - reg = <base, size>;
+ The system physical address range of this nvdimm region.
+
+ If the reg property contains multiple address ranges
+ each address range will be treated as though it was specified
+ in a separate device node. Having multiple address ranges in a
+ node implies no special relationship between the two ranges.
+
+Optional properties:
+ - Any relevant NUMA assocativity properties for the target platform.
+
+ - A "volatile" property indicating that this region is actually in
+ normal DRAM and does not require cache flushes after each write.
+
+ If this property is absent then the OS must assume that the region
+ is backed by non-volatile memory.
+
+A complete example:
+--------------------
+
+Here we define three 4KB regions:
+
+ a) A volatile region at 0x5000 on numa node 0,
+ b) A non-volatile region at 0x6000, and
+ c) A non-volatile region at 0x8000.
+
+/dts-v1/;
+
+/ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ platform {
+ compatible = "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ /*
+ * This node specifies one non-volatile region spanning from
+ * 0x5000 to 0x5fff.
+ */
+ pmem@5000 {
+ compatible = "pmem-region";
+ reg = <0x00005000 0x00001000>;
+ numa-node-id = <0>;
+ volatile;
+ };
+
+ /*
+ * This node specifies two 4KB regions that are backed by
+ * volatile (normal) memory.
+ */
+ pmem@6000 {
+ compatible = "pmem-region";
+ reg = <0x00006000 0x00001000 0x00008000 0x00001000>;
+ };
+ };
+};
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 6ef38be700e8..fa3c9211d6ff 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8041,6 +8041,7 @@ L: linux-nvdimm@lists.01.org
Q: https://patchwork.kernel.org/project/linux-nvdimm/list/
S: Supported
F: drivers/nvdimm/of_pmem.c
+F: Documentation/devicetree/bindings/pmem/pmem-region.txt
LIBNVDIMM: NON-VOLATILE MEMORY DEVICE SUBSYSTEM
M: Dan Williams <dan.j.williams@intel.com>
--
2.9.5
^ permalink raw reply related
* [PATCH 4/4] powerpc/powernv: Create platform devs for nvdimm buses
From: Oliver O'Halloran @ 2018-04-03 14:20 UTC (permalink / raw)
To: linuxppc-dev; +Cc: 000linux-nvdimm, Oliver O'Halloran
In-Reply-To: <20180403142031.28629-1-oohall@gmail.com>
Scan the devicetree for an nvdimm-bus compatible and create
a platform device for them.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
arch/powerpc/platforms/powernv/opal.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index c15182765ff5..c37485a3c5c9 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -821,6 +821,9 @@ static int __init opal_init(void)
/* Create i2c platform devices */
opal_pdev_init("ibm,opal-i2c");
+ /* Handle non-volatile memory devices */
+ opal_pdev_init("pmem-region");
+
/* Setup a heatbeat thread if requested by OPAL */
opal_init_heartbeat();
--
2.9.5
^ permalink raw reply related
* [RESEND v2 1/4] libnvdimm: Add of_node to region and bus descriptors
From: Oliver O'Halloran @ 2018-04-03 14:24 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-nvdimm, Oliver O'Halloran
We want to be able to cross reference the region and bus devices
with the device tree node that they were spawned from. libNVDIMM
handles creating the actual devices for these internally, so we
need to pass in a pointer to the relevant node in the descriptor.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Acked-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/nvdimm/bus.c | 1 +
drivers/nvdimm/region_devs.c | 1 +
include/linux/libnvdimm.h | 3 +++
3 files changed, 5 insertions(+)
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index 78eabc3a1ab1..c6106914f396 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -358,6 +358,7 @@ struct nvdimm_bus *nvdimm_bus_register(struct device *parent,
nvdimm_bus->dev.release = nvdimm_bus_release;
nvdimm_bus->dev.groups = nd_desc->attr_groups;
nvdimm_bus->dev.bus = &nvdimm_bus_type;
+ nvdimm_bus->dev.of_node = nd_desc->of_node;
dev_set_name(&nvdimm_bus->dev, "ndbus%d", nvdimm_bus->id);
rc = device_register(&nvdimm_bus->dev);
if (rc) {
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index e6d01911e092..2f1d5771100e 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -1005,6 +1005,7 @@ static struct nd_region *nd_region_create(struct nvdimm_bus *nvdimm_bus,
dev->parent = &nvdimm_bus->dev;
dev->type = dev_type;
dev->groups = ndr_desc->attr_groups;
+ dev->of_node = ndr_desc->of_node;
nd_region->ndr_size = resource_size(ndr_desc->res);
nd_region->ndr_start = ndr_desc->res->start;
nd_device_register(dev);
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index ff855ed965fb..f61cb5050297 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -76,12 +76,14 @@ typedef int (*ndctl_fn)(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm, unsigned int cmd, void *buf,
unsigned int buf_len, int *cmd_rc);
+struct device_node;
struct nvdimm_bus_descriptor {
const struct attribute_group **attr_groups;
unsigned long bus_dsm_mask;
unsigned long cmd_mask;
struct module *module;
char *provider_name;
+ struct device_node *of_node;
ndctl_fn ndctl;
int (*flush_probe)(struct nvdimm_bus_descriptor *nd_desc);
int (*clear_to_send)(struct nvdimm_bus_descriptor *nd_desc,
@@ -123,6 +125,7 @@ struct nd_region_desc {
int num_lanes;
int numa_node;
unsigned long flags;
+ struct device_node *of_node;
};
struct device;
--
2.9.5
^ permalink raw reply related
* [RESEND v2 2/4] libnvdimm: Add device-tree based driver
From: Oliver O'Halloran @ 2018-04-03 14:24 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-nvdimm, Oliver O'Halloran
In-Reply-To: <20180403142415.30083-1-oohall@gmail.com>
This patch adds peliminary device-tree bindings for persistent memory
regions. The driver registers a libnvdimm bus for each pmem-region
node and each address range under the node is converted to a region
within that bus.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
v2: Made each bus have a separate node rather having a shared bus.
Renamed to of_pmem rather than of_nvdimm.
Changed log level of happy-path messages to debug.
---
MAINTAINERS | 7 +++
drivers/nvdimm/Kconfig | 10 ++++
drivers/nvdimm/Makefile | 1 +
drivers/nvdimm/of_pmem.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 137 insertions(+)
create mode 100644 drivers/nvdimm/of_pmem.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 4e62756936fa..6ef38be700e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8035,6 +8035,13 @@ Q: https://patchwork.kernel.org/project/linux-nvdimm/list/
S: Supported
F: drivers/nvdimm/pmem*
+LIBNVDIMM: DEVICETREE BINDINGS
+M: Oliver O'Halloran <oohall@gmail.com>
+L: linux-nvdimm@lists.01.org
+Q: https://patchwork.kernel.org/project/linux-nvdimm/list/
+S: Supported
+F: drivers/nvdimm/of_pmem.c
+
LIBNVDIMM: NON-VOLATILE MEMORY DEVICE SUBSYSTEM
M: Dan Williams <dan.j.williams@intel.com>
L: linux-nvdimm@lists.01.org
diff --git a/drivers/nvdimm/Kconfig b/drivers/nvdimm/Kconfig
index a65f2e1d9f53..2d6862bf7436 100644
--- a/drivers/nvdimm/Kconfig
+++ b/drivers/nvdimm/Kconfig
@@ -102,4 +102,14 @@ config NVDIMM_DAX
Select Y if unsure
+config OF_PMEM
+ tristate "Device-tree support for persistent memory regions"
+ depends on OF
+ default LIBNVDIMM
+ help
+ Allows regions of persistent memory to be described in the
+ device-tree.
+
+ Select Y if unsure.
+
endif
diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
index 70d5f3ad9909..e8847045dac0 100644
--- a/drivers/nvdimm/Makefile
+++ b/drivers/nvdimm/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_BLK_DEV_PMEM) += nd_pmem.o
obj-$(CONFIG_ND_BTT) += nd_btt.o
obj-$(CONFIG_ND_BLK) += nd_blk.o
obj-$(CONFIG_X86_PMEM_LEGACY) += nd_e820.o
+obj-$(CONFIG_OF_PMEM) += of_pmem.o
nd_pmem-y := pmem.o
diff --git a/drivers/nvdimm/of_pmem.c b/drivers/nvdimm/of_pmem.c
new file mode 100644
index 000000000000..374c796ea1de
--- /dev/null
+++ b/drivers/nvdimm/of_pmem.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#define pr_fmt(fmt) "of_pmem: " fmt
+
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/libnvdimm.h>
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/slab.h>
+
+static const struct attribute_group *region_attr_groups[] = {
+ &nd_region_attribute_group,
+ &nd_device_attribute_group,
+ NULL,
+};
+
+static const struct attribute_group *bus_attr_groups[] = {
+ &nvdimm_bus_attribute_group,
+ NULL,
+};
+
+struct of_nd_private {
+ struct nvdimm_bus_descriptor bus_desc;
+ struct nvdimm_bus *bus;
+};
+
+static int of_nd_region_probe(struct platform_device *pdev)
+{
+ struct of_nd_private *priv;
+ struct device_node *np;
+ struct nvdimm_bus *bus;
+ bool is_volatile;
+ int i;
+
+ np = dev_of_node(&pdev->dev);
+ if (!np)
+ return -ENXIO;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->bus_desc.attr_groups = bus_attr_groups;
+ priv->bus_desc.provider_name = "of_pmem";
+ priv->bus_desc.module = THIS_MODULE;
+ priv->bus_desc.of_node = np;
+
+ priv->bus = bus = nvdimm_bus_register(&pdev->dev, &priv->bus_desc);
+ if (!bus) {
+ kfree(priv);
+ return -ENODEV;
+ }
+ platform_set_drvdata(pdev, priv);
+
+ is_volatile = !!of_find_property(np, "volatile", NULL);
+ dev_dbg(&pdev->dev, "Registering %s regions from %pOF\n",
+ is_volatile ? "volatile" : "non-volatile", np);
+
+ for (i = 0; i < pdev->num_resources; i++) {
+ struct nd_region_desc ndr_desc;
+ struct nd_region *region;
+
+ /*
+ * NB: libnvdimm copies the data from ndr_desc into it's own
+ * structures so passing a stack pointer is fine.
+ */
+ memset(&ndr_desc, 0, sizeof(ndr_desc));
+ ndr_desc.attr_groups = region_attr_groups;
+ ndr_desc.numa_node = of_node_to_nid(np);
+ ndr_desc.res = &pdev->resource[i];
+ ndr_desc.of_node = np;
+ set_bit(ND_REGION_PAGEMAP, &ndr_desc.flags);
+
+ if (is_volatile)
+ region = nvdimm_volatile_region_create(bus, &ndr_desc);
+ else
+ region = nvdimm_pmem_region_create(bus, &ndr_desc);
+
+ if (!region)
+ dev_warn(&pdev->dev, "Unable to register region %pR from %pOF\n",
+ ndr_desc.res, np);
+ else
+ dev_dbg(&pdev->dev, "Registered region %pR from %pOF\n",
+ ndr_desc.res, np);
+ }
+
+ return 0;
+}
+
+static int of_nd_region_remove(struct platform_device *pdev)
+{
+ struct of_nd_private *priv = platform_get_drvdata(pdev);
+
+ nvdimm_bus_unregister(priv->bus);
+ kfree(priv);
+
+ return 0;
+}
+
+static const struct of_device_id of_nd_region_match[] = {
+ { .compatible = "pmem-region" },
+ { },
+};
+
+static struct platform_driver of_nd_region_driver = {
+ .probe = of_nd_region_probe,
+ .remove = of_nd_region_remove,
+ .driver = {
+ .name = "of_pmem",
+ .owner = THIS_MODULE,
+ .of_match_table = of_nd_region_match,
+ },
+};
+
+module_platform_driver(of_nd_region_driver);
+MODULE_DEVICE_TABLE(of, of_nd_region_match);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("IBM Corporation");
--
2.9.5
^ permalink raw reply related
* [RESEND v2 3/4] doc/devicetree: Persistent memory region bindings
From: Oliver O'Halloran @ 2018-04-03 14:24 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-nvdimm, Oliver O'Halloran, devicetree
In-Reply-To: <20180403142415.30083-1-oohall@gmail.com>
Add device-tree binding documentation for the nvdimm region driver.
Cc: devicetree@vger.kernel.org
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
v2: Changed name from nvdimm-region to pmem-region.
Cleaned up the example binding and fixed the overlapping regions.
Added support for multiple regions in a single reg.
---
.../devicetree/bindings/pmem/pmem-region.txt | 80 ++++++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 81 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pmem/pmem-region.txt
diff --git a/Documentation/devicetree/bindings/pmem/pmem-region.txt b/Documentation/devicetree/bindings/pmem/pmem-region.txt
new file mode 100644
index 000000000000..de48dc8cd562
--- /dev/null
+++ b/Documentation/devicetree/bindings/pmem/pmem-region.txt
@@ -0,0 +1,80 @@
+Device-tree bindings for persistent memory regions
+-----------------------------------------------------
+
+Persistent memory refers to a class of memory devices that are:
+
+ a) Usable as main system memory (i.e. cacheable), and
+ b) Retain their contents across power failure.
+
+Given b) it is best to think of persistent memory as a kind of memory mapped
+storage device. To ensure data integrity the operating system needs to manage
+persistent regions separately to the normal memory pool. To aid with that this
+binding provides a standardised interface for discovering where persistent
+memory regions exist inside the physical address space.
+
+Bindings for the region nodes:
+-----------------------------
+
+Required properties:
+ - compatible = "pmem-region"
+
+ - reg = <base, size>;
+ The system physical address range of this nvdimm region.
+
+ If the reg property contains multiple address ranges
+ each address range will be treated as though it was specified
+ in a separate device node. Having multiple address ranges in a
+ node implies no special relationship between the two ranges.
+
+Optional properties:
+ - Any relevant NUMA assocativity properties for the target platform.
+
+ - A "volatile" property indicating that this region is actually in
+ normal DRAM and does not require cache flushes after each write.
+
+ If this property is absent then the OS must assume that the region
+ is backed by non-volatile memory.
+
+A complete example:
+--------------------
+
+Here we define three 4KB regions:
+
+ a) A volatile region at 0x5000 on numa node 0,
+ b) A non-volatile region at 0x6000, and
+ c) A non-volatile region at 0x8000.
+
+/dts-v1/;
+
+/ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ platform {
+ compatible = "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ /*
+ * This node specifies one non-volatile region spanning from
+ * 0x5000 to 0x5fff.
+ */
+ pmem@5000 {
+ compatible = "pmem-region";
+ reg = <0x00005000 0x00001000>;
+ numa-node-id = <0>;
+ volatile;
+ };
+
+ /*
+ * This node specifies two 4KB regions that are backed by
+ * volatile (normal) memory.
+ */
+ pmem@6000 {
+ compatible = "pmem-region";
+ reg = <0x00006000 0x00001000 0x00008000 0x00001000>;
+ };
+ };
+};
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 6ef38be700e8..fa3c9211d6ff 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8041,6 +8041,7 @@ L: linux-nvdimm@lists.01.org
Q: https://patchwork.kernel.org/project/linux-nvdimm/list/
S: Supported
F: drivers/nvdimm/of_pmem.c
+F: Documentation/devicetree/bindings/pmem/pmem-region.txt
LIBNVDIMM: NON-VOLATILE MEMORY DEVICE SUBSYSTEM
M: Dan Williams <dan.j.williams@intel.com>
--
2.9.5
^ permalink raw reply related
* [RESEND v2 4/4] powerpc/powernv: Create platform devs for nvdimm buses
From: Oliver O'Halloran @ 2018-04-03 14:24 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linux-nvdimm, Oliver O'Halloran
In-Reply-To: <20180403142415.30083-1-oohall@gmail.com>
Scan the devicetree for an nvdimm-bus compatible and create
a platform device for them.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
arch/powerpc/platforms/powernv/opal.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index c15182765ff5..c37485a3c5c9 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -821,6 +821,9 @@ static int __init opal_init(void)
/* Create i2c platform devices */
opal_pdev_init("ibm,opal-i2c");
+ /* Handle non-volatile memory devices */
+ opal_pdev_init("pmem-region");
+
/* Setup a heatbeat thread if requested by OPAL */
opal_init_heartbeat();
--
2.9.5
^ permalink raw reply related
* Re: [PATCH] cxl: Fix possible deadlock when processing page faults from cxllib
From: Aneesh Kumar K.V @ 2018-04-03 14:40 UTC (permalink / raw)
To: Frederic Barrat, linuxppc-dev, mpe
Cc: ldufour, clombard, andrew.donnellan, vaibhav
In-Reply-To: <20180403094311.331-1-fbarrat@linux.vnet.ibm.com>
On 04/03/2018 03:13 PM, Frederic Barrat wrote:
> cxllib_handle_fault() is called by an external driver when it needs to
> have the host process page faults for a buffer which may cover several
> pages. Currently the function holds the mm->mmap_sem semaphore with
> read access while iterating over the buffer, since it could spawn
> several VMAs. When calling a lower-level function to handle the page
> fault for a single page, the semaphore is accessed again in read
> mode. That is wrong and can lead to deadlocks if a writer tries to
> sneak in while a buffer of several pages is being processed.
>
> The fix is to release the semaphore once cxllib_handle_fault() got the
> information it needs from the current vma. The address space/VMAs
> could evolve while we iterate over the full buffer, but in the
> unlikely case where we miss a page, the driver will raise a new page
> fault when retrying.
>
> Fixes: 3ced8d730063 ("cxl: Export library to support IBM XSL")
> Cc: stable@vger.kernel.org # 4.13+
> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> ---
> drivers/misc/cxl/cxllib.c | 85 ++++++++++++++++++++++++++++++-----------------
> 1 file changed, 55 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/misc/cxl/cxllib.c b/drivers/misc/cxl/cxllib.c
> index 30ccba436b3b..55cd35d1a9cc 100644
> --- a/drivers/misc/cxl/cxllib.c
> +++ b/drivers/misc/cxl/cxllib.c
> @@ -208,49 +208,74 @@ int cxllib_get_PE_attributes(struct task_struct *task,
> }
> EXPORT_SYMBOL_GPL(cxllib_get_PE_attributes);
>
> -int cxllib_handle_fault(struct mm_struct *mm, u64 addr, u64 size, u64 flags)
> +static int get_vma_info(struct mm_struct *mm, u64 addr,
> + u64 *vma_start, u64 *vma_end,
> + unsigned long *page_size)
> {
> - int rc;
> - u64 dar;
> struct vm_area_struct *vma = NULL;
> - unsigned long page_size;
> -
> - if (mm == NULL)
> - return -EFAULT;
> + int rc = 0;
>
> down_read(&mm->mmap_sem);
>
> vma = find_vma(mm, addr);
> if (!vma) {
> - pr_err("Can't find vma for addr %016llx\n", addr);
> rc = -EFAULT;
> goto out;
> }
> - /* get the size of the pages allocated */
> - page_size = vma_kernel_pagesize(vma);
> -
> - for (dar = (addr & ~(page_size - 1)); dar < (addr + size); dar += page_size) {
> - if (dar < vma->vm_start || dar >= vma->vm_end) {
> - vma = find_vma(mm, addr);
> - if (!vma) {
> - pr_err("Can't find vma for addr %016llx\n", addr);
> - rc = -EFAULT;
> - goto out;
> - }
> - /* get the size of the pages allocated */
> - page_size = vma_kernel_pagesize(vma);
> + *page_size = vma_kernel_pagesize(vma);
> + *vma_start = vma->vm_start;
> + *vma_end = vma->vm_end;
> +out:
> + up_read(&mm->mmap_sem);
> + return rc;
> +}
> +
> +int cxllib_handle_fault(struct mm_struct *mm, u64 addr, u64 size, u64 flags)
> +{
> + int rc;
> + u64 dar, vma_start, vma_end;
> + unsigned long page_size;
> +
> + if (mm == NULL)
> + return -EFAULT;
> +
> + /*
> + * The buffer we have to process can extend over several pages
> + * and may also cover several VMAs.
> + * We iterate over all the pages. The page size could vary
> + * between VMAs.
> + */
> + rc = get_vma_info(mm, addr, &vma_start, &vma_end, &page_size);
> + if (rc)
> + return rc;
> +
> + for (dar = (addr & ~(page_size - 1)); dar < (addr + size);
> + dar += page_size) {
> + if (dar < vma_start || dar >= vma_end) {
IIUC, we are fetching the vma to get just the page_size with which it is
mapped? Can't we iterate with PAGE_SIZE? Considering hugetlb page size
will be larger than PAGE_SIZE, we might call into cxl_handle_mm_fault
multiple times for a hugetlb page. Does that cause any issue? Also can
cxl be used with hugetlb mappings?
> + /*
> + * We don't hold the mm->mmap_sem semaphore
> + * while iterating, since the semaphore is
> + * required by one of the lower-level page
> + * fault processing functions and it could
> + * create a deadlock.
> + *
> + * It means the VMAs can be altered between 2
> + * loop iterations and we could theoretically
> + * miss a page (however unlikely). But that's
> + * not really a problem, as the driver will
> + * retry access, get another page fault on the
> + * missing page and call us again.
> + */
> + rc = get_vma_info(mm, dar, &vma_start, &vma_end,
> + &page_size);
> + if (rc)
> + return rc;
> }
>
> rc = cxl_handle_mm_fault(mm, flags, dar);
> - if (rc) {
> - pr_err("cxl_handle_mm_fault failed %d", rc);
> - rc = -EFAULT;
> - goto out;
> - }
> + if (rc)
> + return -EFAULT;
> }
> - rc = 0;
> -out:
> - up_read(&mm->mmap_sem);
> - return rc;
> + return 0;
> }
> EXPORT_SYMBOL_GPL(cxllib_handle_fault);
>
-aneesh
^ permalink raw reply
* Re: [PATCH] cxl: Fix possible deadlock when processing page faults from cxllib
From: Aneesh Kumar K.V @ 2018-04-03 15:31 UTC (permalink / raw)
To: Frederic Barrat, linuxppc-dev, mpe
Cc: ldufour, clombard, andrew.donnellan, vaibhav
In-Reply-To: <606e7509-5c37-b38e-9b6c-a60a5694c652@linux.ibm.com>
On 04/03/2018 08:10 PM, Aneesh Kumar K.V wrote:
> On 04/03/2018 03:13 PM, Frederic Barrat wrote:
>> cxllib_handle_fault() is called by an external driver when it needs to
>> have the host process page faults for a buffer which may cover several
>> pages. Currently the function holds the mm->mmap_sem semaphore with
>> read access while iterating over the buffer, since it could spawn
>> several VMAs. When calling a lower-level function to handle the page
>> fault for a single page, the semaphore is accessed again in read
>> mode. That is wrong and can lead to deadlocks if a writer tries to
>> sneak in while a buffer of several pages is being processed.
>>
>> The fix is to release the semaphore once cxllib_handle_fault() got the
>> information it needs from the current vma. The address space/VMAs
>> could evolve while we iterate over the full buffer, but in the
>> unlikely case where we miss a page, the driver will raise a new page
>> fault when retrying.
>>
>> Fixes: 3ced8d730063 ("cxl: Export library to support IBM XSL")
>> Cc: stable@vger.kernel.org # 4.13+
>> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
>> ---
>> drivers/misc/cxl/cxllib.c | 85
>> ++++++++++++++++++++++++++++++-----------------
>> 1 file changed, 55 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/misc/cxl/cxllib.c b/drivers/misc/cxl/cxllib.c
>> index 30ccba436b3b..55cd35d1a9cc 100644
>> --- a/drivers/misc/cxl/cxllib.c
>> +++ b/drivers/misc/cxl/cxllib.c
>> @@ -208,49 +208,74 @@ int cxllib_get_PE_attributes(struct task_struct
>> *task,
>> }
>> EXPORT_SYMBOL_GPL(cxllib_get_PE_attributes);
>> -int cxllib_handle_fault(struct mm_struct *mm, u64 addr, u64 size, u64
>> flags)
>> +static int get_vma_info(struct mm_struct *mm, u64 addr,
>> + u64 *vma_start, u64 *vma_end,
>> + unsigned long *page_size)
>> {
>> - int rc;
>> - u64 dar;
>> struct vm_area_struct *vma = NULL;
>> - unsigned long page_size;
>> -
>> - if (mm == NULL)
>> - return -EFAULT;
>> + int rc = 0;
>> down_read(&mm->mmap_sem);
>> vma = find_vma(mm, addr);
>> if (!vma) {
>> - pr_err("Can't find vma for addr %016llx\n", addr);
>> rc = -EFAULT;
>> goto out;
>> }
>> - /* get the size of the pages allocated */
>> - page_size = vma_kernel_pagesize(vma);
>> -
>> - for (dar = (addr & ~(page_size - 1)); dar < (addr + size); dar +=
>> page_size) {
>> - if (dar < vma->vm_start || dar >= vma->vm_end) {
>> - vma = find_vma(mm, addr);
>> - if (!vma) {
>> - pr_err("Can't find vma for addr %016llx\n", addr);
>> - rc = -EFAULT;
>> - goto out;
>> - }
>> - /* get the size of the pages allocated */
>> - page_size = vma_kernel_pagesize(vma);
>> + *page_size = vma_kernel_pagesize(vma);
>> + *vma_start = vma->vm_start;
>> + *vma_end = vma->vm_end;
>> +out:
>> + up_read(&mm->mmap_sem);
>> + return rc;
>> +}
>> +
>> +int cxllib_handle_fault(struct mm_struct *mm, u64 addr, u64 size, u64
>> flags)
>> +{
>> + int rc;
>> + u64 dar, vma_start, vma_end;
>> + unsigned long page_size;
>> +
>> + if (mm == NULL)
>> + return -EFAULT;
>> +
>> + /*
>> + * The buffer we have to process can extend over several pages
>> + * and may also cover several VMAs.
>> + * We iterate over all the pages. The page size could vary
>> + * between VMAs.
>> + */
>> + rc = get_vma_info(mm, addr, &vma_start, &vma_end, &page_size);
>> + if (rc)
>> + return rc;
>> +
>> + for (dar = (addr & ~(page_size - 1)); dar < (addr + size);
>> + dar += page_size) {
>> + if (dar < vma_start || dar >= vma_end) {
>
>
> IIUC, we are fetching the vma to get just the page_size with which it is
> mapped? Can't we iterate with PAGE_SIZE? Considering hugetlb page size
> will be larger than PAGE_SIZE, we might call into cxl_handle_mm_fault
> multiple times for a hugetlb page. Does that cause any issue? Also can
> cxl be used with hugetlb mappings?
>
Can you also try to use a helper like below. That will clarify the need
of find_vma there.
static int __cxllib_handle_fault(struct mm_struct *mm, unsigned long
start, unsigned long end,
unsigned long mapping_psize, u64 flags)
{
int rc;
unsigned long dar;
for (dar = start; dar < end; dar += mapping_psize) {
rc = cxl_handle_mm_fault(mm, flags, dar);
if (rc) {
rc = -EFAULT;
goto out;
}
}
rc = 0;
out:
return rc;
}
-aneesh
^ permalink raw reply
* [PATCH] selftests/powerpc: Fix copyloops build since Power4 assembler change
From: Michael Ellerman @ 2018-04-03 15:53 UTC (permalink / raw)
To: linuxppc-dev
The recent commit 15a3204d24a3 ("powerpc/64s: Set assembler machine
type to POWER4") set the machine type in our ASFLAGS when building the
kernel, and removed some ".machine power4" directives from various asm
files.
This broke the selftests build on old toolchains (that don't assume
Power4), because we build the kernel source files into the selftests
using different ASFLAGS.
The fix is simply to add -mpower4 to the selftest ASFLAGS as well.
Fixes: 15a3204d24a3 ("powerpc/64s: Set assembler machine type to POWER4")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
tools/testing/selftests/powerpc/copyloops/Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/powerpc/copyloops/Makefile b/tools/testing/selftests/powerpc/copyloops/Makefile
index ac4a52e19e59..eedce3366f64 100644
--- a/tools/testing/selftests/powerpc/copyloops/Makefile
+++ b/tools/testing/selftests/powerpc/copyloops/Makefile
@@ -5,8 +5,8 @@ CFLAGS += -I$(CURDIR)
CFLAGS += -D SELFTEST
CFLAGS += -maltivec
-# Use our CFLAGS for the implicit .S rule
-ASFLAGS = $(CFLAGS)
+# Use our CFLAGS for the implicit .S rule & set the asm machine type
+ASFLAGS = $(CFLAGS) -Wa,-mpower4
TEST_GEN_PROGS := copyuser_64 copyuser_power7 memcpy_64 memcpy_power7
EXTRA_SOURCES := validate.c ../harness.c
--
2.14.1
^ permalink raw reply related
* Re: [1/3] powerpc/64s/idle: POWER9 implement a separate idle stop function for hotplug
From: Michael Ellerman @ 2018-04-03 16:03 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Gautham R . Shenoy, Nicholas Piggin
In-Reply-To: <20171117140807.22105-2-npiggin@gmail.com>
On Fri, 2017-11-17 at 14:08:05 UTC, Nicholas Piggin wrote:
> Implement a new function to invoke stop, power9_offline_stop, which is
> like power9_idle_stop but used by the cpu hotplug code.
>
> Move KVM secondary state manipulation code to the offline case.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/3d4fbffdd703d2b968db443911f214
cheers
^ permalink raw reply
* Re: [2/3] powerpc/64s/idle: avoid sync for KVM state when waking from idle
From: Michael Ellerman @ 2018-04-03 16:03 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Gautham R . Shenoy, Nicholas Piggin
In-Reply-To: <20171117140807.22105-3-npiggin@gmail.com>
On Fri, 2017-11-17 at 14:08:06 UTC, Nicholas Piggin wrote:
> When waking from a CPU idle instruction (e.g., nap or stop), the sync
> for ordering the KVM secondary thread state can be avoided if there
> wakeup is coming from a kernel context rather than KVM context.
>
> This improves performance for ping-pong benchmark with the stop0 idle
> state by 0.46% for 2 threads in the same core, and 1.02% for different
> cores.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/8c1c7fb0b5ec95c392e9b585a6cf8c
cheers
^ permalink raw reply
* Re: [1/9] powerpc/64s: cputable add all POWER9 features to CPU_FTRS_ALWAYS
From: Michael Ellerman @ 2018-04-03 16:03 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20180220190832.1872-2-npiggin@gmail.com>
On Tue, 2018-02-20 at 19:08:24 UTC, Nicholas Piggin wrote:
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Series applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/b842bd0f7a61b129a672f8b038325e
cheers
^ permalink raw reply
* Re: powerpc/boot: Remove duplicate typedefs from libfdt_env.h
From: Michael Ellerman @ 2018-04-03 16:03 UTC (permalink / raw)
To: Mark Greer, Benjamin Herrenschmidt, Paul Mackerras
Cc: Oliver O'Halloran, linuxppc-dev, Mark Greer, David Gibson
In-Reply-To: <20180316215443.2807-1-mgreer@animalcreek.com>
On Fri, 2018-03-16 at 21:54:43 UTC, Mark Greer wrote:
> When building a uImage or zImage using ppc6xx_defconfig and some other
> defconfigs, the following error occurs:
>
> BOOTCC arch/powerpc/boot/fdt.o
> In file included from arch/powerpc/boot/fdt.c:51:0:
> ../arch/powerpc/boot/libfdt_env.h:10:13: error: redefinition of typedef 'uint32_t'
> ../arch/powerpc/boot/types.h:21:13: note: previous declaration of 'uint32_t' was here
> ../arch/powerpc/boot/libfdt_env.h:11:13: error: redefinition of typedef 'uint64_t'
> ../arch/powerpc/boot/types.h:22:13: note: previous declaration of 'uint64_t' was here
> ../arch/powerpc/boot/Makefile:210: recipe for target 'arch/powerpc/boot/fdt.o' failed
> make[2]: *** [arch/powerpc/boot/fdt.o] Error 1
>
> The problem is that commit 656ad58ef19e (powerpc/boot: Add OPAL console
> to epapr wrappers) adds typedefs for uint32_t and uint64_t to type.h but
> doesn't remove the pre-existing (and now duplicate) typedefs from
> libfdt_env.h. Fix the error by removing the duplicat typedefs from
> libfdt_env.h
>
> CC: David Gibson <david@gibson.dropbear.id.au>
> CC: Oliver O'Halloran <oohall@gmail.com>
> Signed-off-by: Mark Greer <mgreer@animalcreek.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/147704534e2de30dd47171d55240c3
cheers
^ permalink raw reply
* Re: powerpc/mm/radix: Fix always false comparison against MMU_NO_CONTEXT
From: Michael Ellerman @ 2018-04-03 16:03 UTC (permalink / raw)
To: Mathieu Malaterre; +Cc: Mathieu Malaterre, linuxppc-dev, Aneesh Kumar K . V
In-Reply-To: <20180322210318.21349-1-malat@debian.org>
On Thu, 2018-03-22 at 21:03:18 UTC, Mathieu Malaterre wrote:
> In commit 9690c1574268 ("powerpc/mm/radix: Fix always false comparison
> against MMU_NO_CONTEXT") an issue was discovered where `mm->context.id` was
> being truncated to an `unsigned int`, while the PID is actually an
> `unsigned long`. Update the earlier patch by fixing one remaining
> occurrence. Discovered during a compilation with W=1:
>
> arch/powerpc/mm/tlb-radix.c:702:19: error: comparison is always false due to limited range of data type [-Werror=type-limits]
>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/19e68b2aec3c0a2bd770d3c358a296
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