[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 10:06:00 +0800
On Wed, Nov 6, 2013 at 12:22 AM, Qingping Hou <qingpinghou@xxxxxxxxx> wrote:
> On Tue 05 Nov 2013 02:23:47 AM EST, Jovi Zhangwei wrote:
>> It seems we put the function prototype by table,
>> and parse table in ktapvm to get function prototype data structure.
>> I know this just is a workaround, since cdef parser not finished yet.
>>
>> This remind me maybe it's not time to merge this code right now,
>> there have some code change( in table.c) just for workaround cdef
>> handling, it means we have to revise it later when compiler cdef parser
>> finish, that's not good for code maintenance.
>>
>
> Oh, yes, we did not expect you to merge the code at this point. That's
> why the first mail we send out is not in a patch format and only
> contains a link to github branch. It's just a POC and not very usable
> right now (it's hard to find useful kernel function that only deal with
> int and long ;p)
>
> We just want to get some feedback on the early stage design :)
>
I misunderstood it :)
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.
- 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?
Thanks for your effort.
Jovi
Other related posts:
