[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <53A39A33.6020405@hitachi.com>
Date: Fri, 20 Jun 2014 11:19:31 +0900
From: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Steven Rostedt <rostedt@...dmis.org>,
Josh Poimboeuf <jpoimboe@...hat.com>,
Ingo Molnar <mingo@...nel.org>,
Ananth N Mavinakayanahalli <ananth@...ibm.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH -tip v2 3/3] kprobes: Set IPMODIFY flag only if the probe
can change regs->ip
(2014/06/19 21:34), Namhyung Kim wrote:
> Hi Masami,
>
> 2014-06-17 (화), 11:04 +0000, Masami Hiramatsu:
>> +static int __ftrace_add_filter_ip(struct ftrace_ops *ops, unsigned long ip,
>> + int *ref)
>> +{
>> + int ret;
>> +
>> + /* Try to set given ip to filter */
>> + ret = ftrace_set_filter_ip(ops, ip, 0, 0);
>> + if (ret >= 0) {
>
> Hmm.. this doesn't look comfortable. What not using usual pattern?
>
> if (ret < 0)
> return ret;
>
> This way we can reduce a indent level.
OK, I'll do so :)
>
>
>> + (*ref)++;
>> + if (*ref == 1) {
>> + ret = register_ftrace_function(ops);
>> + if (ret < 0) {
>> + /* Rollback refcounter and filter */
>> + (*ref)--;
>> + ftrace_set_filter_ip(ops, ip, 1, 0);
>> + }
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int __ftrace_remove_filter_ip(struct ftrace_ops *ops, unsigned long ip,
>> + int *ref)
>> +{
>> + int ret = 0;
>> +
>> + (*ref)--;
>> + if (*ref == 0)
>> + ret = unregister_ftrace_function(ops);
>> + if (ret >= 0)
>> + /* Try to remove given ip to filter */
>> + ret = ftrace_set_filter_ip(ops, ip, 1, 0);
>
> I think any failure at this time can be problematic. Since we already
> unregistered the ops but the refcount will get increased again, future
> attemp to register won't enable the ops anymore IMHO.
Agreed.
> I think it'd better just ignoring faiure of filter removal here. We'll
> miss a filter entry but it'll be usable anyway.
>
> What about this?
OK, I'll use your v2 code :)
Thank you for review!
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@...achi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists