* Re: + uprobe-use-original-page-when-all-uprobes-are-removed.patch added to -mm tree
[not found] <20190726230333.drvM6x-wz%akpm@linux-foundation.org>
@ 2019-07-29 15:05 ` Oleg Nesterov
2019-07-29 17:04 ` Song Liu
0 siblings, 1 reply; 4+ messages in thread
From: Oleg Nesterov @ 2019-07-29 15:05 UTC (permalink / raw)
To: akpm
Cc: kirill.shutemov, matthew.wilcox, peterz, rostedt, songliubraving,
srikar, william.kucharski, linux-kernel
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: + uprobe-use-original-page-when-all-uprobes-are-removed.patch added to -mm tree
2019-07-29 15:05 ` + uprobe-use-original-page-when-all-uprobes-are-removed.patch added to -mm tree Oleg Nesterov
@ 2019-07-29 17:04 ` Song Liu
2019-07-30 19:17 ` Andrew Morton
0 siblings, 1 reply; 4+ messages in thread
From: Song Liu @ 2019-07-29 17:04 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andrew Morton, Kirill A. Shutemov, Matthew Wilcox,
Peter Zijlstra, Steven Rostedt, Srikar Dronamraju,
William Kucharski, linux-kernel
> On Jul 29, 2019, at 8:05 AM, Oleg Nesterov <oleg@redhat.com> wrote:
>
> 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/
I see. Do you want me fold this patch with 2/4?
>
>> + 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().
Good catch! Let me fix it.
Hi Andrew,
Do you prefer a whole v10 (1/4 to 4/4) or just newer version of this
one (2/4)?
Thanks,
Song
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: + uprobe-use-original-page-when-all-uprobes-are-removed.patch added to -mm tree
2019-07-29 17:04 ` Song Liu
@ 2019-07-30 19:17 ` Andrew Morton
2019-07-30 19:28 ` Song Liu
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Morton @ 2019-07-30 19:17 UTC (permalink / raw)
To: Song Liu
Cc: Oleg Nesterov, Kirill A. Shutemov, Matthew Wilcox,
Peter Zijlstra, Steven Rostedt, Srikar Dronamraju,
William Kucharski, linux-kernel
On Mon, 29 Jul 2019 17:04:05 +0000 Song Liu <songliubraving@fb.com> wrote:
> > this assumes that __replace_page() can't fail, but it can. I think you
> > should move this into into __replace_page().
>
> Good catch! Let me fix it.
>
> Hi Andrew,
>
> Do you prefer a whole v10 (1/4 to 4/4) or just newer version of this
> one (2/4)?
Either is OK. I guess just a new 2/4 is sufficient, if the difference
is minor.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: + uprobe-use-original-page-when-all-uprobes-are-removed.patch added to -mm tree
2019-07-30 19:17 ` Andrew Morton
@ 2019-07-30 19:28 ` Song Liu
0 siblings, 0 replies; 4+ messages in thread
From: Song Liu @ 2019-07-30 19:28 UTC (permalink / raw)
To: Andrew Morton
Cc: Oleg Nesterov, Kirill A. Shutemov, Matthew Wilcox,
Peter Zijlstra, Steven Rostedt, Srikar Dronamraju,
William Kucharski, linux-kernel
> On Jul 30, 2019, at 12:17 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Mon, 29 Jul 2019 17:04:05 +0000 Song Liu <songliubraving@fb.com> wrote:
>
>>> this assumes that __replace_page() can't fail, but it can. I think you
>>> should move this into into __replace_page().
>>
>> Good catch! Let me fix it.
>>
>> Hi Andrew,
>>
>> Do you prefer a whole v10 (1/4 to 4/4) or just newer version of this
>> one (2/4)?
>
> Either is OK. I guess just a new 2/4 is sufficient, if the difference
> is minor.
I sent v10 last night. Then Oleg found some issue with 3/4 (thanks!) Let
me resend 2/4 (with change in commit log) and 3/4 (with code change).
Thanks,
Song
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-07-30 19:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20190726230333.drvM6x-wz%akpm@linux-foundation.org>
2019-07-29 15:05 ` + uprobe-use-original-page-when-all-uprobes-are-removed.patch added to -mm tree Oleg Nesterov
2019-07-29 17:04 ` Song Liu
2019-07-30 19:17 ` Andrew Morton
2019-07-30 19:28 ` Song Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).