Skip to contentSkip to conversation

videobuf-dma-contig: zero copy USERPTR support V2

PATCH In review Magnus Damm 4 participants 15 messages Apr 28, 2009 – May 8, 2009

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 videobuf-dma-contig: zero copy USERPTR support V2

From: Magnus Damm <[email protected]>

This is V2 of the V4L2 videobuf-dma-contig USERPTR zero copy patch.

Since videobuf-dma-contig is designed to handle physically contiguous memory, this patch modifies the videobuf-dma-contig code to only accept a pointer to physically contiguous memory. For now only VM_PFNMAP vmas are supported, so forget hotplug.

On SuperH Mobile we use this approach for our sh_mobile_ceu_camera driver together with various multimedia accelerator blocks that are exported to user space using UIO. The UIO kernel code exports physically contiugous memory to user space and lets the user space application mmap() this memory and pass a pointer using the USERPTR interface for V4L2 zero copy operation.

With this approach we support zero copy capture, hardware scaling and various forms of hardware encoding and decoding.

---

Many thanks to Hannes for the feedback! Tested on SH7722 Migo-R with a hacked up capture.c

Changes since V1:
- minor cleanups and formatting changes
- use follow_phys() in videobuf-dma-contig instead of duplicating code
- since videobuf-dma-contig can be a module: EXPORT_SYMBOL(follow_phys)
- move CONFIG_HAVE_IOREMAP_PROT to always build follow_phys()

 drivers/media/video/videobuf-dma-contig.c |   82 +++++++++++++++++++++++++++--
 mm/memory.c                               |    3 -
 2 files changed, 79 insertions(+), 6 deletions(-)
drivers/media/video/videobuf-dma-contig.c +77 -5
@@ -17,6 +17,7 @@
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/mm.h>
+#include <linux/pagemap.h>
 #include <linux/dma-mapping.h>
 #include <media/videobuf-dma-contig.h>
 
@@ -25,6 +26,7 @@ struct videobuf_dma_contig_memory {
 	void *vaddr;
 	dma_addr_t dma_handle;
 	unsigned long size;
+	int is_userptr;
 };
 
 #define MAGIC_DC_MEM 0x0733ac61
@@ -108,6 +110,70 @@ static struct vm_operations_struct video
 	.close    = videobuf_vm_close,
 };
 
+static void videobuf_dma_contig_user_put(struct videobuf_dma_contig_memory *mem)
+{
+	mem->is_userptr = 0;
+	mem->dma_handle = 0;
+	mem->size = 0;
+}
+
+static int videobuf_dma_contig_user_get(struct videobuf_dma_contig_memory *mem,
+					struct videobuf_buffer *vb)
+{
+	struct mm_struct *mm = current->mm;
+	struct vm_area_struct *vma;
+	unsigned long prev_pfn, this_pfn;
+	unsigned long pages_done, user_address;
+	unsigned long prot;
+	resource_size_t phys;
+	int ret;
+
+	mem->size = PAGE_ALIGN(vb->size);
+	mem->is_userptr = 0;
+	ret = -EINVAL;
+
+	down_read(&mm->mmap_sem);
+
+	vma = find_vma(mm, vb->baddr);
+	if (!vma)
+		goto out_up;
+
+	if ((vb->baddr + mem->size) > vma->vm_end)
+		goto out_up;
+
+	pages_done = 0;
+	prev_pfn = 0; /* kill warning */
+	user_address = vb->baddr;
+
+	while (pages_done < (mem->size >> PAGE_SHIFT)) {
+		ret = follow_phys(vma, user_address, 0, &prot, &phys);
+		if (ret)
+			break;
+
+		this_pfn = phys >> PAGE_SHIFT;
+
+		if (pages_done == 0)
+			mem->dma_handle = phys;
+		else if (this_pfn != (prev_pfn + 1))
+			ret = -EFAULT;
+
+		if (ret)
+			break;
+
+		prev_pfn = this_pfn;
+		user_address += PAGE_SIZE;
+		pages_done++;
+	}
+
+	if (!ret)
+		mem->is_userptr = 1;
+
+ out_up:
+	up_read(&current->mm->mmap_sem);
+
+	return ret;
+}
+
 static void *__videobuf_alloc(size_t size)
 {
 	struct videobuf_dma_contig_memory *mem;
@@ -154,12 +220,11 @@ static int __videobuf_iolock(struct vide
 	case V4L2_MEMORY_USERPTR:
 		dev_dbg(q->dev, "%s memory method USERPTR\n", __func__);
 
-		/* The only USERPTR currently supported is the one needed for
-		   read() method.
-		 */
+		/* handle pointer from user space */
 		if (vb->baddr)
-			return -EINVAL;
+			return videobuf_dma_contig_user_get(mem, vb);
 
+		/* allocate memory for the read() method */
 		mem->size = PAGE_ALIGN(vb->size);
 		mem->vaddr = dma_alloc_coherent(q->dev, mem->size,
 						&mem->dma_handle, GFP_KERNEL);
@@ -386,7 +451,7 @@ void videobuf_dma_contig_free(struct vid
 	   So, it should free memory only if the memory were allocated for
 	   read() operation.
 	 */
-	if ((buf->memory != V4L2_MEMORY_USERPTR) || buf->baddr)
+	if (buf->memory != V4L2_MEMORY_USERPTR)
 		return;
 
 	if (!mem)
@@ -394,6 +459,13 @@ void videobuf_dma_contig_free(struct vid
 
 	MAGIC_CHECK(mem->magic, MAGIC_DC_MEM);
 
+	/* handle user space pointer case */
+	if (buf->baddr) {
+		videobuf_dma_contig_user_put(mem);
+		return;
+	}
+
+	/* read() method */
 	dma_free_coherent(q->dev, mem->size, mem->vaddr, mem->dma_handle);
 	mem->vaddr = NULL;
 }
mm/memory.c +2 -1
@@ -3009,7 +3009,6 @@ int in_gate_area_no_task(unsigned long a
 
 #endif	/* __HAVE_ARCH_GATE_AREA */
 
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 int follow_phys(struct vm_area_struct *vma,
 		unsigned long address, unsigned int flags,
 		unsigned long *prot, resource_size_t *phys)
@@ -3063,7 +3062,9 @@ unlock:
 out:
 	return ret;
 }
+EXPORT_SYMBOL(follow_phys);
 
+#ifdef CONFIG_HAVE_IOREMAP_PROT
 int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
 			void *buf, int len, int write)
 {

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#1 lore raw
Quoted 1 line · Magnus Damm

This is V2 of the V4L2 videobuf-dma-contig USERPTR zero copy patch.

I guess the V4L2 specific bits are pretty simple.

As for the minor mm modifications below,

mm/memory.c    2009-04-28 14:56:43.000000000 +0900 int in_gate_area_no_task(unsigned long a · line 3014

Quoted 10 lines
mm/memory.c    2009-04-28 14:56:43.000000000 +0900 renamed from mm/memory.c +0 -1
@@ -3009,7 +3009,6 @@ int in_gate_area_no_task(unsigned long a
 
  #endif /* __HAVE_ARCH_GATE_AREA */
 
-#ifdef CONFIG_HAVE_IOREMAP_PROT
  int follow_phys(struct vm_area_struct *vma,
                unsigned long address, unsigned int flags,
                unsigned long *prot, resource_size_t *phys)

Is it ok with the memory management guys to always build follow_phys()?

Quoted 10 lines
mm/memory.c    2009-04-28 14:56:43.000000000 +0900 renamed from mm/memory.c +0 -0
@@ -3063,7 +3062,9 @@ unlock:

 out:
       return ret;
 }

mm/memory.c    2009-04-28 14:56:43.000000000 +0900 renamed from mm/memory.c +2 -0
+EXPORT_SYMBOL(follow_phys);
 
+#ifdef CONFIG_HAVE_IOREMAP_PROT

 int generic_access_phys(struct vm_area_struct *vma, unsigned long addr, void *buf, int len, int write)
 {

How about exporting follow_phys()? This because the user videobuf-dma-contig.c can be built as a module.

Should I use EXPORT_SYMBOL_GPL() instead of EXPORT_SYMBOL()?

Any comments?

Thanks,

/ magnus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#2 lore raw
Quoted 11 lines · Magnus Damm
Quoted 1 line · Magnus Damm

This is V2 of the V4L2 videobuf-dma-contig USERPTR zero copy patch.

I guess the V4L2 specific bits are pretty simple.

As for the minor mm modifications below,

Quoted 4 lines
mm/memory.c 2009-04-28 14:56:43.000000000 +0900 renamed from mm/memory.c +0 -0
@@ -3009,7 +3009,6 @@ int in_gate_area_no_task(unsigned long a
 
Quoted 8 lines

mm/memory.c 2009-04-28 14:56:43.000000000 +0900 int in_gate_area_no_task(unsigned long a · line 3014

Quoted 6 lines
mm/memory.c 2009-04-28 14:56:43.000000000 +0900 renamed from mm/memory.c +0 -1
 #endif /* __HAVE_ARCH_GATE_AREA */
 
-#ifdef CONFIG_HAVE_IOREMAP_PROT
 int follow_phys(struct vm_area_struct *vma,
               unsigned long address, unsigned int flags,
               unsigned long *prot, resource_size_t *phys)

Is it ok with the memory management guys to always build follow_phys()?

AFAICS, pte_pgprot is only defined on three architectures that have the config symbol above set. It shouldn't compile on the others.

I have a patch that factors out follow_pte and builds follow_pfn and follow_phys on top of that. I can send it monday, no access to it from here right now.

Then we can keep follow_phys private to this configuration.

Hannes

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#3 lore raw

PATCH 1/3 mm: introduce follow_pte()

A generic readonly page table lookup helper to map an address space and an address from it to a pte.

---
 mm/memory.c |   37 +++++++++++++++++++++++++++++++++++++
 1 files changed, 37 insertions(+), 0 deletions(-)
mm/memory.c +37 -0
index cf6873e..a621319 100644
@@ -3009,6 +3009,43 @@ int in_gate_area_no_task(unsigned long addr)
 
 #endif	/* __HAVE_ARCH_GATE_AREA */
 
+static int follow_pte(struct mm_struct *mm, unsigned long address,
+		pte_t **ptepp, spinlock_t **ptlp)
+{
+	pgd_t *pgd;
+	pud_t *pud;
+	pmd_t *pmd;
+	pte_t *ptep;
+
+	pgd = pgd_offset(mm, address);
+	if (pgd_none(*pgd) || unlikely(pgd_bad(*pgd)))
+		goto out;
+
+	pud = pud_offset(pgd, address);
+	if (pud_none(*pud) || unlikely(pud_bad(*pud)))
+		goto out;
+
+	pmd = pmd_offset(pud, address);
+	if (pmd_none(*pmd) || unlikely(pmd_bad(*pmd)))
+		goto out;
+
+	/* We cannot handle huge page PFN maps. Luckily they don't exist. */
+	if (pmd_huge(*pmd))
+		goto out;
+
+	ptep = pte_offset_map_lock(mm, pmd, address, ptlp);
+	if (!ptep)
+		goto out;
+	if (!pte_present(*ptep))
+		goto unlock;
+	*ptepp = ptep;
+	return 0;
+unlock:
+	pte_unmap_unlock(ptep, *ptlp);
+out:
+	return -EINVAL;
+}
+
 #ifdef CONFIG_HAVE_IOREMAP_PROT
 int follow_phys(struct vm_area_struct *vma,
 		unsigned long address, unsigned int flags,
Signature · 7 lines
1.6.2.1.135.gde769

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
maintainer
#4 lore raw

Re: [patch 1/3] mm: introduce follow_pte()

Quoted 2 lines · On Mon, 4 May 2009 11:54:32 +0200 Johannes Weiner <[email protected]> wrote:

A generic readonly page table lookup helper to map an address space and an address from it to a pte.

umm, OK.

Is there actually some point to these three patches? If so, what is it?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#5 lore raw

Re: [patch 1/3] mm: introduce follow_pte()

Quoted 9 lines · Andrew Morton
Quoted 2 lines · On Mon, 4 May 2009 11:54:32 +0200 Johannes Weiner <[email protected]> wrote:

A generic readonly page table lookup helper to map an address space and an address from it to a pte.

umm, OK.

Is there actually some point to these three patches? If so, what is it?

Magnus needs to check for physical contiguity of a VMAs backing pages to support zero-copy exportation of video data to userspace.

This series implements follow_pfn() so he can walk the VMA backing pages and ensure their PFNs are in linear order.

[ This patch can be collapsed with 2/3, I just thought it would be
easier to read the diffs when having them separate. ]

1/3 and 2/3: factor out the page table walk from follow_phys() into follow_pte().

3/3: implement follow_pfn() on top of follow_pte().

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

maintainer
#6 lore raw

Re: [patch 1/3] mm: introduce follow_pte()

Quoted 17 lines · On Tue, 5 May 2009 22:38:07 +0200 Johannes Weiner <[email protected]> wrote:
Quoted 9 lines · Andrew Morton
Quoted 2 lines · On Mon, 4 May 2009 11:54:32 +0200 Johannes Weiner <[email protected]> wrote:

A generic readonly page table lookup helper to map an address space and an address from it to a pte.

umm, OK.

Is there actually some point to these three patches? If so, what is it?

Magnus needs to check for physical contiguity of a VMAs backing pages to support zero-copy exportation of video data to userspace.

This series implements follow_pfn() so he can walk the VMA backing pages and ensure their PFNs are in linear order.

Quoted 7 lines

[ This patch can be collapsed with 2/3, I just thought it would be
easier to read the diffs when having them separate. ]

1/3 and 2/3: factor out the page table walk from follow_phys() into follow_pte().

3/3: implement follow_pfn() on top of follow_pte().

So we could bundle these patches with Magnus's patchset, or we could consider these three patches as a cleanup or something.

Given that 3/3 introduces an unused function, I'm inclined to sit tight and await Magnus's work.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#7 lore raw

Re: [patch 1/3] mm: introduce follow_pte()

Quoted 26 lines · Andrew Morton
Quoted 23 lines · On Tue, 5 May 2009 22:38:07 +0200 Johannes Weiner <[email protected]> wrote:
Quoted 9 lines · Andrew Morton
Quoted 2 lines · On Mon, 4 May 2009 11:54:32 +0200 Johannes Weiner <[email protected]> wrote:

A generic readonly page table lookup helper to map an address space and an address from it to a pte.

umm, OK.

Is there actually some point to these three patches? If so, what is it?

Magnus needs to check for physical contiguity of a VMAs backing pages to support zero-copy exportation of video data to userspace.

This series implements follow_pfn() so he can walk the VMA backing pages and ensure their PFNs are in linear order.

[ This patch can be collapsed with 2/3, I just thought it would be
easier to read the diffs when having them separate. ]

1/3 and 2/3: factor out the page table walk from follow_phys() into follow_pte().

Quoted 7 lines
Quoted 1 line

3/3: implement follow_pfn() on top of follow_pte().

So we could bundle these patches with Magnus's patchset, or we could consider these three patches as a cleanup or something.

Given that 3/3 introduces an unused function, I'm inclined to sit tight and await Magnus's work.

Yeah, I didn't see the video guys responding on Magnus' patch yet, so let's wait for them.

Magnus, the actual conversion of your code should be trivial, could you respin it on top of these three patches using follow_pfn() then?

Thanks, Hannes

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author Acked-by
#8 lore raw

Re: [patch 1/3] mm: introduce follow_pte()

Quoted 31 lines · Johannes Weiner
Quoted 30 lines · Andrew Morton
Quoted 17 lines · On Tue, 5 May 2009 22:38:07 +0200 Johannes Weiner <[email protected]> wrote:
Quoted 9 lines · Andrew Morton
Quoted 2 lines · On Mon,  4 May 2009 11:54:32 +0200 Johannes Weiner <[email protected]> wrote:

A generic readonly page table lookup helper to map an address space and an address from it to a pte.

umm, OK.

Is there actually some point to these three patches?  If so, what is it?

Magnus needs to check for physical contiguity of a VMAs backing pages to support zero-copy exportation of video data to userspace.

This series implements follow_pfn() so he can walk the VMA backing pages and ensure their PFNs are in linear order.

Quoted 7 lines

[ This patch can be collapsed with 2/3, I just thought it would be easier to read the diffs when having them separate. ]

1/3 and 2/3: factor out the page table walk from follow_phys() into follow_pte().

3/3: implement follow_pfn() on top of follow_pte().

So we could bundle these patches with Magnus's patchset, or we could consider these three patches as a cleanup or something.

Quoted 8 lines
Quoted 2 lines

Given that 3/3 introduces an unused function, I'm inclined to sit tight and await Magnus's work.

Yeah, I didn't see the video guys responding on Magnus' patch yet, so let's wait for them.

Magnus, the actual conversion of your code should be trivial, could you respin it on top of these three patches using follow_pfn() then?

So I tested the patches in -mm (1/3, 2/3, 3/3) together with the zero copy patch and everything seems fine. Feel free to add acks from me, least for patch 1/3 and 3/3 - i know too little about the generic case to say anything about 2/3.

I'll send V3 of my zero copy patch in a little while. Thanks a lot for the help!

Cheers,

/ magnus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#9 lore raw

PATCH 2/3 mm: use generic follow_pte() in follow_phys()

In reply to Johannes Weiner
---
 mm/memory.c |   37 ++++++-------------------------------
 1 files changed, 6 insertions(+), 31 deletions(-)
mm/memory.c +6 -31
index a621319..aee167d 100644
@@ -3051,50 +3051,25 @@ int follow_phys(struct vm_area_struct *vma,
 		unsigned long address, unsigned int flags,
 		unsigned long *prot, resource_size_t *phys)
 {
-	pgd_t *pgd;
-	pud_t *pud;
-	pmd_t *pmd;
+	int ret = -EINVAL;
 	pte_t *ptep, pte;
 	spinlock_t *ptl;
-	resource_size_t phys_addr = 0;
-	struct mm_struct *mm = vma->vm_mm;
-	int ret = -EINVAL;
 
 	if (!(vma->vm_flags & (VM_IO | VM_PFNMAP)))
 		goto out;
 
-	pgd = pgd_offset(mm, address);
-	if (pgd_none(*pgd) || unlikely(pgd_bad(*pgd)))
-		goto out;
-
-	pud = pud_offset(pgd, address);
-	if (pud_none(*pud) || unlikely(pud_bad(*pud)))
-		goto out;
-
-	pmd = pmd_offset(pud, address);
-	if (pmd_none(*pmd) || unlikely(pmd_bad(*pmd)))
-		goto out;
-
-	/* We cannot handle huge page PFN maps. Luckily they don't exist. */
-	if (pmd_huge(*pmd))
+	if (follow_pte(vma->vm_mm, address, &ptep, &ptl))
 		goto out;
-
-	ptep = pte_offset_map_lock(mm, pmd, address, &ptl);
-	if (!ptep)
-		goto out;
-
 	pte = *ptep;
-	if (!pte_present(pte))
-		goto unlock;
+
 	if ((flags & FOLL_WRITE) && !pte_write(pte))
 		goto unlock;
-	phys_addr = pte_pfn(pte);
-	phys_addr <<= PAGE_SHIFT; /* Shift here to avoid overflow on PAE */
 
 	*prot = pgprot_val(pte_pgprot(pte));
-	*phys = phys_addr;
-	ret = 0;
+	/* Shift here to avoid overflow on PAE */
+	*phys = pte_pfn(pte) << PAGE_SHIFT;
 
+	ret = 0;
 unlock:
 	pte_unmap_unlock(ptep, ptl);
 out:
Signature · 7 lines
1.6.2.1.135.gde769

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#10 lore raw

Re: [patch 2/3] mm: use generic follow_pte() in follow_phys()

Quoted 51 lines · Johannes Weiner
---
 mm/memory.c |   37 ++++++-------------------------------
 1 files changed, 6 insertions(+), 31 deletions(-)
mm/memory.c +3 -27
index a621319..aee167d 100644
@@ -3051,50 +3051,25 @@ int follow_phys(struct vm_area_struct *vma,
 		unsigned long address, unsigned int flags,
 		unsigned long *prot, resource_size_t *phys)
 {
-	pgd_t *pgd;
-	pud_t *pud;
-	pmd_t *pmd;
+	int ret = -EINVAL;
 	pte_t *ptep, pte;
 	spinlock_t *ptl;
-	resource_size_t phys_addr = 0;
-	struct mm_struct *mm = vma->vm_mm;
-	int ret = -EINVAL;
 
 	if (!(vma->vm_flags & (VM_IO | VM_PFNMAP)))
 		goto out;
 
-	pgd = pgd_offset(mm, address);
-	if (pgd_none(*pgd) || unlikely(pgd_bad(*pgd)))
-		goto out;
-
-	pud = pud_offset(pgd, address);
-	if (pud_none(*pud) || unlikely(pud_bad(*pud)))
-		goto out;
-
-	pmd = pmd_offset(pud, address);
-	if (pmd_none(*pmd) || unlikely(pmd_bad(*pmd)))
-		goto out;
-
-	/* We cannot handle huge page PFN maps. Luckily they don't exist. */
-	if (pmd_huge(*pmd))
+	if (follow_pte(vma->vm_mm, address, &ptep, &ptl))
 		goto out;
-
-	ptep = pte_offset_map_lock(mm, pmd, address, &ptl);
-	if (!ptep)
-		goto out;
-
 	pte = *ptep;
-	if (!pte_present(pte))
-		goto unlock;
+

mm/memory.c int follow_phys(struct vm_area_struct *vma, · line 3070

Quoted 10 lines
mm/memory.c +2 -4
 	if ((flags & FOLL_WRITE) && !pte_write(pte))
 		goto unlock;
-	phys_addr = pte_pfn(pte);
-	phys_addr <<= PAGE_SHIFT; /* Shift here to avoid overflow on PAE */
 
 	*prot = pgprot_val(pte_pgprot(pte));
-	*phys = phys_addr;
-	ret = 0;
+	/* Shift here to avoid overflow on PAE */
+	*phys = pte_pfn(pte) << PAGE_SHIFT;

Yuk, had my head in the butt here. Changed to

	*phys = (resource_size_t)pte_pfn(pte) << PAGE_SHIFT;

---
From: Johannes Weiner <[email protected]>
Subject: mm: use generic follow_pte() in follow_phys()

---
 mm/memory.c |   36 +++++-------------------------------
 1 files changed, 5 insertions(+), 31 deletions(-)
mm/memory.c +5 -31
index a621319..c047950 100644
@@ -3051,50 +3051,24 @@ int follow_phys(struct vm_area_struct *vma,
 		unsigned long address, unsigned int flags,
 		unsigned long *prot, resource_size_t *phys)
 {
-	pgd_t *pgd;
-	pud_t *pud;
-	pmd_t *pmd;
+	int ret = -EINVAL;
 	pte_t *ptep, pte;
 	spinlock_t *ptl;
-	resource_size_t phys_addr = 0;
-	struct mm_struct *mm = vma->vm_mm;
-	int ret = -EINVAL;
 
 	if (!(vma->vm_flags & (VM_IO | VM_PFNMAP)))
 		goto out;
 
-	pgd = pgd_offset(mm, address);
-	if (pgd_none(*pgd) || unlikely(pgd_bad(*pgd)))
-		goto out;
-
-	pud = pud_offset(pgd, address);
-	if (pud_none(*pud) || unlikely(pud_bad(*pud)))
-		goto out;
-
-	pmd = pmd_offset(pud, address);
-	if (pmd_none(*pmd) || unlikely(pmd_bad(*pmd)))
-		goto out;
-
-	/* We cannot handle huge page PFN maps. Luckily they don't exist. */
-	if (pmd_huge(*pmd))
+	if (follow_pte(vma->vm_mm, address, &ptep, &ptl))
 		goto out;
-
-	ptep = pte_offset_map_lock(mm, pmd, address, &ptl);
-	if (!ptep)
-		goto out;
-
 	pte = *ptep;
-	if (!pte_present(pte))
-		goto unlock;
+
 	if ((flags & FOLL_WRITE) && !pte_write(pte))
 		goto unlock;
-	phys_addr = pte_pfn(pte);
-	phys_addr <<= PAGE_SHIFT; /* Shift here to avoid overflow on PAE */
 
 	*prot = pgprot_val(pte_pgprot(pte));
-	*phys = phys_addr;
-	ret = 0;
+	*phys = (resource_size_t)pte_pfn(pte) << PAGE_SHIFT;
 
+	ret = 0;
 unlock:
 	pte_unmap_unlock(ptep, ptl);
 out:
Signature · 7 lines
1.6.2.1.135.gde769

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#11 lore raw

PATCH 3/3 mm: introduce follow_pfn()

In reply to Johannes Weiner

Analoguous to follow_phys(), add a helper that looks up the PFN instead. It also only allows IO mappings or PFN mappings.

---
 include/linux/mm.h |    2 ++
 mm/memory.c        |   19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 0 deletions(-)
include/linux/mm.h +2 -0
index bff1f0d..1cca8b6 100644
@@ -794,6 +794,8 @@ int copy_page_range(struct mm_struct *dst, struct mm_struct *src,
 			struct vm_area_struct *vma);
 void unmap_mapping_range(struct address_space *mapping,
 		loff_t const holebegin, loff_t const holelen, int even_cows);
+int follow_pfn(struct vm_area_struct *vma, unsigned long address,
+	unsigned long *pfn);
 int follow_phys(struct vm_area_struct *vma, unsigned long address,
 		unsigned int flags, unsigned long *prot, resource_size_t *phys);
 int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
mm/memory.c +19 -0
index aee167d..05fc8e5 100644
@@ -3046,6 +3046,25 @@ out:
 	return -EINVAL;
 }
 
+int follow_pfn(struct vm_area_struct *vma, unsigned long address,
+	unsigned long *pfn)
+{
+	int ret = -EINVAL;
+	spinlock_t *ptl;
+	pte_t *ptep;
+
+	if (!(vma->vm_flags & (VM_IO | VM_PFNMAP)))
+		return ret;
+
+	ret = follow_pte(vma->vm_mm, address, &ptep, &ptl);
+	if (ret)
+		return ret;
+	*pfn = pte_pfn(*ptep);
+	pte_unmap_unlock(ptep, ptl);
+	return 0;
+}
+EXPORT_SYMBOL(follow_pfn);
+
 #ifdef CONFIG_HAVE_IOREMAP_PROT
 int follow_phys(struct vm_area_struct *vma,
 		unsigned long address, unsigned int flags,
Signature · 7 lines
1.6.2.1.135.gde769

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#12 lore raw

Re: [patch 3/3] mm: introduce follow_pfn()

Quoted 2 lines · Johannes Weiner

Analoguous to follow_phys(), add a helper that looks up the PFN instead. It also only allows IO mappings or PFN mappings.

A kerneldoc describing what it does and the limitations would be extremly helpful.

#13 lore raw

PATCH v2 3/3 mm: introduce follow_pfn()

Analoguous to follow_phys(), add a helper that looks up the PFN at a user virtual address in an IO mapping or a raw PFN mapping.

---
 include/linux/mm.h |    2 ++
 mm/memory.c        |   29 +++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 0 deletions(-)
Quoted 6 lines · Christoph Hellwig
Quoted 2 lines · Johannes Weiner

Analoguous to follow_phys(), add a helper that looks up the PFN instead. It also only allows IO mappings or PFN mappings.

A kerneldoc describing what it does and the limitations would be extremly helpful.

Agreed. How is this?

include/linux/mm.h +2 -0
index bff1f0d..1cca8b6 100644
@@ -794,6 +794,8 @@ int copy_page_range(struct mm_struct *dst, struct mm_struct *src,
 			struct vm_area_struct *vma);
 void unmap_mapping_range(struct address_space *mapping,
 		loff_t const holebegin, loff_t const holelen, int even_cows);
+int follow_pfn(struct vm_area_struct *vma, unsigned long address,
+	unsigned long *pfn);
 int follow_phys(struct vm_area_struct *vma, unsigned long address,
 		unsigned int flags, unsigned long *prot, resource_size_t *phys);
 int generic_access_phys(struct vm_area_struct *vma, unsigned long addr,
mm/memory.c +29 -0
index c047950..f86aee1 100644
@@ -3046,6 +3046,35 @@ out:
 	return -EINVAL;
 }
 
+/**
+ * follow_pfn - look up PFN at a user virtual address
+ * @vma: memory mapping
+ * @address: user virtual address
+ * @pfn: location to store found PFN
+ *
+ * Only IO mappings and raw PFN mappings are allowed.
+ *
+ * Returns zero and the pfn at @pfn on success, -ve otherwise.
+ */
+int follow_pfn(struct vm_area_struct *vma, unsigned long address,
+	unsigned long *pfn)
+{
+	int ret = -EINVAL;
+	spinlock_t *ptl;
+	pte_t *ptep;
+
+	if (!(vma->vm_flags & (VM_IO | VM_PFNMAP)))
+		return ret;
+
+	ret = follow_pte(vma->vm_mm, address, &ptep, &ptl);
+	if (ret)
+		return ret;
+	*pfn = pte_pfn(*ptep);
+	pte_unmap_unlock(ptep, ptl);
+	return 0;
+}
+EXPORT_SYMBOL(follow_pfn);
+
 #ifdef CONFIG_HAVE_IOREMAP_PROT
 int follow_phys(struct vm_area_struct *vma,
 		unsigned long address, unsigned int flags,
Signature · 7 lines
1.6.2.1.135.gde769

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>