Messages in this thread | | | From | Souptick Joarder <> | Date | Fri, 21 Sep 2018 18:55:47 +0530 | Subject | Re: [PATCH] kernel/kcov: Replace vm_insert_page with vmf_insert_page |
| |
On Fri, Sep 21, 2018 at 5:22 PM Andrey Ryabinin <aryabinin@virtuozzo.com> wrote: > > > > On 09/21/2018 01:03 PM, Souptick Joarder wrote: > > On Fri, Sep 21, 2018 at 3:06 PM Andrey Ryabinin <aryabinin@virtuozzo.com> wrote: > >> > >> On 09/20/2018 10:12 PM, Souptick Joarder wrote: > >>> There is a plan to replace vm_insert_page with new API > >>> vmf_insert_page. As part of it, converting vm_insert_page > >>> to use vmf_insert_page. > >>> > >>> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com> > >>> --- > >>> kernel/kcov.c | 5 +++-- > >>> 1 file changed, 3 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/kernel/kcov.c b/kernel/kcov.c > >>> index 3ebd09e..8900d8e 100644 > >>> --- a/kernel/kcov.c > >>> +++ b/kernel/kcov.c > >>> @@ -293,8 +293,9 @@ static int kcov_mmap(struct file *filep, struct vm_area_struct *vma) > >>> spin_unlock(&kcov->lock); > >>> for (off = 0; off < size; off += PAGE_SIZE) { > >>> page = vmalloc_to_page(kcov->area + off); > >>> - if (vm_insert_page(vma, vma->vm_start + off, page)) > >>> - WARN_ONCE(1, "vm_insert_page() failed"); > >>> + if (vmf_insert_page(vma, vma->vm_start + off, page) > >>> + != VM_FAULT_NOPAGE) > >>> + WARN_ONCE(1, "vmf_insert_page() failed"); > >> > >> Nack, don't see the reason for such change, it only makes code worse. > > > > Yes, it needed. > > No, it's not needed. vm_insert_page() works perfectly fine here. > > > Going forward vm_insert_page will be converted to > > vmf_insert_page. As part of it, this code has to be converted to use > > vmf_insert_page(). > > This doesn't explain why such conversion would make sense for kcov_mmap().
vm_insert_page used to return errno which individual drivers have to map to VM_FAULT_CODE. There were also places where return value of vm_insert_page was ignored and return a TRUE value.
As part of vm_fault_t migration patches, we have identified and clean up all these. The plan was to introduce inline wrapper vmf_insert_page(), convert all the page fault handlers to use it and then convert vm_insert_page to vmf_insert_page. By now we have converted all the page fault handlers to use this API.
Still their are few places where vm_insert_page is called outside fault handlers context to map kernel page to user vma and kernel/kcov.c is one of them.
Now removing vm_insert_page from those few places one by one.
Going forward vm_insert_page will be removed from kernel permanently, so that new drivers can't use this API and create new errno to VM_FAULT_CODE mapping code.
> > > please refer below commit on linus tree - > > 1c8f422059ae5da07db74 > > That the commit has nothing to deal with kcov, it doesn't explain why kcov_mmap() > can't keep using vm_insert_page() and/or why we should prefer vmf_insert_page() > over vm_insert_page() particularly in the case of kcov_mmap(). >
| |