From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boaz Harrosh Subject: Re: [PATCH 1/3 v6] mm(v4.1): New pfn_mkwrite same as page_mkwrite for VM_PFNMAP Date: Tue, 07 Apr 2015 16:37:07 +0300 Message-ID: <5523DD83.4050609@plexistor.com> References: <55239645.9000507@plexistor.com> <552397E6.5030506@plexistor.com> <5523D43C.1060708@plexistor.com> <20150407131700.GA13946@node.dhcp.inet.fi> <20150407132601.GA14252@node.dhcp.inet.fi> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Dave Chinner , Matthew Wilcox , Andrew Morton , "Kirill A. Shutemov" , Jan Kara , Hugh Dickins , Mel Gorman , linux-mm@kvack.org, linux-nvdimm , linux-fsdevel , Eryu Guan , Christoph Hellwig , Stable Tree To: "Kirill A. Shutemov" Return-path: Received: from mail-wi0-f174.google.com ([209.85.212.174]:37426 "EHLO mail-wi0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753591AbbDGNhL (ORCPT ); Tue, 7 Apr 2015 09:37:11 -0400 Received: by wiaa2 with SMTP id a2so19072251wia.0 for ; Tue, 07 Apr 2015 06:37:10 -0700 (PDT) In-Reply-To: <20150407132601.GA14252@node.dhcp.inet.fi> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 04/07/2015 04:26 PM, Kirill A. Shutemov wrote: > On Tue, Apr 07, 2015 at 04:17:00PM +0300, Kirill A. Shutemov wrote: >> On Tue, Apr 07, 2015 at 03:57:32PM +0300, Boaz Harrosh wrote: >>> +/* >>> + * Handle write page faults for VM_MIXEDMAP or VM_PFNMAP for a VM_SHARED >>> + * mapping >>> + */ >>> +static int wp_pfn_shared(struct mm_struct *mm, >>> + struct vm_area_struct *vma, unsigned long address, >>> + pte_t *page_table, spinlock_t *ptl, pte_t orig_pte, >>> + pmd_t *pmd) >>> +{ >>> + if (vma->vm_ops && vma->vm_ops->pfn_mkwrite) { >>> + struct vm_fault vmf = { >>> + .page = NULL, >>> + .pgoff = linear_page_index(vma, address), >>> + .virtual_address = (void __user *)(address & PAGE_MASK), >>> + .flags = FAULT_FLAG_WRITE | FAULT_FLAG_MKWRITE, >>> + }; >>> + int ret; >>> + >>> + pte_unmap_unlock(page_table, ptl); >>> + ret = vma->vm_ops->pfn_mkwrite(vma, &vmf); >>> + if (ret & VM_FAULT_ERROR) >>> + return ret; >>> + page_table = pte_offset_map_lock(mm, pmd, address, &ptl); >>> + /* Did pfn_mkwrite already fixed up the pte */ > > Oh. I guess you've missunderstood why we need pte_same() check below. > It's not about ->pfn_mkwrite() changing the pte (generatlly, it should > not). It's requited to address race with parallel page fault to the pte. > >>> + if (!pte_same(*page_table, orig_pte)) { >>> + pte_unmap_unlock(page_table, ptl); >>> + return ret; >> >> This should be "return 0;", shouldn't it? >> >> VM_FAULT_NOPAGE would imply you've installed new pte, but you did not. Changing this to "return 0" would be very scary for me. Because I'm running with this code for 1/2 a year now. And it is stable. You see since the original code it was always doing just that pte_unmap_unlock && return ret. (See the patch based on 4.0) I did not understand if you want that I keep it "return ret". I gather that you would like the comment changed, about the changed pte. Both here and at Documentation/.../locking. I'll send a new patch just tell me if you want the reurn thing Thank you Boaz From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-ID: <5523DD83.4050609@plexistor.com> Date: Tue, 07 Apr 2015 16:37:07 +0300 From: Boaz Harrosh MIME-Version: 1.0 To: "Kirill A. Shutemov" CC: Dave Chinner , Matthew Wilcox , Andrew Morton , "Kirill A. Shutemov" , Jan Kara , Hugh Dickins , Mel Gorman , linux-mm@kvack.org, linux-nvdimm , linux-fsdevel , Eryu Guan , Christoph Hellwig , Stable Tree Subject: Re: [PATCH 1/3 v6] mm(v4.1): New pfn_mkwrite same as page_mkwrite for VM_PFNMAP References: <55239645.9000507@plexistor.com> <552397E6.5030506@plexistor.com> <5523D43C.1060708@plexistor.com> <20150407131700.GA13946@node.dhcp.inet.fi> <20150407132601.GA14252@node.dhcp.inet.fi> In-Reply-To: <20150407132601.GA14252@node.dhcp.inet.fi> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: On 04/07/2015 04:26 PM, Kirill A. Shutemov wrote: > On Tue, Apr 07, 2015 at 04:17:00PM +0300, Kirill A. Shutemov wrote: >> On Tue, Apr 07, 2015 at 03:57:32PM +0300, Boaz Harrosh wrote: >>> +/* >>> + * Handle write page faults for VM_MIXEDMAP or VM_PFNMAP for a VM_SHARED >>> + * mapping >>> + */ >>> +static int wp_pfn_shared(struct mm_struct *mm, >>> + struct vm_area_struct *vma, unsigned long address, >>> + pte_t *page_table, spinlock_t *ptl, pte_t orig_pte, >>> + pmd_t *pmd) >>> +{ >>> + if (vma->vm_ops && vma->vm_ops->pfn_mkwrite) { >>> + struct vm_fault vmf = { >>> + .page = NULL, >>> + .pgoff = linear_page_index(vma, address), >>> + .virtual_address = (void __user *)(address & PAGE_MASK), >>> + .flags = FAULT_FLAG_WRITE | FAULT_FLAG_MKWRITE, >>> + }; >>> + int ret; >>> + >>> + pte_unmap_unlock(page_table, ptl); >>> + ret = vma->vm_ops->pfn_mkwrite(vma, &vmf); >>> + if (ret & VM_FAULT_ERROR) >>> + return ret; >>> + page_table = pte_offset_map_lock(mm, pmd, address, &ptl); >>> + /* Did pfn_mkwrite already fixed up the pte */ > > Oh. I guess you've missunderstood why we need pte_same() check below. > It's not about ->pfn_mkwrite() changing the pte (generatlly, it should > not). It's requited to address race with parallel page fault to the pte. > >>> + if (!pte_same(*page_table, orig_pte)) { >>> + pte_unmap_unlock(page_table, ptl); >>> + return ret; >> >> This should be "return 0;", shouldn't it? >> >> VM_FAULT_NOPAGE would imply you've installed new pte, but you did not. Changing this to "return 0" would be very scary for me. Because I'm running with this code for 1/2 a year now. And it is stable. You see since the original code it was always doing just that pte_unmap_unlock && return ret. (See the patch based on 4.0) I did not understand if you want that I keep it "return ret". I gather that you would like the comment changed, about the changed pte. Both here and at Documentation/.../locking. I'll send a new patch just tell me if you want the reurn thing Thank you Boaz -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org