From mboxrd@z Thu Jan 1 00:00:00 1970 From: puck.chen@hisilicon.com (chenfeng) Date: Mon, 18 Jan 2016 11:44:54 +0800 Subject: [PATCH] staging: ion: make the pte default none PTE_RDONLY In-Reply-To: <20160115232334.GD19062@n2100.arm.linux.org.uk> References: <1452825753-36364-1-git-send-email-puck.chen@hisilicon.com> <56997ACE.80604@redhat.com> <20160115232334.GD19062@n2100.arm.linux.org.uk> Message-ID: <569C5FB6.9000702@hisilicon.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 2016/1/16 7:23, Russell King - ARM Linux wrote: > 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: >>> 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 >>> Signed-off-by: Wei Dong >>> Reviewed-by: Zhuangluan Su >>> --- >>> 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. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753098AbcARDpi (ORCPT ); Sun, 17 Jan 2016 22:45:38 -0500 Received: from szxga03-in.huawei.com ([119.145.14.66]:24568 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751281AbcARDpg (ORCPT ); Sun, 17 Jan 2016 22:45:36 -0500 Subject: Re: [PATCH] staging: ion: make the pte default none PTE_RDONLY To: Russell King - ARM Linux , Laura Abbott References: <1452825753-36364-1-git-send-email-puck.chen@hisilicon.com> <56997ACE.80604@redhat.com> <20160115232334.GD19062@n2100.arm.linux.org.uk> CC: , , , , , , , , , , , , , , , , , , , , , , , , Catalin Marinas , Will Deacon , , , , , , linux-arm-kernel From: chenfeng Message-ID: <569C5FB6.9000702@hisilicon.com> Date: Mon, 18 Jan 2016 11:44:54 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <20160115232334.GD19062@n2100.arm.linux.org.uk> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.142.192.172] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090204.569C5FCB.004A,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-05-26 15:14:31, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 936c8757ff88865376aa17e893272f7a Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016/1/16 7:23, Russell King - ARM Linux wrote: > 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: >>> 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 >>> Signed-off-by: Wei Dong >>> Reviewed-by: Zhuangluan Su >>> --- >>> 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.