Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1318237 > unrolled thread

[PATCH] mm: fix pfn_t to page conversion in vm_insert_mixed

Back to article view | Back to linux.kernel

Contents

  [PATCH] mm: fix pfn_t to page conversion in vm_insert_mixed Dan Williams <dan.j.williams@intel.com> - 2016-01-26 19:40 +0100
    Re: [PATCH] mm: fix pfn_t to page conversion in vm_insert_mixed Dan Williams <dan.j.williams@intel.com> - 2016-01-26 21:40 +0100
    Re: [PATCH] mm: fix pfn_t to page conversion in vm_insert_mixed Tomi Valkeinen <tomi.valkeinen@ti.com> - 2016-01-27 08:10 +0100

#1318237 — [PATCH] mm: fix pfn_t to page conversion in vm_insert_mixed

pfn_t_to_page() honors the flags in the pfn_t value to determine if a
pfn is backed by a page.  However, vm_insert_mixed() was originally
written to use pfn_valid() to make this determination.  To restore the
old/correct behavior, ignore the pfn_t flags in the !pfn_t_devmap() case
and fallback to trusting pfn_valid().

Fixes: 01c8f1c44b83 ("mm, dax, gpu: convert vm_insert_mixed to pfn_t")
Cc: Dave Hansen <dave@sr71.net>
Cc: David Airlie <airlied@linux.ie>
Reported-by: Julian Margetson <runaway@candw.ms>
Reported-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 mm/memory.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 30991f83d0bf..93ce37989471 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1591,10 +1591,15 @@ int vm_insert_mixed(struct vm_area_struct *vma, unsigned long addr,
 	 * than insert_pfn).  If a zero_pfn were inserted into a VM_MIXEDMAP
 	 * without pte special, it would there be refcounted as a normal page.
 	 */
-	if (!HAVE_PTE_SPECIAL && pfn_t_valid(pfn)) {
+	if (!HAVE_PTE_SPECIAL && !pfn_t_devmap(pfn) && pfn_t_valid(pfn)) {
 		struct page *page;
 
-		page = pfn_t_to_page(pfn);
+		/*
+		 * At this point we are committed to insert_page()
+		 * regardless of whether the caller specified flags that
+		 * result in pfn_t_has_page() == false.
+		 */
+		page = pfn_to_page(pfn_t_to_pfn(pfn));
 		return insert_page(vma, addr, page, vma->vm_page_prot);
 	}
 	return insert_pfn(vma, addr, pfn, vma->vm_page_prot);

[toc] | [next] | [standalone]

#1318346

On Tue, Jan 26, 2016 at 11:46 AM, Julian Margetson <runaway@candw.ms> wrote:
> On 1/26/2016 2:37 PM, Dan Williams wrote:
>
> pfn_t_to_page() honors the flags in the pfn_t value to determine if a
> pfn is backed by a page.  However, vm_insert_mixed() was originally
> written to use pfn_valid() to make this determination.  To restore the
> old/correct behavior, ignore the pfn_t flags in the !pfn_t_devmap() case
> and fallback to trusting pfn_valid().
>
> Fixes: 01c8f1c44b83 ("mm, dax, gpu: convert vm_insert_mixed to pfn_t")
> Cc: Dave Hansen <dave@sr71.net>
> Cc: David Airlie <airlied@linux.ie>
> Reported-by: Julian Margetson <runaway@candw.ms>
> Reported-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  mm/memory.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 30991f83d0bf..93ce37989471 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1591,10 +1591,15 @@ int vm_insert_mixed(struct vm_area_struct *vma,
> unsigned long addr,
>   * than insert_pfn).  If a zero_pfn were inserted into a VM_MIXEDMAP
>   * without pte special, it would there be refcounted as a normal page.
>   */
> - if (!HAVE_PTE_SPECIAL && pfn_t_valid(pfn)) {
> + if (!HAVE_PTE_SPECIAL && !pfn_t_devmap(pfn) && pfn_t_valid(pfn)) {
>   struct page *page;
>
> - page = pfn_t_to_page(pfn);
> + /*
> + * At this point we are committed to insert_page()
> + * regardless of whether the caller specified flags that
> + * result in pfn_t_has_page() == false.
> + */
> + page = pfn_to_page(pfn_t_to_pfn(pfn));
>   return insert_page(vma, addr, page, vma->vm_page_prot);
>   }
>   return insert_pfn(vma, addr, pfn, vma->vm_page_prot);
>
>
>
> [   16.503323] systemd[1]: Mounting FUSE Control File System...
> [   42.703092] Oops: Machine check, sig: 7 [#1]
> [   42.707624] PREEMPT Canyonlands
> [   42.710959] Modules linked in:
> [   42.714201] CPU: 0 PID: 553 Comm: Xorg Not tainted 4.5.0-rc1-Sam460ex #1
> [   42.721283] task: ee1e45c0 ti: ecd46000 task.ti: ecd46000
> [   42.726983] NIP: 1fed2480 LR: 1fed2404 CTR: 1fed24d0
> [   42.732227] REGS: ecd47f10 TRAP: 0214   Not tainted  (4.5.0-rc1-Sam460ex)
> [   42.739395] MSR: 0002d000 <CE,EE,PR,ME>  CR: 28004262  XER: 00000000
> [   42.746244]
> GPR00: 1f396134 bfcb0970 b77fc6f0 b6fbeffc b67d5008 00000780 00000004
> 00000000
> GPR08: 00000000 b6fbeffc 00000000 bfcb0920 1fed2404 2076dff4 00000000
> 00000780
> GPR16: 00000000 00000020 00000000 00000000 00001e00 209be650 00000438
> b67d5008
> GPR24: 00000780 bfcb09c8 209a8728 b6fbf000 b6fbf000 b67d5008 1ffdaff4
> 00001e00
> [   42.778096] NIP [1fed2480] 0x1fed2480
> [   42.781967] LR [1fed2404] 0x1fed2404
> [   42.785741] Call Trace:
> [   42.943688] ---[ end trace 5d20a91d2d30d9d6 ]---
> [   42.948311]
> [   46.641774] Machine check in kernel mode.
> [   46.645805] Data Write PLB Error
> [   46.649031] Machine Check exception is imprecise
> [   46.653658] Vector: 214  at [eccfbf10]
> [   46.657408]     pc: 1ffa9480
> [   46.660325]     lr: 1ffa9404
> [   46.663241]     sp: bf9252b0
> [   46.666123]    msr: 2d000
> [   46.668746]   current = 0xee1e73c0
> [   46.672149]     pid   = 663, comm = Xorg
> [   46.676074] Linux version 4.5.0-rc1-Sam460ex (root@julian-VirtualBox)

Ok, I think the patch is still needed for the issue Tomi reported,
this appears to be a separate bug.

Can you send me your kernel config?

[toc] | [prev] | [next] | [standalone]

#1318708

[Multipart message — attachments visible in raw view] — view raw

On 26/01/16 20:37, Dan Williams wrote:
> pfn_t_to_page() honors the flags in the pfn_t value to determine if a
> pfn is backed by a page.  However, vm_insert_mixed() was originally
> written to use pfn_valid() to make this determination.  To restore the
> old/correct behavior, ignore the pfn_t flags in the !pfn_t_devmap() case
> and fallback to trusting pfn_valid().
> 
> Fixes: 01c8f1c44b83 ("mm, dax, gpu: convert vm_insert_mixed to pfn_t")

Thanks, this fixes the crash with omapdrm.

Tested-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

 Tomi

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web