Re: runtime committed to cvs

["Frank Ch. Eigler" <fche at redhat dot com>]

Hi -


hunt wrote:

> I've checked in my current runtime sources to cvs in src/runtime.

Thank you.

> I've built sample probes using it and run a bunch of tests.  Seems
> solid enough to start using. [...]

It's off to a fine start.  Some items to consider:

The hash tables rely greatly on dynamic memory allocation, for nodes
as well as individual string objects.  As mentioned before, this is
not appropriate in the context of an embedded probe that could easily
gobble up GFP_ATOMIC memory.  I believe support for preallocation of
such data, including strings, is critical.  (This may require changes
to its style of passing/returning string values by pointer-reference.)
OOM handling should be left to the caller.

Your tests don't include any mutual exclusion primitives, even though
your hashtable operations are stateful call sequences.  This may work
for now, as the kprobes layer has a Big Lock of its own, but we should
not count on this limitation persisting too long.  You might entertain
treating the runtime as a user-level library, and writing a
multi-threaded testsuite for it to give it a more thorough workout.

It would be interesting to see how your copy-from-user wrapper code
behaves under VM-stressful situations.  Would you mind trying to see
if the page-fault or bad-pointer error situations are politely
handled?  I guess 4g4g is out for now, and context-validity checking
is left to the caller.

The statistics object manipulation should probably be moved out of the
map code, since scalar statistics variables should be possible.  The
map.c code could treat them as opaquely as it does strings.

Sticking "inline" on every function is probably not appropriate.

I am somewhat sorry to see all the runtime type-dispatching fragments
throughout the hashtable code, but I guess that's the only alternative
to the #define-template generics from the other prototype.  I hope it
doesn't get much worse as higher arity tables are considered.


- FChE

Attachment: pgp00000.pgp
Description: PGP signature