Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-05 Thread Oleg Nesterov
On 12/05, Borislav Petkov wrote: > > On Thu, Dec 05, 2013 at 06:23:55PM +0100, Oleg Nesterov wrote: > > This is almost off-topic, but I am wondering if (in the long term) we > > can avoid this "insert the bp into every mm" altogether. > > > > Instead, uprobe_write_opcode() should only unmap this pa

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-05 Thread Borislav Petkov
On Thu, Dec 05, 2013 at 06:23:55PM +0100, Oleg Nesterov wrote: > This is almost off-topic, but I am wondering if (in the long term) we > can avoid this "insert the bp into every mm" altogether. > > Instead, uprobe_write_opcode() should only unmap this page and set Ok, sorry if I'm completely off b

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-05 Thread Oleg Nesterov
On 12/04, Oleg Nesterov wrote: > > And/Or. Are you still saying that on x86 (and powerpc?) we do not need > these pte games at all and uprobe_write_opcode() can simply do: > > /* Break the mapping unless the page is already anonymous */ > ret = get_user_pages(NULL, mm, v

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-05 Thread Jon Medhurst (Tixy)
On Wed, 2013-12-04 at 09:15 -0800, Linus Torvalds wrote: > On Wed, Dec 4, 2013 at 8:54 AM, H. Peter Anvin wrote: > > > > That is why I talk about the atomic instruction word... most (but not > > *all*) architectures have a fundamental minimum unit of instructions > > which is aligned and can be at

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-04 Thread H. Peter Anvin
On 12/04/2013 09:15 AM, Linus Torvalds wrote: > On Wed, Dec 4, 2013 at 8:54 AM, H. Peter Anvin wrote: >> >> That is why I talk about the atomic instruction word... most (but not >> *all*) architectures have a fundamental minimum unit of instructions >> which is aligned and can be atomically writte

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-04 Thread Oleg Nesterov
Peter, Linus, I got lost. So what do you finally think about this change? Please see v2 below: - update the comment above gup(write, force) - add flush_icache_page() before set_pte_at() (nop on x86 and powerpc) - fix the returned value, and with this change it

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-04 Thread Linus Torvalds
On Wed, Dec 4, 2013 at 8:54 AM, H. Peter Anvin wrote: > > That is why I talk about the atomic instruction word... most (but not > *all*) architectures have a fundamental minimum unit of instructions > which is aligned and can be atomically written. Typically this is 1, 2, > or 4 bytes. Note that

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-04 Thread H. Peter Anvin
On 12/04/2013 08:48 AM, Oleg Nesterov wrote: > On 12/04, H. Peter Anvin wrote: >> >> On 12/04/2013 03:11 AM, Oleg Nesterov wrote: >>> >>> It is still not clear to me if we can simply change a single byte on >>> x86 or not, but at least on powerpc we need to update 4 bytes. Perhaps >>> we can condit

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-04 Thread Oleg Nesterov
On 12/04, H. Peter Anvin wrote: > > On 12/04/2013 03:11 AM, Oleg Nesterov wrote: > > > > It is still not clear to me if we can simply change a single byte on > > x86 or not, but at least on powerpc we need to update 4 bytes. Perhaps > > we can conditionalize these pte games later. > > > > But 4 ali

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-04 Thread H. Peter Anvin
On 12/04/2013 03:11 AM, Oleg Nesterov wrote: > > It is still not clear to me if we can simply change a single byte on > x86 or not, but at least on powerpc we need to update 4 bytes. Perhaps > we can conditionalize these pte games later. > But 4 aligned bytes can be written as a single transacti

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-04 Thread Oleg Nesterov
On 12/03, H. Peter Anvin wrote: > > On 12/03/2013 02:01 PM, Linus Torvalds wrote: > > On Tue, Dec 3, 2013 at 12:54 PM, Oleg Nesterov wrote: > >> > >> So do you think the patch I sent is wrong? Why? > > > > I think the TLB shootdown should guarantee that it's ok on other > > CPU's, since that's bas

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-04 Thread Oleg Nesterov
On 12/03, Linus Torvalds wrote: > > On Tue, Dec 3, 2013 at 12:54 PM, Oleg Nesterov wrote: > > > > So do you think the patch I sent is wrong? Why? > > I think the TLB shootdown should guarantee that it's ok on other > CPU's, since that's basically what we do on mmap. OK, thanks. I'll resend this p

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-03 Thread H. Peter Anvin
On 12/03/2013 02:01 PM, Linus Torvalds wrote: > On Tue, Dec 3, 2013 at 12:54 PM, Oleg Nesterov wrote: >> >> So do you think the patch I sent is wrong? Why? > > I think the TLB shootdown should guarantee that it's ok on other > CPU's, since that's basically what we do on mmap. > I think that is

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-03 Thread H. Peter Anvin
On 12/03/2013 12:54 PM, Oleg Nesterov wrote: > On 12/03, H. Peter Anvin wrote: >> >> There is no architecture-independent way to make code globally visible. > > Then I probably misunderstood "globally". Or something else. > > So do you think the patch I sent is wrong? Why? > It is wrong in the

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-03 Thread Linus Torvalds
On Tue, Dec 3, 2013 at 12:54 PM, Oleg Nesterov wrote: > > So do you think the patch I sent is wrong? Why? I think the TLB shootdown should guarantee that it's ok on other CPU's, since that's basically what we do on mmap. But looking closer at this, I think I see why the old code did what it did.

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-03 Thread Oleg Nesterov
On 12/03, H. Peter Anvin wrote: > > There is no architecture-independent way to make code globally visible. Then I probably misunderstood "globally". Or something else. So do you think the patch I sent is wrong? Why? I am totally confused. Oleg. -- To unsubscribe from this list: send the line

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-03 Thread H. Peter Anvin
There is no architecture-independent way to make code globally visible. Oleg Nesterov wrote: >On 12/03, H. Peter Anvin wrote: >> >> On 12/03/2013 12:01 PM, Oleg Nesterov wrote: >> > >> >> followed by whatever synchronization necessary to make it globally >visible. >> > >> > Could you explain what

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-03 Thread Oleg Nesterov
On 12/03, H. Peter Anvin wrote: > > On 12/03/2013 12:01 PM, Oleg Nesterov wrote: > > > >> followed by whatever synchronization necessary to make it globally visible. > > > > Could you explain what this synchronization should actually do ? > > > > That is architecture-dependent. On x86 it requires e

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-03 Thread H. Peter Anvin
On 12/03/2013 12:01 PM, Oleg Nesterov wrote: > >> followed by whatever synchronization necessary to make it globally visible. > > Could you explain what this synchronization should actually do ? > That is architecture-dependent. On x86 it requires each CPU on which this code is visible to be I

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-03 Thread Oleg Nesterov
On 12/03, H. Peter Anvin wrote: > > I guess it would have to be checked, but I would be *highly* surprised > if UPROBE_SWBP_INSN_SIZE ever[1] could be anything than the fundamental > instruction quantum, which means it should never be able to wrap a page, Yes, it can't. > but *also* should mean i

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-03 Thread Oleg Nesterov
On 12/03, Linus Torvalds wrote: > > On Tue, Dec 3, 2013 at 10:49 AM, Oleg Nesterov wrote: > > > > See the patch below. For review only > > Looks completely broken. Where do you guarantee that it's just a single page? Yes, it is always a single page on all supported architectures. This is even do

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-03 Thread H. Peter Anvin
On 12/03/2013 11:00 AM, Linus Torvalds wrote: > > Yes, on x86, UPROBE_SWBP_INSN_SIZE is a single byte. But quite > frankly, on x86, exactly *because* it's a single byte, I don't > understand why we don't just write the damn thing with a single > "put_user()", and stop with all the idiotic games. N

Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-03 Thread Linus Torvalds
On Tue, Dec 3, 2013 at 10:49 AM, Oleg Nesterov wrote: > > See the patch below. For review only Looks completely broken. Where do you guarantee that it's just a single page? Yes, on x86, UPROBE_SWBP_INSN_SIZE is a single byte. But quite frankly, on x86, exactly *because* it's a single byte, I don

[PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

2013-12-03 Thread Oleg Nesterov
On 11/29, Linus Torvalds wrote: > > On Fri, Nov 29, 2013 at 3:24 PM, Jiri Kosina wrote: > > > > Do you think this'd be faster than the int3-based aproach? > > Unlikely to be faster, but perhaps more robust and more portable. Maybe. Can't we at least change uprobe_write_opcode() and kill the nontr