Messages in this thread | | | Subject | Re: [PATCH] kernel/kcov: Replace vm_insert_page with vmf_insert_page | From | Andrey Ryabinin <> | Date | Fri, 21 Sep 2018 14:53:15 +0300 |
| |
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().
> 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().
| |