Messages in this thread | | | Date | Wed, 20 Jun 2012 17:37:42 +0530 | From | Srikar Dronamraju <> | Subject | Re: [PATCH 2/5] uprobes: __replace_page() should not use page_address_in_vma() |
| |
* Oleg Nesterov <oleg@redhat.com> [2012-06-19 21:47:12]:
> page_address_in_vma(old_page) in __replace_page() is ugly and wrong. > The caller already knows the correct virtual address, this page was > found by get_user_pages(vaddr). > > However, page_address_in_vma() can actually fail if page->mapping was > cleared by __delete_from_page_cache() after get_user_pages() returns. > But this means the race with page reclaim, write_opcode() should not > fail, it should retry and read this page again. Not sure this race is > really possible though, page_freeze_refs() logic should prevent it. > > We could change __replace_page() to return -EAGAIN in this case, but > it would be better to simply use the caller's vaddr and rely on > page_check_address(). > > Signed-off-by: Oleg Nesterov <oleg@redhat.com> > --- > kernel/events/uprobes.c | 10 +++------- > 1 files changed, 3 insertions(+), 7 deletions(-) > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > index a2b32a5..5b10705 100644 > --- a/kernel/events/uprobes.c > +++ b/kernel/events/uprobes.c > @@ -132,17 +132,13 @@ static loff_t vma_address(struct vm_area_struct *vma, loff_t offset) > * > * Returns 0 on success, -EFAULT on failure. > */ > -static int __replace_page(struct vm_area_struct *vma, struct page *page, struct page *kpage) > +static int __replace_page(struct vm_area_struct *vma, unsigned long addr, > + struct page *page, struct page *kpage)
Could please update the comment above __replace_page to mention that it now takes addr as a parameter?
| |