Re: [PATCH] staging: ion: make the pte default none PTE_RDONLY

3 messages, 3 authors, 2016-01-18 · open the first message on its own page

Re: [PATCH] staging: ion: make the pte default none PTE_RDONLY

From: Laura Abbott <hidden>
Date: 2016-01-15 23:03:55

(adding linux-arm and a few people)

On 01/14/2016 06:42 PM, Chen Feng wrote:
quoted hunk
The page is already alloc at ion_alloc function,
ion_mmap map the alloced pages to user-space.

The default prot can be PTE_RDONLY. Take a look at
here:
set_pte_at()
arch/arm64/include/asm:
		if (pte_dirty(pte) && pte_write(pte))
			pte_val(pte) &= ~PTE_RDONLY;
		else
			pte_val(pte) |= PTE_RDONLY;

So with the dirty bit,it can improve the efficiency
and donnot need to handle memory fault when use access.

Signed-off-by: Chen Feng <redacted>
Signed-off-by: Wei Dong <redacted>
Reviewed-by: Zhuangluan Su <redacted>
---
  drivers/staging/android/ion/ion.c | 3 +++
  1 file changed, 3 insertions(+)
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index e237e9f..dba5942 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -1026,6 +1026,9 @@ static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
  	if (!(buffer->flags & ION_FLAG_CACHED))
  		vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);

+	/*Default writeable*/
+	vma->vm_page_prot = pte_mkdirty(vma->vm_page_prot);
+
  	mutex_lock(&buffer->lock);
  	/* now map it to userspace */
  	ret = buffer->heap->ops->map_user(buffer->heap, buffer, vma);
The extra fault is unfortunate but I'm skeptical about just setting
pte_mkdirty.

Catalin/Will, do you have any thoughts? Right now it seems like any
range mapped with remap_pfn_range will have this extra fault
behavior. Is marking the range dirty the best solution?

Thanks,
Laura

Re: [PATCH] staging: ion: make the pte default none PTE_RDONLY

From: Russell King - ARM Linux <hidden>
Date: 2016-01-15 23:24:14

On Fri, Jan 15, 2016 at 03:03:42PM -0800, Laura Abbott wrote:
(adding linux-arm and a few people)

On 01/14/2016 06:42 PM, Chen Feng wrote:
quoted
The page is already alloc at ion_alloc function,
ion_mmap map the alloced pages to user-space.

The default prot can be PTE_RDONLY. Take a look at
here:
set_pte_at()
arch/arm64/include/asm:
	if (pte_dirty(pte) && pte_write(pte))
		pte_val(pte) &= ~PTE_RDONLY;
	else
		pte_val(pte) |= PTE_RDONLY;

So with the dirty bit,it can improve the efficiency
and donnot need to handle memory fault when use access.

Signed-off-by: Chen Feng <redacted>
Signed-off-by: Wei Dong <redacted>
Reviewed-by: Zhuangluan Su <redacted>
---
 drivers/staging/android/ion/ion.c | 3 +++
 1 file changed, 3 insertions(+)
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index e237e9f..dba5942 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -1026,6 +1026,9 @@ static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
 	if (!(buffer->flags & ION_FLAG_CACHED))
 		vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);

+	/*Default writeable*/
+	vma->vm_page_prot = pte_mkdirty(vma->vm_page_prot);
+
 	mutex_lock(&buffer->lock);
 	/* now map it to userspace */
 	ret = buffer->heap->ops->map_user(buffer->heap, buffer, vma);
The extra fault is unfortunate but I'm skeptical about just setting
pte_mkdirty.

Catalin/Will, do you have any thoughts? Right now it seems like any
range mapped with remap_pfn_range will have this extra fault
behavior. Is marking the range dirty the best solution?
What happens if the mapping requested was read only - at the very
least, I don't think this should be done unconditionally.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH] staging: ion: make the pte default none PTE_RDONLY

From: chenfeng <hidden>
Date: 2016-01-18 03:45:38


On 2016/1/16 7:23, Russell King - ARM Linux wrote:
On Fri, Jan 15, 2016 at 03:03:42PM -0800, Laura Abbott wrote:
quoted
(adding linux-arm and a few people)

On 01/14/2016 06:42 PM, Chen Feng wrote:
quoted
The page is already alloc at ion_alloc function,
ion_mmap map the alloced pages to user-space.

The default prot can be PTE_RDONLY. Take a look at
here:
set_pte_at()
arch/arm64/include/asm:
		if (pte_dirty(pte) && pte_write(pte))
			pte_val(pte) &= ~PTE_RDONLY;
		else
			pte_val(pte) |= PTE_RDONLY;

So with the dirty bit,it can improve the efficiency
and donnot need to handle memory fault when use access.

Signed-off-by: Chen Feng <redacted>
Signed-off-by: Wei Dong <redacted>
Reviewed-by: Zhuangluan Su <redacted>
---
 drivers/staging/android/ion/ion.c | 3 +++
 1 file changed, 3 insertions(+)
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index e237e9f..dba5942 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -1026,6 +1026,9 @@ static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
 	if (!(buffer->flags & ION_FLAG_CACHED))
 		vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);

+	/*Default writeable*/
+	vma->vm_page_prot = pte_mkdirty(vma->vm_page_prot);
+
 	mutex_lock(&buffer->lock);
 	/* now map it to userspace */
 	ret = buffer->heap->ops->map_user(buffer->heap, buffer, vma);
The extra fault is unfortunate but I'm skeptical about just setting
pte_mkdirty.

Catalin/Will, do you have any thoughts? Right now it seems like any
range mapped with remap_pfn_range will have this extra fault
behavior. Is marking the range dirty the best solution?
Laura Abbott,

I agree with you, it seems all the remap_pfn_range have this fault behavior.

What happens if the mapping requested was read only - at the very
least, I don't think this should be done unconditionally.
Russell,
I am not sure about doing this unconditionally, but it can waste memory&time
while handling page fault with ion alloced page.

And the page can be used directly.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help