Re: [PATCH 0/1] uprobes: Kill __replace_page(), changeuprobe_write_opcode() to rely on gup(WRITE)

[Linus Torvalds]

On Tue, Dec 10, 2013 at 12:04 PM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
>> We'd be *much* better off using get_user_pages_fast() if possible -
>> and I bet _is_ possible in 99% of all cases.
>
> We can't. get_user_pages_fast() takes mmap_sem.

Yeah, and we need to look up the page table entry anyway, so what we
actually want here is just the page table walker, none of the "get
page" crap at all.

So the core function should (I think) just do something like:

 - new_page = NULL
 - get page table lock
 - look up page tables
 - if it's dirty and private, or we have a new page for it, replace
the instruction
 - drop page table lock
 - we're done, free the new page if we didn't use it.

with a retry for the "uhhuh, it's not dirty and private, and we don't
have a new page", so we do
 - drop the lock
 - use a "get_user()" or something to page it in.
 - allocate a new page if necessary
 - goto retry

which gets us a single page table walk for the common case, and a
retry for the "uhhuh, we needed to page it in or allocate a new page"
condition.

Put another way: I actually think the existing "__replace_page()" code
is closer to being good than that disgusting uprobe_write_opcode()
function. I think you may be getting rid of the wrong ugly function.

                Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/