From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751315AbdI2H3m (ORCPT ); Fri, 29 Sep 2017 03:29:42 -0400 Received: from mail.kernel.org ([198.145.29.99]:36030 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750709AbdI2H3k (ORCPT ); Fri, 29 Sep 2017 03:29:40 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9561921869 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=mhiramat@kernel.org Date: Fri, 29 Sep 2017 00:29:38 -0700 From: Masami Hiramatsu To: Ingo Molnar Cc: mingo@redhat.com, x86@kernel.org, Steven Rostedt , linux-kernel@vger.kernel.org, Peter Zijlstra , Ananth N Mavinakayanahalli , Thomas Gleixner , "H . Peter Anvin" , "Paul E . McKenney" , Alexei Starovoitov , Alexei Starovoitov Subject: Re: [PATCH -tip v3 7/7] kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT Message-Id: <20170929002938.64cb852d6d921ec013d7f052@kernel.org> In-Reply-To: <20170928072220.iv5subwl4u4zcqcp@gmail.com> References: <150581509713.32348.1905525476438163954.stgit@devbox> <150581537997.32348.14125457458719053753.stgit@devbox> <20170928072220.iv5subwl4u4zcqcp@gmail.com> X-Mailer: Sylpheed 3.5.0 (GTK+ 2.24.30; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 28 Sep 2017 09:22:20 +0200 Ingo Molnar wrote: > > * Masami Hiramatsu wrote: > > > To enable jump optimized probe with CONFIG_PREEMPT, use > > synchronize_rcu_tasks() to wait for all tasks preempted > > on trampoline code back on track. > > This sentence does not parse. It's missing a verb, but I'm not sure. Hmm, how about this? Use synchthnize_rcu_tasks() to wait for all tasks preempted on trampoline code back on track so that jump optimized probe can be enabled with CONFIG_PREEMPT. > > > Since the jump optimized kprobes can replace multiple > > instructions, there can be tasks which are preempted > > on the 2nd (or 3rd) instructions. If the kprobe > > replaces those instructions by a jump instruction, > > when those tasks back to the preempted place, it is > > a middle of the jump instruction and causes a kernel > > panic. > > > Again, sentence appears to be missing a verb and also an adjective I think. > Hmm, I couldn't understand, I think you are pointing below sentence, ---- If the kprobe replaces those instructions by a jump instruction, when those tasks back to the preempted place, it is a middle of the jump instruction and causes a kernel panic. ---- Of course "If" and "when" look ugly, but both have verb... > > To avoid such tragedies in advance, kprobe optimizer > > prepare a detour route using normal kprobe (e.g. > > int3 breakpoint on x86), and wait for the tasks which > > is interrrupted on such place by synchronize_sched() > > when CONFIG_PREEMPT=n. > > s/tragedies/mishaps I got it. > > Part after the first comma does not parse. Yeah, some typos, but kprobe optimizer prepares a detour route using normal kprobe () and waits for the tasks, which is interrupted on such place, by synchronize_sched(), when CONFIG_PREEMPT=n. will be able to parsed. ( at least google translate can ...) > > Also the way to refer to kprobes is "kprobes" and "normal kprobes". > Use 'kprobe' only when talking about a specific kprobe instance or such. > You use this correctly later on in the changelog ... > > > If CONFIG_PREEMPT=y, things be more complicated, because > > s/be/are or s/be/get thanks, get is preferred :) > > > such interrupted thread can be preempted (other thread > > can be scheduled in interrupt handler.) So, kprobes > > full stop in the wrong place. > > > optimizer has to wait for those tasks scheduled normally. > > missing verb. kprobe optimizer must wait for those ... will it work? > > > In this case we can use synchronize_rcu_tasks() which > > ensures that all preempted tasks back on track and > > schedule it. > > More careful changelogs please. > > > + * are done. Because optprobe may modify multiple instructions, > > + * there is a chance that the Nth instruction is interrupted. In that > > + * case, running interrupt can return to the Nth byte of jump > > + * instruction. This can be avoided by waiting for returning of > > + * such interrupts, since (until here) the first byte of the optimized > > + * probe is already replaced with normal kprobe (sw breakpoint) and > > + * all threads which reach to the probed address will hit it and > > + * bypass the copied instructions (instead of executing the original.) > > + * With CONFIG_PREEMPT, such interrupts can be preepmted. To wait > > + * for such thread, we will use synchronize_rcu_tasks() which ensures > > + * all preeempted tasks are scheduled normally (not preempted). > > + * So we can ensure there is no threads preempted at probed address. > > What? Interrupts cannot be preempted. Steve, could you correct me if I'm wrong. I thought if the kernel is compiled with CONFIG_PREEMPT=y, even in the kernel, it can be preempted suddenly. It means timer interrupt occurs at kernel path and it yield to new task (=preempt.) Do I miss something? > > Also, "To wait for such threads", or "To wait for such a thread". OK, Thank you, > > Thanks, > > Ingo -- Masami Hiramatsu