Re: [PATCH 15/28] ktap: add built-in functions and library (runtime/lib_*.c)

[Jovi Zhangwei]

On Mon, Mar 31, 2014 at 9:13 PM, Andi Kleen <andi@xxxxxxxxxxxxxx> wrote:
> On Mon, Mar 31, 2014 at 10:01:04AM +0800, Jovi Zhangwei wrote:
>> On Sun, Mar 30, 2014 at 8:58 AM, Andi Kleen <andi@xxxxxxxxxxxxxx> wrote:
>> >> Maybe in future, after ktap support "include" or "require" to
>> >> import user defined library in userspace.
>> >
>> > Can't you just have some hardcoded standard script for now that is
>> > always appeneded and provides these functions?
>> >
>> Maybe it's fine to hardcoded just for now.
>>
>> Since we are agreed on review userspace part in another schedule,
>> so I will remove this ansi library from kernel part in next version.
>>
>> Thanks for this suggestion.
>
> Please do the following for the next version:
>
> - Don't repost with all the TODOs regarding changing other
> kernel parts. Just fix them.
>
Sure.

> - As others pointed out elsewhere it's too big and full featured right
> now. Please find ways to define a useful "core ktap" for now with less
> features. This could be dropping library parts or dropping some of the
> probe types or some parts of the language. These can then be later phased
> in over time. For the library it may make sense to add some module
> interface and keep parts of it external for now?
>
I will remove some library and raw tracing interface(designed for benchmark)

> - Please run as much test content as you have with all the kernel debug
> options to automatically find bugs. I would at least two runs one with
> lockdep/lock debugging/preempt debugging/slab/page debugging etc.
> and another with kmemleak (that is exclusive with some other options)
> Some of these checks may have false positives, but the messages
> should be all analyzed at least, and false positives commented.
>
ktap runs fine in lockdep/lock debugging/preempt debugging, but not
try to run in slab/page debugging and kmemleak. I will do that.

> - Do similar with the static compile time checks: checkpatch,
> sparse, coccinelle. There will be likely a lot more false positives
> here, so it may not be feasible to check all, but should at least
> eyeball the output to see if there are some obvious problems.
> For sparse it would be also good to annotate the user ioctl
> parts with __user and carefully look at the warnings there,
> as that is a common problem area.
>
Sure.

I will send out new version once we make agreement on ktap
upstream which discussing in another thread.

Thanks for these comments.

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