Re: [PATCH tip/master v3] kprobes: extable: Identify kprobes' insn-slots as kernel text area

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Dec 27, 2016 at 03:14:10PM +0900, Masami Hiramatsu wrote:

> diff --git a/kernel/extable.c b/kernel/extable.c
> index e820cce..81c9633 100644
> --- a/kernel/extable.c
> +++ b/kernel/extable.c

> @@ -123,7 +126,11 @@ int kernel_text_address(unsigned long addr)
>  		return 1;
>  	if (is_module_text_address(addr))
>  		return 1;
> -	return is_ftrace_trampoline(addr);
> +	if (is_ftrace_trampoline(addr))
> +		return 1;
> +	if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr))
> +		return 1;
> +	return 0;
>  }

> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index d630954..be41f6d 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c

> +/*
> + * Check given address is on the page of kprobe instruction slots.
> + * This will be used for checking whether the address on a stack
> + * is on a text area or not.
> + */
> +bool __is_insn_slot_addr(struct kprobe_insn_cache *c, unsigned long addr)
> +{
> +	struct kprobe_insn_page *kip;
> +	bool ret = false;
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(kip, &c->pages, list) {
> +		if (addr >= (unsigned long)kip->insns &&
> +		    addr < (unsigned long)kip->insns + PAGE_SIZE) {
> +			ret = true;
> +			break;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	return ret;
> +}

How many entries should one expect on that list? I spend quite a bit of
time reducing the cost of is_module_text_address() a while back and see
that both ftrace (which actually needs this to be fast) and now
kprobes have linear list walks in here.

I'm assuming the ftrace thing to be mostly empty, since I never saw it
on my benchmarks back then, but it is something Steve should look at I
suppose.

Similarly, the changelog here should include some talk about worst case
costs.

FWIW, see commit: 93c2e105f6bc ("module: Optimize __module_address() using a latched RB-tree")