Re: [PATCH 3/4] x86: add kprobe-booster to X86_64

[Masami Hiramatsu <mhiramat@redhat.com>]

Harvey Harrison wrote:
> On Tue, 2007-12-18 at 08:50 -0500, Masami Hiramatsu wrote:
>> Hi Harvey,
>>
>> Thank you for cleaning this up.
>>
>> Harvey Harrison wrote:
>>> Subject: [PATCH] x86: kprobes leftover cleanups
>>>
>>> Eliminate __always_inline, all of these static functions are
>>> only called once.  Minor whitespace cleanup.  Eliminate one
>>> supefluous return at end of void function.  Reverse sense of
>>> #ifndef to be #ifdef to show the case only affects X86_32.
>> Unfortunately, to prevent kprobe recursive call, all functions which
>> is called from kprobes must be inlined or have __kprobes.
>> If __always_inline macro still work, I prefer to use it. If not,
>> it must have a __kprobe attribute like as below.
> 
> I thought all static functions that were only called once were
> automatically inlined these days?  Otherwise __always_inline and
> inline are exactly the same in the kernel.

Yes, it will be (not obviously) inlined, currently.
However, IMHO, it is not fail-safe coding.

I think we might better take care of someone who will modify the code
in the future. If they call those functions from other place,
it will not be inlined, and may be placed out of .kprobes.text.
In that case, we can not prevent inserting kprobes in those functions.

Thus, I recommend you to add __kprobes on those functions.
That indicates which functions will be used by kprobes and gives
hints how to write functions which will be called from kprobes.
(And also, it simplifies coding rule.)

Thank you,

> 
> Harvey
> 

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com, masami.hiramatsu.pt@hitachi.com