[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190729150539.GB11349@redhat.com>
Date: Mon, 29 Jul 2019 17:05:40 +0200
From: Oleg Nesterov <oleg@...hat.com>
To: akpm@...ux-foundation.org
Cc: kirill.shutemov@...ux.intel.com, matthew.wilcox@...cle.com,
peterz@...radead.org, rostedt@...dmis.org, songliubraving@...com,
srikar@...ux.vnet.ibm.com, william.kucharski@...cle.com,
linux-kernel@...r.kernel.org
Subject: Re: + uprobe-use-original-page-when-all-uprobes-are-removed.patch
added to -mm tree
I didn't see this version, so let me reply here.
On 07/26, Andrew Morton wrote:
>
> + /* try orig_page only for unregister and anonymous old_page */
> + if (!is_register && PageAnon(old_page)) {
Well, this is confusing... nothing really wrong, but we certainly do not
want to install the new anonymous page if !is_register && !PageAnon(old).
And in this case we do not even want to call __replace page().
OK, I won't insist, this should almost never happen, but again, please
see https://lore.kernel.org/lkml/20190726084423.GA16112@redhat.com/
> + struct page *orig_page;
> + pgoff_t index;
> +
> + index = vaddr_to_offset(vma, vaddr & PAGE_MASK) >> PAGE_SHIFT;
> + orig_page = find_get_page(vma->vm_file->f_inode->i_mapping,
> + index);
> +
> + if (orig_page) {
> + if (PageUptodate(orig_page) &&
> + pages_identical(new_page, orig_page)) {
> + /* let go new_page */
> + put_page(new_page);
> + new_page = NULL;
> +
> + /* dec_mm_counter for old_page */
> + dec_mm_counter(mm, MM_ANONPAGES);
this assumes that __replace_page() can't fail, but it can. I think you
should move this into into __replace_page().
Oleg.
Powered by blists - more mailing lists