Re: [PATCH 1/9] trace-cruncher: Refactor the part that wraps ftrace

[Yordan Karadzhov <y.karadz@xxxxxxxxx>]

Hi Steven,

Thanks a lot for the review!

I will add all your fixes in v2.


On 21.04.21 г. 5:13, Steven Rostedt wrote:
> > +}
> > +
> > +static void start_tracing(struct tracefs_instance *instance,
> > +			  char **argv, char **env)
> > +{
> > +	if(tracefs_option_enable(instance, TRACEFS_OPTION_EVENT_FORK) < 0 ||
> > +	   tracefs_option_enable(instance, TRACEFS_OPTION_FUNCTION_FORK) < 0 ||
> > +	   !set_pid(instance, "set_ftrace_pid", getpid()) ||
> > +	   !set_pid(instance, "set_event_pid", getpid()))
> > +		exit(1);
> So trace cruncher will only trace the given process and its children.
> There's no other alternative?

Of course not. This is just a static helper function that is used by the 
trace_shell_process(). It has no exposure to the user of the module.

trace_shell_process() on the other hand is part of the API of the module 
and indeed will trace only one process.

It is just the first method to be implemented. You will see more methods 
for tracing in the incoming patches.

But you are right that I have to rename start_tracing(), so that it 
doesn't sound like something that has a more general use.


Thanks!

Yordan




> -- Steve
>
>
>
> > +
> > +	tracing_ON(instance);
> > +	if (execve(argv[0], argv, env) < 0) {
> > +		PyErr_Format(TFS_ERROR, "Failed to exec \'%s\'",
> > +			     argv[0]);
> > +	}
> > +
> > +	exit(1);
> > +}
> > +
> > +static int callback(struct tep_event *event, struct tep_record *record,
> > +		    int cpu, void *py_func)
> > +{
> > +	record->cpu = cpu; // Remove when the bug in libtracefs is fixed.
> > +
> > +	PyObject *py_tep_event = PyTepEvent_New(event);
> > +	PyObject *py_tep_record = PyTepRecord_New(record);
> > +
> > +	PyObject *arglist = PyTuple_New(2);
> > +	PyTuple_SetItem(arglist, 0, py_tep_event);
> > +	PyTuple_SetItem(arglist, 1, py_tep_record);
> > +
> > +	PyObject_CallObject((PyObject *) py_func, arglist);
> > +
> > +	return 0;
> > +}
> > +
> > +PyObject *PyFtrace_trace_shell_process(PyObject *self, PyObject *args,
> > +						       PyObject *kwargs)
> > +{
> > +	const char *plugin = "__main__", *py_callback = "callback";
> > +	char *process, *instance_name;
> > +	struct tracefs_instance *instance;
> > +	static char *kwlist[] = {"process", "plugin", "callback", "instance", NULL};
> > +	struct tep_handle *tep;
> > +	PyObject *py_func;
> > +	pid_t pid;
> > +
> > +	instance_name = NO_ARG;
> > +	if(!PyArg_ParseTupleAndKeywords(args,
> > +					kwargs,
> > +					"s|sss",
> > +					kwlist,
> > +					&process,
> > +					&plugin,
> > +					&py_callback,
> > +					&instance_name)) {
> > +		return NULL;
> > +	}
> > +
> > +	py_func = get_callback_func(plugin, py_callback);
> > +	if (!py_func)
> > +		return NULL;
> > +
> > +	if (!get_optional_instance(instance_name, &instance))
> > +		return NULL;
> > +
> > +	tep = tracefs_local_events(tracefs_instance_get_dir(instance));
> > +
> > +	char *argv[] = {getenv("SHELL"), "-c", process, NULL};
> > +	char *env[] = {NULL};
> > +
> > +	pid = fork();
> > +	if (pid < 0) {
> > +		PyErr_SetString(TFS_ERROR, "Failed to fork");
> > +		return NULL;
> > +	}
> > +
> > +	if (pid == 0)
> > +		start_tracing(instance, argv, env);
> > +
> > +	do {
> > +		tracefs_iterate_raw_events(tep, instance, NULL, 0, callback, py_func);
> > +	} while (waitpid(pid, NULL, WNOHANG) != pid);
> > +
> > +	Py_RETURN_NONE;
> > +}
> > +