| Message ID | 1478233517-3571-18-git-send-email-jack@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Nov 04, 2016 at 05:25:13AM +0100, Jan Kara wrote: > Currently finish_mkwrite_fault() returns 0 when PTE got changed before > we acquired PTE lock and VM_FAULT_WRITE when we succeeded in modifying > the PTE. This is somewhat confusing since 0 generally means success, it > is also inconsistent with finish_fault() which returns 0 on success. > Change finish_mkwrite_fault() to return 0 on success and VM_FAULT_NOPAGE > when PTE changed. Practically, there should be no behavioral difference > since we bail out from the fault the same way regardless whether we > return 0, VM_FAULT_NOPAGE, or VM_FAULT_WRITE. Also note that > VM_FAULT_WRITE has no effect for shared mappings since the only two > places that check it - KSM and GUP - care about private mappings only. > Generally the meaning of VM_FAULT_WRITE for shared mappings is not well > defined and we should probably clean that up. > > Signed-off-by: Jan Kara <jack@suse.cz> Sounds right. Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
diff --git a/mm/memory.c b/mm/memory.c index 1517ff91c743..b3bd6b6c6472 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -2296,10 +2296,10 @@ int finish_mkwrite_fault(struct vm_fault *vmf) */ if (!pte_same(*vmf->pte, vmf->orig_pte)) { pte_unmap_unlock(vmf->pte, vmf->ptl); - return 0; + return VM_FAULT_NOPAGE; } wp_page_reuse(vmf); - return VM_FAULT_WRITE; + return 0; } /* @@ -2342,8 +2342,7 @@ static int wp_page_shared(struct vm_fault *vmf) return tmp; } tmp = finish_mkwrite_fault(vmf); - if (unlikely(!tmp || (tmp & - (VM_FAULT_ERROR | VM_FAULT_NOPAGE)))) { + if (unlikely(tmp & (VM_FAULT_ERROR | VM_FAULT_NOPAGE))) { unlock_page(vmf->page); put_page(vmf->page); return tmp;
Currently finish_mkwrite_fault() returns 0 when PTE got changed before we acquired PTE lock and VM_FAULT_WRITE when we succeeded in modifying the PTE. This is somewhat confusing since 0 generally means success, it is also inconsistent with finish_fault() which returns 0 on success. Change finish_mkwrite_fault() to return 0 on success and VM_FAULT_NOPAGE when PTE changed. Practically, there should be no behavioral difference since we bail out from the fault the same way regardless whether we return 0, VM_FAULT_NOPAGE, or VM_FAULT_WRITE. Also note that VM_FAULT_WRITE has no effect for shared mappings since the only two places that check it - KSM and GUP - care about private mappings only. Generally the meaning of VM_FAULT_WRITE for shared mappings is not well defined and we should probably clean that up. Signed-off-by: Jan Kara <jack@suse.cz> --- mm/memory.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)