Re: [PATCH v4 3.0-rc2-tip 2/22] 2: uprobes: Breakground page replacement.

quoted

quoted

quoted

+
+	/* Read the page with vaddr into memory */
+	ret = get_user_pages(tsk, tsk->mm, vaddr, 1, 1, 1, &old_page, &vma);

Sorry if this was already discussed... But why we are using FOLL_WRITE here?
We are not going to write into this page, and this provokes the unnecessary
cow, no?

Yes, We are not going to write to the page returned by get_user_pages
but a copy of that page.

Yes I see. But the page returned by get_user_pages(write => 1) is already
a cow'ed copy (this mapping should be read-only).

quoted

The idea was if we cow the page then we dont
need to cow it at the replace_page time

Yes, replace_page() shouldn't cow.

quoted

and since get_user_pages knows
the right way to cow the page, we dont have to write another routine to
cow the page.

Confused. write_opcode() allocs another page and does memcpy. This is
correct, but I don't understand the first cow.

we decided on get_user_pages(FOLL_WRITE|FOLL_FORCE) based on discussions
in these threads https://lkml.org/lkml/2010/4/23/327 and
https://lkml.org/lkml/2010/5/12/119

Summary of those two sub-threads as I understand was to have
get_user_pages do the "real" cow for us.

If I understand correctly, your concern is on the extra overhead added
by the get_user_pages. Other than that is there any side-effect of we
forcing the cow through get_user_pages.

quoted

I am still not clear on your concern.

Probably I missed something... but could you please explain why we can't

	- ret = get_user_pages(tsk, tsk->mm, vaddr, 1, 1, 1, &old_page, &vma);
	+ ret = get_user_pages(tsk, tsk->mm, vaddr, 1, 0, 0, &old_page, &vma);

?

I tried the code with this change and it works for regular cases.
I am not sure if it affects cases where programs do mprotect 
So I am okay to not force cow through get_user_pages.

quoted

quoted

Also. This is called under down_read(mmap_sem), can't we race with
access_process_vm() modifying the same memory?

Yes, we could be racing with access_process_vm on the same memory.

Do we have any other option other than making write_opcode/read_opcode
being called under down_write(mmap_sem)?

I dunno. Probably we can simply ignore this issue, there are other ways
to modify this memory.

Okay.

-- 
Thanks and Regards
Srikar

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>