Re: [PATCH v2] powerpc/uprobes: Validation for prefixed instruction

On 02/04, Ravi Bangoria wrote: > > +static int get_instr(struct mm_struct *mm, unsigned long addr, u32 *instr) > +{ > + struct page *page; > + struct vm_area_struct *vma; > + void *kaddr; > + unsigned int gup_flags = FOLL_FORCE | FOLL_SPLIT_PMD; > + > + if

Re: [PATCH] powerpc/uprobes: Don't allow probe on suffix of prefixed instruction

On 01/19, Ravi Bangoria wrote: > > Probe on 2nd word of a prefixed instruction is invalid scenario and > should be restricted. I don't understand this ppc-specific problem, but... > +#ifdef CONFIG_PPC64 > +int arch_uprobe_verify_opcode(struct page *page, unsigned long vaddr, > +

Re: [PATCH v3 2/2] powerpc/ptrace: Hard wire PT_SOFTE value to 1 in gpr_get() too

Christophe, et al, So what? Are you going to push your change or should I re-send 1-2 without whitespace cleanups? On 11/19, Oleg Nesterov wrote: > > On 11/19, Christophe Leroy wrote: > > > > I think the following should work, and not require the first patch (compi

Re: [PATCH v3 2/2] powerpc/ptrace: Hard wire PT_SOFTE value to 1 in gpr_get() too

On 11/19, Christophe Leroy wrote: > > I think the following should work, and not require the first patch (compile > tested only). > > --- a/arch/powerpc/kernel/ptrace/ptrace-view.c > +++ b/arch/powerpc/kernel/ptrace/ptrace-view.c > @@ -234,9 +234,21 @@ static int gpr_get(struct task_struct

Re: [PATCH v3 0/2] powerpc/ptrace: Hard wire PT_SOFTE value to 1 in gpr_get() too

On 11/19, Christophe Leroy wrote: > > > Le 19/11/2020 à 17:01, Oleg Nesterov a écrit : > >Can we finally fix this problem? ;) > > > >My previous attempt was ignored, see > > That doesn't seems right. > > Michael made some suggestion it seems, ca

Re: [PATCH v3 1/2] powerpc/ptrace: simplify gpr_get/tm_cgpr_get

On 11/19, Christophe Leroy wrote: > > > Le 19/11/2020 à 17:02, Oleg Nesterov a écrit : > >gpr_get() does membuf_write() twice to override pt_regs->msr in between. > > Is there anything wrong with that ? Nothing wrong, but imo the code and 2/2 looks simpler after this p

Re: [PATCH v3 2/2] powerpc/ptrace: Hard wire PT_SOFTE value to 1 in gpr_get() too

On 11/19, Oleg Nesterov wrote: > > This is not consistent and this breaks the user-regs-peekpoke test > from https://sourceware.org/systemtap/wiki/utrace/tests/ See the test-case below. Oleg. /* Test case for PTRACE_SETREGS modifying the requested ragisters. x86* counterpart of

[PATCH v3 2/2] powerpc/ptrace: Hard wire PT_SOFTE value to 1 in gpr_get() too

are.org/systemtap/wiki/utrace/tests/ Reported-by: Jan Kratochvil Signed-off-by: Oleg Nesterov --- arch/powerpc/kernel/ptrace/ptrace-tm.c | 8 +++- arch/powerpc/kernel/ptrace/ptrace-view.c | 8 +++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kernel/ptra

[PATCH v3 1/2] powerpc/ptrace: simplify gpr_get/tm_cgpr_get

after membuf_write(). Signed-off-by: Oleg Nesterov --- arch/powerpc/kernel/ptrace/ptrace-tm.c | 13 + arch/powerpc/kernel/ptrace/ptrace-view.c | 13 + include/linux/regset.h | 12 3 files changed, 22 insertions(+), 16 deletions(-) diff

[PATCH v3 0/2] powerpc/ptrace: Hard wire PT_SOFTE value to 1 in gpr_get() too

Can we finally fix this problem? ;) My previous attempt was ignored, see https://lore.kernel.org/lkml/20190917121256.ga8...@redhat.com/ Now that gpr_get() was changed to use membuf API we can make a simpler fix. Sorry, uncompiled/untested, I don't have a ppc machine. Oleg.

Re: [PATCH? v2] powerpc: Hard wire PT_SOFTE value to 1 in gpr_get() too

On 06/11, Madhavan Srinivasan wrote: > > > On 6/10/20 8:37 PM, Oleg Nesterov wrote: > >Hi, > > > >looks like this patch was forgotten. > > yep, I missed this. But mpe did have comments for the patch. > > https://lkml.org/lkml/2019/9/19/107 Yes, and I th

Re: [PATCH? v2] powerpc: Hard wire PT_SOFTE value to 1 in gpr_get() too

Hi, looks like this patch was forgotten. Do you think this should be fixed or should we document that PTRACE_GETREGS is not consistent with PTRACE_PEEKUSER on ppc64? On 09/17, Oleg Nesterov wrote: > > I don't have a ppc machine, this patch wasn't even compile tested, > could you plea

[PATCH? v2] powerpc: Hard wire PT_SOFTE value to 1 in gpr_get() too

pt_regs->softe as is. This is not consistent and this breaks http://sourceware.org/systemtap/wiki/utrace/tests/user-regs-peekpoke Reported-by: Jan Kratochvil Signed-off-by: Oleg Nesterov --- arch/powerpc/kernel/ptrace.c | 25 + 1 file changed, 25 insertions(+) diff --git a/arch/powerpc/

[PATCH?] powerpc: Hard wire PT_SOFTE value to 1 in gpr_get() too

pt_regs->softe as is. This is not consistent and this breaks http://sourceware.org/systemtap/wiki/utrace/tests/user-regs-peekpoke Reported-by: Jan Kratochvil Signed-off-by: Oleg Nesterov --- arch/powerpc/kernel/ptrace.c | 25 + 1 file changed, 25 insertions(+) diff --git a/arch/powerpc/

Re: [PATCH v1 1/2] open: add close_range()

On 05/23, Christian Brauner wrote: > > So given that we would really need another find_next_open_fd() I think > sticking to the simple cond_resched() version I sent before is better > for now until we see real-world performance issues. OK, agreed. Oleg.

Re: [PATCH v2 1/2] open: add close_range()

On 05/23, Christian Brauner wrote: > > +int __close_range(struct files_struct *files, unsigned fd, unsigned max_fd) > +{ > + unsigned int cur_max; > + > + if (fd > max_fd) > + return -EINVAL; > + > + rcu_read_lock(); > + cur_max = files_fdtable(files)->max_fds; > +

Re: [PATCH v1 1/2] open: add close_range()

On 05/22, Christian Brauner wrote: > > +static struct file *pick_file(struct files_struct *files, unsigned fd) > { > - struct file *file; > + struct file *file = NULL; > struct fdtable *fdt; > > spin_lock(>file_lock); > @@ -632,15 +629,65 @@ int __close_fd(struct

Re: [PATCH 5/5] asm-generic: remove ptrace.h

asm-generic/ptrace.h | 74 -- > 3 files changed, 80 deletions(-) > delete mode 100644 include/asm-generic/ptrace.h Acked-by: Oleg Nesterov

Re: [PATCH 4/5] x86: don't use asm-generic/ptrace.h

On 05/20, Christoph Hellwig wrote: > > Doing the indirection through macros for the regs accessors just > makes them harder to read, so implement the helpers directly. Acked-by: Oleg Nesterov

Re: [PATCH v1 1/2] pid: add pidfd_open()

On 05/17, Aleksa Sarai wrote: > > On 2019-05-16, Oleg Nesterov wrote: > > On 05/17, Aleksa Sarai wrote: > > > On 2019-05-16, Oleg Nesterov wrote: > > > > On 05/16, Christian Brauner wrote: > > > > > With the introduction of pidfds through CLONE_

Re: [PATCH v1 1/2] pid: add pidfd_open()

On 05/17, Aleksa Sarai wrote: > > On 2019-05-16, Oleg Nesterov wrote: > > On 05/16, Christian Brauner wrote: > > > > > > With the introduction of pidfds through CLONE_PIDFD it is possible to > > > created pidfds at process creation time. > > > >

Re: [PATCH v1 1/2] pid: add pidfd_open()

roup leader we might return the old leader. > + */ > + tsk = pid_task(p, PIDTYPE_TGID); > + if (!tsk) > + ret = -ESRCH; > + rcu_read_unlock(); > + > + fd = ret ?: pidfd_create(p); > + put_pid(p); > + return fd; > +} Looks correc

Re: [PATCH 1/2] pid: add pidfd_open()

On 05/15, Oleg Nesterov wrote: > > On 05/15, Christian Brauner wrote: > > > > +SYSCALL_DEFINE2(pidfd_open, pid_t, pid, unsigned int, flags) > > +{ > > + int fd, ret; > > + struct pid *p; > > + struct task_struct *tsk; > > + &

Re: [PATCH 1/2] pid: add pidfd_open()

On 05/15, Christian Brauner wrote: > > On Wed, May 15, 2019 at 04:38:58PM +0200, Oleg Nesterov wrote: > > > > it seems that you can do a single check > > > > tsk = pid_task(p, PIDTYPE_TGID); > > if (!tsk) > > ret = -ESRCH; >

Re: [PATCH 1/2] pid: add pidfd_open()

On 05/15, Christian Brauner wrote: > > +SYSCALL_DEFINE2(pidfd_open, pid_t, pid, unsigned int, flags) > +{ > + int fd, ret; > + struct pid *p; > + struct task_struct *tsk; > + > + if (flags) > + return -EINVAL; > + > + if (pid <= 0) > + return -EINVAL; >

Re: [PATCH linux-next v10 0/7] ptrace: add PTRACE_GET_SYSCALL_INFO request

On 04/16, Dmitry V. Levin wrote: > > [Andrew, could you take this patchset into your tree, please?] Just in case... I have already acked 6/7. Other patches look good to me too, just I don't think I can actually review these non-x86 changes. Oleg.

Re: remove asm-generic/ptrace.h

On 05/01, Christoph Hellwig wrote: > > Hi all, > > asm-generic/ptrace.h is a little weird in that it doesn't actually > implement any functionality, but it provided multiple layers of macros > that just implement trivial inline functions. We implement those > directly in the few architectures and

Re: [PATCH v2 3/6] x86: clean up _TIF_SYSCALL_EMU handling using ptrace_syscall_enter hook

On 04/30, Sudeep Holla wrote: > > On Mon, Mar 18, 2019 at 04:33:22PM +0100, Oleg Nesterov wrote: > > > > And it seems that _TIF_WORK_SYSCALL_ENTRY needs some cleanups too... We > > don't need > > "& _TIF_WORK_SYSCALL_ENTRY" in syscall_trace_enter,

Re: [PATCH v2 4/6] powerpc: use common ptrace_syscall_enter hook to handle _TIF_SYSCALL_EMU

On 03/19, Oleg Nesterov wrote: > > Well, personally I see no point... Again, after the trivial simplification > x86 does > > if (work & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE)) { > ret = tracehook_report_syscall_entry(regs); > if (ret ||

Re: [PATCH v2 4/6] powerpc: use common ptrace_syscall_enter hook to handle _TIF_SYSCALL_EMU

On 03/18, Sudeep Holla wrote: > > On Mon, Mar 18, 2019 at 06:33:41PM +0100, Oleg Nesterov wrote: > > On 03/18, Sudeep Holla wrote: > > > > > > On Mon, Mar 18, 2019 at 06:20:24PM +0100, Oleg Nesterov wrote: > > > > > > > > Again, to me

Re: [PATCH v2 4/6] powerpc: use common ptrace_syscall_enter hook to handle _TIF_SYSCALL_EMU

On 03/18, Sudeep Holla wrote: > > On Mon, Mar 18, 2019 at 06:20:24PM +0100, Oleg Nesterov wrote: > > > > Again, to me this patch just makes the code look worse. Honestly, I don't > > think that the new (badly named) ptrace_syscall_enter() hook makes any > > sense. &

Re: [PATCH v2 1/6] ptrace: move clearing of TIF_SYSCALL_EMU flag to core

On 03/18, Sudeep Holla wrote: > @@ -534,6 +534,10 @@ static int ptrace_detach(struct task_struct *child, unsigned int data) > /* Architecture-specific hardware disable .. */ > ptrace_disable(child); > > +#ifdef TIF_SYSCALL_EMU > + clear_tsk_thread_flag(child, TIF_SYSCALL_EMU);

Re: [PATCH v2 4/6] powerpc: use common ptrace_syscall_enter hook to handle _TIF_SYSCALL_EMU

On 03/18, Sudeep Holla wrote: > > --- a/arch/powerpc/kernel/ptrace.c > +++ b/arch/powerpc/kernel/ptrace.c > @@ -3278,35 +3278,29 @@ long do_syscall_trace_enter(struct pt_regs *regs) > > user_exit(); > > - flags = READ_ONCE(current_thread_info()->flags) & > -

Re: [PATCH v2 3/6] x86: clean up _TIF_SYSCALL_EMU handling using ptrace_syscall_enter hook

On 03/18, Sudeep Holla wrote: > > --- a/arch/x86/entry/common.c > +++ b/arch/x86/entry/common.c > @@ -70,22 +70,16 @@ static long syscall_trace_enter(struct pt_regs *regs) > > struct thread_info *ti = current_thread_info(); > unsigned long ret = 0; > - bool emulated = false; >

Re: [PATCH] powerpc/ptrace: cleanup do_syscall_trace_enter

On 12/16, Dmitry V. Levin wrote: > > long do_syscall_trace_enter(struct pt_regs *regs) > { > + u32 cached_flags; > + > user_exit(); > > - if (test_thread_flag(TIF_SYSCALL_EMU)) { > - /* > - * A nonzero return code from tracehook_report_syscall_entry() > -

Re: [PATCH v6] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call

On 12/07, Dmitry V. Levin wrote: > > Please make either v5 or v6 edition of this fix, or any similar fix, > into v4.20. IIUC, v5 above means [PATCH v5 23/25] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call you sent in another series... > long

Re: [PATCH v4] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call

On 12/07, Dmitry V. Levin wrote: > > On Fri, Dec 07, 2018 at 10:12:49PM +1100, Michael Ellerman wrote: > > > > Sorry, this patch does not work, please ignore it. > > > > Hmm OK. Why exactly? > > Unfortunately, I have no idea why it doesn't work. > All I can say is it breaks strace because the

Re: [PATCH v3 1/2] livepatch: send a fake signal to all blocking tasks

On 11/02, Miroslav Benes wrote: > > On Wed, 1 Nov 2017, Oleg Nesterov wrote: > > > Note also that wake_up_state(TASK_INTERRUPTIBLE) won't wakeup the TASK_IDLE > > kthreads, and most of the kthreads which use TASK_INTERRUPTIBLE should use > > TASK_IDLE t

Re: [PATCH v3 1/2] livepatch: send a fake signal to all blocking tasks

On 11/01, Petr Mladek wrote: > > On Tue 2017-10-31 12:48:52, Miroslav Benes wrote: > > + if (task->flags & PF_KTHREAD) { > > + /* > > +* Wake up a kthread which still has not been migrated. > > +*/ > > +

Re: [PATCH] ptrace: Add compat PTRACE_{G,S}ETSIGMASK handlers

On 06/29, James Morse wrote: > > compat_ptrace_request() lacks handlers for PTRACE_{G,S}ETSIGMASK, > instead using those in ptrace_request(). The compat variant should > read a compat_sigset_t from userspace instead of ptrace_request()s > sigset_t. Acked-by: Oleg Nesterov <o...@redhat.com>

Re: [PATCH v5] powerpc: Do not make the entire heap executable

On 09/28, Kees Cook wrote: > > This is where the flags are actually built from what's coming in > through the newly created exported function vm_brk_flags() below. The > only flag we're acting on is VM_EXEC (passed in from set_brk() above). > I think do_brk_flags() should mask the valid flags, or

Re: [PATCH v4] powerpc: Do not make the entire heap executable

On 08/10, Denys Vlasenko wrote: > > Currently, to support 32-bit binaries with PLT in BSS kernel maps *entire > brk area* with executable rights for all binaries, even --secure-plt ones. > > Stop doing that. Can't really review this patch, but at least the change in mm/mmap.c looks technically

Re: [PATCH 09/14] resource limits: track highwater mark of locked memory

On 07/15, Topi Miettinen wrote: > > On 07/15/16 15:14, Oleg Nesterov wrote: > > > > Btw this is not right. The same for the previous patch which tracks > > RLIMIT_STACK. The "current" task can debugger/etc. > > acct_stack_growth() is called from expand_upwar

Re: [PATCH 09/14] resource limits: track highwater mark of locked memory

On 07/15, Topi Miettinen wrote: > > Track maximum size of locked memory, to be able to configure > RLIMIT_MEMLOCK resource limits. The information is available > with taskstats and cgroupstats netlink socket. So I personally still dislike the very idea of this series... but I won't argue if you

Re: [PATCH v2 00/20] Fix handling of compat_siginfo_t

On 11/07, Andy Lutomirski wrote: > > On Wed, Nov 4, 2015 at 4:50 PM, Amanieu d'Antras wrote: > > One issue that isn't resolved in this series is sending signals between a > > 32-bit > > process and 64-bit process. Sending a si_int will work correctly, but a > > si_ptr > >

Re: [PATCH 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE-READ_ONCE

On 01/15, Christian Borntraeger wrote: Am 15.01.2015 um 20:38 schrieb Oleg Nesterov: On 01/15, Christian Borntraeger wrote: --- a/arch/x86/include/asm/spinlock.h +++ b/arch/x86/include/asm/spinlock.h @@ -186,7 +186,7 @@ static inline void arch_spin_unlock_wait(arch_spinlock_t *lock

Re: [PATCH 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE-READ_ONCE

On 01/15, Christian Borntraeger wrote: --- a/arch/x86/include/asm/spinlock.h +++ b/arch/x86/include/asm/spinlock.h @@ -186,7 +186,7 @@ static inline void arch_spin_unlock_wait(arch_spinlock_t *lock) __ticket_t head = ACCESS_ONCE(lock-tickets.head); for (;;) { -

Re: [PATCH v4 1/3] init/main.c: Give init_task a canary

but was never merged. [1]: http://marc.info/?l=linux-kernelm=127144305403241w=2 Signed-off-by: Aaron Tomlin atom...@redhat.com Acked-by: Michael Ellerman m...@ellerman.id.au Acked-by: Oleg Nesterov o...@redhat.com ___ Linuxppc-dev mailing list Linuxppc-dev

Re: [PATCH 1/2] sched: Add helper for task stack page overrun checking

On 09/04, Aaron Tomlin wrote: +#define task_stack_end_corrupted(task) \ + (*(end_of_stack(task)) != STACK_END_MAGIC) and it is always used along with tsk != init_task. But why we exclude swapper/0? Can't we add end_of_stack(current) = STACK_END_MAGIC; somewhere at the

Re: bit fields data tearing

On 07/13, Benjamin Herrenschmidt wrote: On Sat, 2014-07-12 at 22:51 +0200, Oleg Nesterov wrote: OK, looks like this is compiler bug, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 Thanks to Dan who informed me privately. So yes, there's is this compiler bug which means a bitfield

bit fields data tearing

Hello, I am not sure I should ask here, but since Documentation/memory-barriers.txt mentions load/store tearing perhaps my question is not completely off-topic... I am fighting with mysterious RHEL bug, it can be reproduced on ppc and s390 but not on x86. Finally I seem to understand the

Re: bit fields data tearing

OK, looks like this is compiler bug, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080 Thanks to Dan who informed me privately. On 07/12, Oleg Nesterov wrote: Hello, I am not sure I should ask here, but since Documentation/memory-barriers.txt mentions load/store tearing perhaps my

Re: perf events ring buffer memory barrier on powerpc

On 10/29, Peter Zijlstra wrote: On Tue, Oct 29, 2013 at 11:30:57AM +0100, Peter Zijlstra wrote: @@ -154,9 +175,11 @@ int perf_output_begin(struct perf_output * Userspace could choose to issue a mb() before updating the * tail pointer. So that all reads will be

Re: perf events ring buffer memory barrier on powerpc

On 10/28, Peter Zijlstra wrote: Lets add Paul and Oleg to the thread; this is getting far more 'fun' that it should be ;-) Heh. All I can say is that I would like to see the authoritative answer, you know who can shed a light ;) But to avoid the confusion, wmb() added by this patch looks

Re: perf events ring buffer memory barrier on powerpc

On 10/28, Paul E. McKenney wrote: On Mon, Oct 28, 2013 at 02:26:34PM +0100, Peter Zijlstra wrote: --- linux-2.6.orig/kernel/events/ring_buffer.c +++ linux-2.6/kernel/events/ring_buffer.c @@ -87,10 +87,31 @@ static void perf_output_put_handle(struc goto out; /* -

Re: linux-next: manual merge of the akpm tree with the powerpc tree

. -- From: Oleg Nesterov o...@redhat.com Subject: ptrace/powerpc: revert hw_breakpoints: Fix racy access to ptrace breakpoints This reverts commit 07fa7a0a8a586 (hw_breakpoints: Fix racy access to ptrace breakpoints) and removes ptrace_get

[PATCH] ptrace/powerpc: dont flush_ptrace_hw_breakpoint() on fork()

arch_dup_task_struct() does flush_ptrace_hw_breakpoint(src), this destroys the parent's breakpoints for no reason. We should clear child-thread.ptrace_bps[] copied by dup_task_struct() instead. Signed-off-by: Oleg Nesterov o...@redhat.com Acked-by: Michael Neuling mi...@neuling.org --- x/arch

Re: [PATCH v2 1/4] uprobes: add trap variant helper

On 03/22, Ananth N Mavinakayanahalli wrote: Some architectures like powerpc have multiple variants of the trap instruction. Introduce an additional helper is_trap_insn() for run-time handling of non-uprobe traps on such architectures. Looks fine to me. I am going to take this into my tree,

Re: [PATCH] powerpc/uprobes: teach uprobes to ignore gdb breakpoints

On 03/22, Ananth N Mavinakayanahalli wrote: On Thu, Mar 21, 2013 at 04:58:09PM +0100, Oleg Nesterov wrote: - verify_opcode()-is_swbp_insn() means: is this insn fine for uprobe? (we do not care about gdb, we simply ignore this problem) I will write up

Re: [PATCH 1/3] uprobes: add trap variant helper

On 03/22, Ananth N Mavinakayanahalli wrote: +/** + * is_trap_insn - check if instruction is breakpoint instruction. + * @insn: instruction to be checked. + * Default implementation of is_trap_insn + * Returns true if @insn is a breakpoint instruction. + * + * This function is needed for

Re: [PATCH] powerpc/uprobes: teach uprobes to ignore gdb breakpoints

On 03/21, Ananth N Mavinakayanahalli wrote: On Wed, Mar 20, 2013 at 05:06:44PM +0100, Oleg Nesterov wrote: But we did not install UPROBE_SWBP_INSN. Is it fine? I hope yes, just to verify. If not, we need 2 definitions. is_uprobe_insn() should still check insns

Re: [PATCH] powerpc/uprobes: teach uprobes to ignore gdb breakpoints

On 03/21, Ananth N Mavinakayanahalli wrote: ? On Wed, Mar 20, 2013 at 05:07:28PM +0100, Oleg Nesterov wrote: On 03/20, Ananth N Mavinakayanahalli wrote: On Wed, Mar 20, 2013 at 01:43:01PM +0100, Oleg Nesterov wrote: On 03/20, Oleg Nesterov wrote: IOW, if I wasn't clear

Re: [PATCH] powerpc/uprobes: teach uprobes to ignore gdb breakpoints

Hi Ananth, First of all, let me remind that I know nothing about powerpc ;) But iirc we already discussed this a bit, I forgot the details but still I have some concerns... On 03/20, Ananth N Mavinakayanahalli wrote: GDB uses a variant of the trap instruction that is different from the one

Re: [PATCH] powerpc/uprobes: teach uprobes to ignore gdb breakpoints

On 03/20, Oleg Nesterov wrote: But we did not install UPROBE_SWBP_INSN. Is it fine? I hope yes, just to verify. If not, we need 2 definitions. is_uprobe_insn() should still check insns == UPROBE_SWBP_INSN, and is_swbp_insn() should check is_trap(). And I am just curious, could you explain

Re: [PATCH] powerpc/uprobes: teach uprobes to ignore gdb breakpoints

On 03/20, Ananth N Mavinakayanahalli wrote: On Wed, Mar 20, 2013 at 01:26:39PM +0100, Oleg Nesterov wrote: But, at the same time, is the new definition fine for verify_opcode()? IOW, powerpc has another is_trap() insn(s) used by gdb, lets denote it X. X != UPROBE_SWBP_INSN. Suppose

Re: [PATCH] powerpc/uprobes: teach uprobes to ignore gdb breakpoints

On 03/20, Ananth N Mavinakayanahalli wrote: On Wed, Mar 20, 2013 at 01:43:01PM +0100, Oleg Nesterov wrote: On 03/20, Oleg Nesterov wrote: But we did not install UPROBE_SWBP_INSN. Is it fine? I hope yes, just to verify. If not, we need 2 definitions. is_uprobe_insn() should still check

Re: [PATCH] lglock: add read-preference local-global rwlock

On 03/05, Lai Jiangshan wrote: On 03/03/13 01:06, Oleg Nesterov wrote: On 03/02, Michel Lespinasse wrote: My version would be slower if it needs to take the slow path in a reentrant way, but I'm not sure it matters either :) I'd say, this doesn't matter at all, simply because

Re: [PATCH V2] lglock: add read-preference local-global rwlock

On 03/05, Lai Jiangshan wrote: On 03/03/13 01:20, Oleg Nesterov wrote: On 03/02, Lai Jiangshan wrote: +void lg_rwlock_local_read_unlock(struct lgrwlock *lgrw) +{ + switch (__this_cpu_read(*lgrw-reader_refcnt)) { + case 1: + __this_cpu_write(*lgrw-reader_refcnt, 0

Re: [PATCH V2] lglock: add read-preference local-global rwlock

On 03/02, Oleg Nesterov wrote: On 03/02, Lai Jiangshan wrote: +void lg_rwlock_local_read_unlock(struct lgrwlock *lgrw) +{ + switch (__this_cpu_read(*lgrw-reader_refcnt)) { + case 1: + __this_cpu_write(*lgrw-reader_refcnt, 0); + lg_local_unlock(lgrw-lglock

Re: [PATCH] lglock: add read-preference local-global rwlock

On 03/02, Lai Jiangshan wrote: On 02/03/13 02:28, Oleg Nesterov wrote: Lai, I didn't read this discussion except the code posted by Michel. I'll try to read this patch carefully later, but I'd like to ask a couple of questions. This version looks more complex than Michel's, why? Just

Re: [PATCH] lglock: add read-preference local-global rwlock

On 03/02, Michel Lespinasse wrote: My version would be slower if it needs to take the slow path in a reentrant way, but I'm not sure it matters either :) I'd say, this doesn't matter at all, simply because this can only happen if we race with the active writer. Oleg.

Re: [PATCH V2] lglock: add read-preference local-global rwlock

On 03/02, Lai Jiangshan wrote: +void lg_rwlock_local_read_unlock(struct lgrwlock *lgrw) +{ + switch (__this_cpu_read(*lgrw-reader_refcnt)) { + case 1: + __this_cpu_write(*lgrw-reader_refcnt, 0); + lg_local_unlock(lgrw-lglock); + return; +

Re: [PATCH] lglock: add read-preference local-global rwlock

Lai, I didn't read this discussion except the code posted by Michel. I'll try to read this patch carefully later, but I'd like to ask a couple of questions. This version looks more complex than Michel's, why? Just curious, I am trying to understand what I missed. See

Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks

On 02/28, Michel Lespinasse wrote: On Thu, Feb 28, 2013 at 3:25 AM, Oleg Nesterov o...@redhat.com wrote: On 02/27, Michel Lespinasse wrote: +void lg_rwlock_local_read_lock(struct lgrwlock *lgrw) +{ + preempt_disable(); + + if (__this_cpu_read(*lgrw-local_refcnt

Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks

On 02/28, Oleg Nesterov wrote: On 02/28, Michel Lespinasse wrote: On Thu, Feb 28, 2013 at 3:25 AM, Oleg Nesterov o...@redhat.com wrote: On 02/27, Michel Lespinasse wrote: +void lg_rwlock_local_read_lock(struct lgrwlock *lgrw) +{ + preempt_disable

Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks

On 02/27, Michel Lespinasse wrote: +void lg_rwlock_local_read_lock(struct lgrwlock *lgrw) +{ + preempt_disable(); + + if (__this_cpu_read(*lgrw-local_refcnt) || + arch_spin_trylock(this_cpu_ptr(lgrw-lglock-lock))) { + __this_cpu_inc(*lgrw-local_refcnt);

Re: [PATCH v5 04/45] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks

On 02/08, Paul E. McKenney wrote: On Tue, Jan 22, 2013 at 01:03:53PM +0530, Srivatsa S. Bhat wrote: void percpu_read_unlock(struct percpu_rwlock *pcpu_rwlock) { - read_unlock(pcpu_rwlock-global_rwlock); We need an smp_mb() here to keep the critical section ordered before the

Re: [PATCH v5 05/45] percpu_rwlock: Make percpu-rwlocks IRQ-safe, optimally

only one cosmetic nit... On 01/22, Srivatsa S. Bhat wrote: +#define READER_PRESENT (1UL 16) +#define READER_REFCNT_MASK (READER_PRESENT - 1) + #define reader_uses_percpu_refcnt(pcpu_rwlock, cpu) \

Re: [PATCH v5 04/45] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks

On 02/11, Srivatsa S. Bhat wrote: On 02/10/2013 11:36 PM, Oleg Nesterov wrote: +static void announce_writer_inactive(struct percpu_rwlock *pcpu_rwlock) +{ + unsigned int cpu; + + drop_writer_signal(pcpu_rwlock, smp_processor_id()); Why do we drop ourselves twice? More

Re: [PATCH v5 04/45] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks

On 02/11, Srivatsa S. Bhat wrote: On 02/09/2013 04:40 AM, Paul E. McKenney wrote: +static void announce_writer_inactive(struct percpu_rwlock *pcpu_rwlock) +{ + unsigned int cpu; + + drop_writer_signal(pcpu_rwlock, smp_processor_id()); Why do we drop ourselves twice? More to the

Re: [PATCH v5 01/45] percpu_rwlock: Introduce the global reader-writer lock backend

On 01/23, Michel Lespinasse wrote: On Tue, Jan 22, 2013 at 11:32 AM, Steven Rostedt rost...@goodmis.org wrote: I thought global locks are now fair. That is, a reader will block if a writer is waiting. Hence, the above should deadlock on the current rwlock_t types. I believe you are

Re: [PATCH v4 2/2] powerpc: Uprobes port to powerpc

On 08/23, Benjamin Herrenschmidt wrote: On Thu, 2012-08-23 at 11:02 +0530, Srikar Dronamraju wrote: insn is updated/accessed in the arch independent code. Size of uprobe_opcode_t could be different for different archs. uprobe_opcode_t represents the size of the smallest breakpoint

Re: [PATCH v4 2/2] powerpc: Uprobes port to powerpc

On 08/22, Ananth N Mavinakayanahalli wrote: +int arch_uprobe_analyze_insn(struct arch_uprobe *auprobe, struct mm_struct *mm, unsigned long addr) +{ + unsigned int insn; + + if (addr 0x03) + return -EINVAL; + + memcpy(insn, auprobe-insn, MAX_UINSN_BYTES); +

Re: [PATCH v3 2/2] powerpc: Uprobes port to powerpc

On 08/21, Ananth N Mavinakayanahalli wrote: On Fri, Aug 17, 2012 at 05:00:31PM +0200, Oleg Nesterov wrote: We should also take care of the in-memory copy, in case gdb had inserted a breakpoint at the same location, right? gdb (or even the application itself) and uprobes can

Re: [PATCH v3 2/2] powerpc: Uprobes port to powerpc

On 08/17, Ananth N Mavinakayanahalli wrote: On Thu, Aug 16, 2012 at 05:21:12PM +0200, Oleg Nesterov wrote: Hmm, I am not sure. is_swbp_insn(insn), as it is used in the arch agnostic code, should only return true if insn == UPROBE_SWBP_INSN (just in case, this logic needs more fixes

Re: [PATCH v3 2/2] powerpc: Uprobes port to powerpc

On 08/16, Ananth N Mavinakayanahalli wrote: On Thu, Aug 16, 2012 at 07:41:53AM +1000, Benjamin Herrenschmidt wrote: On Wed, 2012-08-15 at 18:59 +0200, Oleg Nesterov wrote: On 07/26, Ananth N Mavinakayanahalli wrote: From: Ananth N Mavinakayanahalli ana...@in.ibm.com

Re: [PATCH v3 2/2] powerpc: Uprobes port to powerpc

On 07/26, Ananth N Mavinakayanahalli wrote: From: Ananth N Mavinakayanahalli ana...@in.ibm.com This is the port of uprobes to powerpc. Usage is similar to x86. I am just curious why this series was ignored by powerpc maintainers... Of course I can not review this code, I know nothing about

Re: [PATCH v2 1/2] uprobes: Pass probed vaddr to arch_uprobe_analyze_insn()

On 06/14, Srikar Dronamraju wrote: * Oleg Nesterov o...@redhat.com [2012-06-13 21:15:19]: For example. Suppose there is some instruction in /lib64/libc.so which is valid for 64-bit, but not for 32-bit. Suppose that a 32-bit application does mmap(/lib64/libc.so, PROT_EXEC). How

Re: [PATCH v2 1/2] uprobes: Pass probed vaddr to arch_uprobe_analyze_insn()

On 06/12, Oleg Nesterov wrote: On 06/12, Srikar Dronamraju wrote: Note also that we should move this !UPROBE_COPY_INSN from install_breakpoint() to somewhere near alloc_uprobe(). This code is called only once, it looks a bit strange to use the random mm (the first mm

Re: Q: a_ops-readpage() struct file

On 06/13, Peter Zijlstra wrote: On Mon, 2012-06-11 at 21:09 +0200, Oleg Nesterov wrote: Stupid question. I'm afraid the answer is no but I'll ask anyway. Is it safe to pass filp == NULL to mapping-readpage()? In fact I do not understand why it needs struct file* and I do not see any

Re: [PATCH v2 1/2] uprobes: Pass probed vaddr to arch_uprobe_analyze_insn()

On 06/12, Srikar Dronamraju wrote: Note also that we should move this !UPROBE_COPY_INSN from install_breakpoint() to somewhere near alloc_uprobe(). This code is called only once, it looks a bit strange to use the random mm (the first mm vma_prio_tree_foreach() finds) and its mapping to

Re: [PATCH v2 1/2] uprobes: Pass probed vaddr to arch_uprobe_analyze_insn()

Ananth, Srikar, I think the patch is correct and I am sorry for nit-picking, this is really minor. But, On 06/08, Ananth N Mavinakayanahalli wrote: Changes in V2: Pass (unsigned long)addr Well, IMHO, this is confusing. First of all, why do we have this addr or even vaddr? It should not

Q: a_ops-readpage() struct file

On 06/11, Oleg Nesterov wrote: Note also that we should move this !UPROBE_COPY_INSN from install_breakpoint() to somewhere near alloc_uprobe(). The main problem is, uprobe_register() doesn't have struct file for read_mapping_page(). Stupid question. I'm afraid the answer is no but I'll ask

Re: [PATCH 1/2] uprobes: Pass probed vaddr to arch_uprobe_analyze_insn()

On 06/06, Ananth N Mavinakayanahalli wrote: From: Ananth N Mavinakayanahalli ana...@in.ibm.com On RISC architectures like powerpc, instructions are fixed size. Instruction analysis on such platforms is just a matter of (insn % 4). Pass the vaddr at which the uprobe is to be inserted so that

Re: [PATCH -v2] Audit: push audit success and retcode into arch ptrace.h

On 06/07, Eric Paris wrote: On Tue, 2011-06-07 at 19:19 +0200, Oleg Nesterov wrote: With or without this patch, can't we call audit_syscall_exit() twice if there is something else in _TIF_WORK_SYSCALL_EXIT mask apart from SYSCALL_AUDIT ? First time it is called from asm, then from

Re: [PATCH -v2] Audit: push audit success and retcode into arch ptrace.h

On 06/08, Eric Paris wrote: On Wed, 2011-06-08 at 18:36 +0200, Oleg Nesterov wrote: And I guess, all CONFIG_AUDITSYSCALL code in entry.S is only needed to microoptimize the case when TIF_SYSCALL_AUDIT is the only reason for the slow path. I wonder if it really makes the measureble

Re: [PATCH -v2] Audit: push audit success and retcode into arch ptrace.h

On 06/08, Oleg Nesterov wrote: OK. Thanks a lot Eric for your explanations. Yes. but may I ask another one? Shouldn't copy_process()-audit_alloc(tsk) path do clear_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT) if it doesn't set tsk-audit_context? I can be easily wrong, but afaics otherwise

Re: [PATCH -v2] Audit: push audit success and retcode into arch ptrace.h

On 06/03, Eric Paris wrote: The audit system previously expected arches calling to audit_syscall_exit to supply as arguments if the syscall was a success and what the return code was. Audit also provides a helper AUDITSC_RESULT which was supposed to simplify things by converting from