Re: Re: [PATCH -tip v2 1/3] kprobes: Introduce nokprobe annotation for non-probe-able functions

2013-11-04 Thread Masami Hiramatsu
(2013/11/01 22:55), Steven Rostedt wrote:
> On Fri, 01 Nov 2013 11:25:32 +
> Masami Hiramatsu  wrote:
> 
>> Instead of __kprobes annotation, introduce 'nokprobe' new annotation
>> to annotate that the function is not probed by kprobes.
>>
>> Previously the '__kprobes' is used just for avoiding probes on
>> kprobes-related functions which will be used from kprobes. However
>> nowadays we use it for prohibiting probing the functions implicitly
>> invoked from kprobes int3 handler, since that causes infinit-loop
>> lockup or sudden reboot. In this case, the annotated functions are
>> not limited in the kprobes-related functions, and __kprobes looks
>> very confusing. (Moreover, actually, most of control-side kprobes
>> functions like as register_kprobes() are safely probed by kprobes)
>>
>> Thus, we decide to introduce 'nokprobe' new annotation. We leave
>> "__kprobes" just for compatibility but it should be replaced or
>> removed eventually.
>>
>> New commits must use 'nokprobe' for this purpose.
>>
>> Signed-off-by: Masami Hiramatsu 
>> Cc: Ingo Molnar 
>> Cc: Ananth N Mavinakayanahalli 
>> Cc: "David S. Miller" 
>> ---
>>  include/linux/compiler.h |6 --
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
>> index 92669cd..173c64e 100644
>> --- a/include/linux/compiler.h
>> +++ b/include/linux/compiler.h
>> @@ -353,8 +353,10 @@ void ftrace_likely_update(struct ftrace_branch_data *f, 
>> int val, int expect);
>>  
>>  /* Ignore/forbid kprobes attach on very low level functions marked by this 
>> attribute: */
>>  #ifdef CONFIG_KPROBES
>> -# define __kprobes  __attribute__((__section__(".kprobes.text")))
>> +# define nokprobe   __attribute__((__section__(".kprobes.text")))
> 
> I wonder if we should have both a __kprobes and nokprobe annotation,
> such that we have:
> 
> # define __kprobes__attribute__((__section__(".kprobes.text")))
> # define nokprobe __attribute__((__section__(".nokprobes.text")))
> 
> Then use __kprobes for the actual kprobes code, and nokprobe for all
> the places that must not be traced by kprobes.

No, actually, we don't need __kprobes anymore. That has started
historically by misunderstanding the problem. kprobes is using
the .kprobes.text only for blacklisting the non probe-able functions.
Thus, eventually it should be renamed .nokprobe.text, not be added.

> It just seems strange to me grouping kprobes code with non kprobes code.

Yeah, so I'd like to cleanup all the __kprobes finally (and classify which
is not needed).

Thank you,

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Re: [PATCH -tip v2 1/3] kprobes: Introduce nokprobe annotation for non-probe-able functions

2013-11-04 Thread Masami Hiramatsu
(2013/11/01 22:55), Steven Rostedt wrote:
 On Fri, 01 Nov 2013 11:25:32 +
 Masami Hiramatsu masami.hiramatsu...@hitachi.com wrote:
 
 Instead of __kprobes annotation, introduce 'nokprobe' new annotation
 to annotate that the function is not probed by kprobes.

 Previously the '__kprobes' is used just for avoiding probes on
 kprobes-related functions which will be used from kprobes. However
 nowadays we use it for prohibiting probing the functions implicitly
 invoked from kprobes int3 handler, since that causes infinit-loop
 lockup or sudden reboot. In this case, the annotated functions are
 not limited in the kprobes-related functions, and __kprobes looks
 very confusing. (Moreover, actually, most of control-side kprobes
 functions like as register_kprobes() are safely probed by kprobes)

 Thus, we decide to introduce 'nokprobe' new annotation. We leave
 __kprobes just for compatibility but it should be replaced or
 removed eventually.

 New commits must use 'nokprobe' for this purpose.

 Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
 Cc: Ingo Molnar mi...@kernel.org
 Cc: Ananth N Mavinakayanahalli ana...@in.ibm.com
 Cc: David S. Miller da...@davemloft.net
 ---
  include/linux/compiler.h |6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

 diff --git a/include/linux/compiler.h b/include/linux/compiler.h
 index 92669cd..173c64e 100644
 --- a/include/linux/compiler.h
 +++ b/include/linux/compiler.h
 @@ -353,8 +353,10 @@ void ftrace_likely_update(struct ftrace_branch_data *f, 
 int val, int expect);
  
  /* Ignore/forbid kprobes attach on very low level functions marked by this 
 attribute: */
  #ifdef CONFIG_KPROBES
 -# define __kprobes  __attribute__((__section__(.kprobes.text)))
 +# define nokprobe   __attribute__((__section__(.kprobes.text)))
 
 I wonder if we should have both a __kprobes and nokprobe annotation,
 such that we have:
 
 # define __kprobes__attribute__((__section__(.kprobes.text)))
 # define nokprobe __attribute__((__section__(.nokprobes.text)))
 
 Then use __kprobes for the actual kprobes code, and nokprobe for all
 the places that must not be traced by kprobes.

No, actually, we don't need __kprobes anymore. That has started
historically by misunderstanding the problem. kprobes is using
the .kprobes.text only for blacklisting the non probe-able functions.
Thus, eventually it should be renamed .nokprobe.text, not be added.

 It just seems strange to me grouping kprobes code with non kprobes code.

Yeah, so I'd like to cleanup all the __kprobes finally (and classify which
is not needed).

Thank you,

-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/