From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1756040AbdKCVuB (ORCPT <rfc822;w@1wt.eu>);
        Fri, 3 Nov 2017 17:50:01 -0400
Received: from mail.kernel.org ([198.145.29.99]:59700 "EHLO mail.kernel.org"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1755924AbdKCVuA (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 3 Nov 2017 17:50:00 -0400
DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A117C2170D
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: Sat, 4 Nov 2017 06:49:55 +0900
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Jessica Yu <jeyu@kernel.org>
Cc: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>,
        Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>,
        "David S . Miller" <davem@davemloft.net>,
        Ingo Molnar <mingo@kernel.org>, Petr Mladek <pmladek@suse.com>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Joe Lawrence <joe.lawrence@redhat.com>, Jiri Kosina <jikos@kernel.org>,
        Miroslav Benes <mbenes@suse.cz>, Steven Rostedt <rostedt@goodmis.org>,
        live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] kprobes: propagate error from
 arm_kprobe_ftrace()
Message-Id: <20171104064955.acb6e1c4225084fbc2aa954c@kernel.org>
In-Reply-To: <20171102163334.3947-2-jeyu@kernel.org>
References: <20171102163334.3947-1-jeyu@kernel.org>
        <20171102163334.3947-2-jeyu@kernel.org>
X-Mailer: Sylpheed 3.5.1 (GTK+ 2.24.31; x86_64-redhat-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: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Jessica,

Since this is under discussion about ftrace part, I just comment another one.

On Thu,  2 Nov 2017 17:33:33 +0100
Jessica Yu <jeyu@kernel.org> wrote:

[...]
> @@ -1362,9 +1372,14 @@ static int register_aggr_kprobe(struct kprobe *orig_p, struct kprobe *p)
>  
>  	if (ret == 0 && kprobe_disabled(ap) && !kprobe_disabled(p)) {
>  		ap->flags &= ~KPROBE_FLAG_DISABLED;
> -		if (!kprobes_all_disarmed)
> +		if (!kprobes_all_disarmed) {
>  			/* Arm the breakpoint again. */
> -			arm_kprobe(ap);
> +			ret = arm_kprobe(ap);
> +			if (ret) {
> +				ap->flags |= KPROBE_FLAG_DISABLED;
> +				list_del_rcu(&p->list);

You also have to wait rcu here.

> +			}
> +		}
>  	}
>  	return ret;
>  }
[...]
> @@ -2428,16 +2452,26 @@ static void arm_all_kprobes(void)
>  	/* Arming kprobes doesn't optimize kprobe itself */
>  	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
>  		head = &kprobe_table[i];
> -		hlist_for_each_entry_rcu(p, head, hlist)
> -			if (!kprobe_disabled(p))
> -				arm_kprobe(p);
> +		/* Arm all kprobes on a best-effort basis */
> +		hlist_for_each_entry_rcu(p, head, hlist) {
> +			if (!kprobe_disabled(p)) {
> +				err = arm_kprobe(p);
> +				if (err)  {
> +					errors++;
> +					ret = err;
> +				}
> +			}
> +		}
>  	}
>  
> -	printk(KERN_INFO "Kprobes globally enabled\n");
> +	if (errors)
> +		pr_warn("Kprobes globally enabled, but failed to arm %d kprobes\n", errors);

The last "kprobes" should be "probes" because it can include kretprobes :P

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>