| Message ID | 20181127144351.9137-1-mans@mansr.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm: fix insert_pfn() return value | expand |
On Tue, Nov 27, 2018 at 02:43:51PM +0000, Mans Rullgard wrote: > Commit 9b5a8e00d479 ("mm: convert insert_pfn() to vm_fault_t") accidentally > made insert_pfn() always return an error. Fix this. Umm. VM_FAULT_NOPAGE is not an error. It's saying "I inserted the PFN, there's no struct page for the core VM to do anything with". Which is the correct response from a device driver which has called insert_pfn(). Could you explain a bit more what led you to think there's a problem here? Also, rather rude of you not to cc the patch author when you claim to be fixing a bug in their patch. > Fixes: 9b5a8e00d479 ("mm: convert insert_pfn() to vm_fault_t") > Signed-off-by: Mans Rullgard <mans@mansr.com> > --- > mm/memory.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/mm/memory.c b/mm/memory.c > index 4ad2d293ddc2..15baf50e3908 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -1524,12 +1524,14 @@ static vm_fault_t insert_pfn(struct vm_area_struct *vma, unsigned long addr, > pfn_t pfn, pgprot_t prot, bool mkwrite) > { > struct mm_struct *mm = vma->vm_mm; > + int retval; > pte_t *pte, entry; > spinlock_t *ptl; > > pte = get_locked_pte(mm, addr, &ptl); > if (!pte) > return VM_FAULT_OOM; > + retval = VM_FAULT_NOPAGE; > if (!pte_none(*pte)) { > if (mkwrite) { > /* > @@ -1567,9 +1569,10 @@ static vm_fault_t insert_pfn(struct vm_area_struct *vma, unsigned long addr, > set_pte_at(mm, addr, pte, entry); > update_mmu_cache(vma, addr, pte); /* XXX: why not for insert_page? */ > > + retval = 0; > out_unlock: > pte_unmap_unlock(pte, ptl); > - return VM_FAULT_NOPAGE; > + return retval; > } > > /** > -- > 2.19.2 >
Matthew Wilcox <willy@infradead.org> writes: > On Tue, Nov 27, 2018 at 02:43:51PM +0000, Mans Rullgard wrote: >> Commit 9b5a8e00d479 ("mm: convert insert_pfn() to vm_fault_t") accidentally >> made insert_pfn() always return an error. Fix this. > > Umm. VM_FAULT_NOPAGE is not an error. It's saying "I inserted the PFN, > there's no struct page for the core VM to do anything with". Which is > the correct response from a device driver which has called insert_pfn(). > > Could you explain a bit more what led you to think there's a problem here? Apparently some (not in mainline) driver code had been hastily converted to the vm_fault_t codes, and that is where the error is. Sorry for the noise. Please disregard this. (The quickest way to get the correct answer is still to send a bad patch.) > Also, rather rude of you not to cc the patch author when you claim to > be fixing a bug in their patch. Sorry about that. Blame the get-maintainers script.
diff --git a/mm/memory.c b/mm/memory.c index 4ad2d293ddc2..15baf50e3908 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -1524,12 +1524,14 @@ static vm_fault_t insert_pfn(struct vm_area_struct *vma, unsigned long addr, pfn_t pfn, pgprot_t prot, bool mkwrite) { struct mm_struct *mm = vma->vm_mm; + int retval; pte_t *pte, entry; spinlock_t *ptl; pte = get_locked_pte(mm, addr, &ptl); if (!pte) return VM_FAULT_OOM; + retval = VM_FAULT_NOPAGE; if (!pte_none(*pte)) { if (mkwrite) { /* @@ -1567,9 +1569,10 @@ static vm_fault_t insert_pfn(struct vm_area_struct *vma, unsigned long addr, set_pte_at(mm, addr, pte, entry); update_mmu_cache(vma, addr, pte); /* XXX: why not for insert_page? */ + retval = 0; out_unlock: pte_unmap_unlock(pte, ptl); - return VM_FAULT_NOPAGE; + return retval; } /**
Commit 9b5a8e00d479 ("mm: convert insert_pfn() to vm_fault_t") accidentally made insert_pfn() always return an error. Fix this. Fixes: 9b5a8e00d479 ("mm: convert insert_pfn() to vm_fault_t") Signed-off-by: Mans Rullgard <mans@mansr.com> --- mm/memory.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)