[ktap] Re: [PATCH] FFI support for ktap
- From: Jovi Zhangwei <jovi.zhangwei@xxxxxxxxx>
- To: Qingping Hou <qingpinghou@xxxxxxxxx>, ktap@xxxxxxxxxxxxx
- Date: Thu, 7 Nov 2013 12:41:10 +0800
On Thu, Nov 7, 2013 at 12:07 PM, Qingping Hou <qingpinghou@xxxxxxxxx> wrote:
> On Wed 06 Nov 2013 09:06:00 PM EST, Jovi Zhangwei wrote:
>>
>> For me there just have some small comments which some maybe
>> not need to address now.
>>
>> - C stack
>> It's unacceptable to kmalloc C stack for each ffi call, so might we
>> need to allocate percpu C stack only for FFI call, but I think this is
>> not urgent, we can do it later.
>>
>> - cdata_type definition rename
>> KTAP_CDINT(and others) looks like it's a ktap built-in type, but it's not,
>> KTAP_CDATA is.
>> I would suggest like: KTAP_TCDATA for reclaimable c object, and
>> CTYPE_INT (C_CHAR, and more) for subtype in ktap_cdata.type field,
>> to make it clear for reading.
>>
>
> Agree, how about following structure?
>
> ```
> typedef struct ktap_cdata {
> CommonHeader;
> /* type can be struct, int, function, etc.. */
> int type;
> union {
> csymbol *f;
> int i;
> char c;
> ....
> ctype_ptr p;
> ctype_struct s;
> } u;
> } ktap_cdata;
> ```
>
> It's basically the same as what you had suggested, but scalar cdata
> types (except pointer) are represented using C scalar types.
>
Looks good for me. :)
> We should be able to implement pointer support this week and struct
> support next week.
>
That's great.
>
>> - ffi.sizeof or ffi.new
>> maybe it's time to release these two functions :)
>>
>> - table hack in this patch
>> We know that table hacking in this patch would be a temp solution,
>> so remember to kill it after cdef parser done. :)
>>
>> - CONFIG_KTAP_FFI macro
>> All ffi code should be under guard of CONFIG_KTAP_FFI macro,
>> then enable/disable this macro in Makefile by "make --enable-ffi"
>>
>> What do you think?
>
> The rest looks good to me :)
>
> Another somehow related question, is ktap going to support "require" or
> "include" in the future?
>
Currently I don't see the necessary to support "require" or "include" in ktap,
everything is fine at present, I guess FFI can perform very well even don't
have it ;)
But anyway, if it's requested by more people in future, and can be proven
useful in real case, then we can take consider that.
Thank you.
Jovi
Other related posts:
