| Message ID | 20210712123242.223500-4-y.karadz@gmail.com (mailing list archive) |
|---|---|
| State | Deferred |
| Headers | show |
| Series | [1/4] trace-cruncher: Add set_ftrace_loglevel() | expand |
On Mon, 12 Jul 2021 15:32:42 +0300 "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote: > @@ -1617,8 +1622,9 @@ PyObject *PyFtrace_register_kretprobe(PyObject *self, PyObject *args, > return NULL; > } > > - if (!store_new_kprobe(event)) > - return NULL; > + if (!detached) > + if (!store_new_kprobe(event)) > + return NULL; I wonder if it would be more consistent and helpful if we created a structure for kprobes like we do for instances, and this way we can save the "detached" field in that structure, and not free it on destroy. This way, even though the kprobes are detached it may still be needed in the future to list all kprobes that were created by the application, regardless if they are detached or not. -- Steve
On 26.07.21 г. 12:07, Yordan Karadzhov (VMware) wrote: > > > On 23.07.21 г. 0:24, Steven Rostedt wrote: >> On Mon, 12 Jul 2021 15:32:42 +0300 >> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote: >> >> >>> @@ -1617,8 +1622,9 @@ PyObject *PyFtrace_register_kretprobe(PyObject *self, PyObject *args, >>> return NULL; >>> } >>> - if (!store_new_kprobe(event)) >>> - return NULL; >>> + if (!detached) >>> + if (!store_new_kprobe(event)) >>> + return NULL; >> >> I wonder if it would be more consistent and helpful if we created a >> structure for kprobes like we do for instances, and this way we can >> save the "detached" field in that structure, and not free it on destroy. >> >> This way, even though the kprobes are detached it may still be needed >> in the future to list all kprobes that were created by the application, >> regardless if they are detached or not. > > Yes, this make sense. > > BTW we should discuss what is the best data structure to store the kprobes that were created. In the case of the > instances we really need to use a binary tree, because we identify the instances only by name (string) so we are forced > to do a lot of searches. Initially I assumed that we will need to do searches for kprobes as well. However, so far we > only do listing. > > Maybe, we should switch from tree to linked list? This will greatly simplify everything. > I am deferring this patch together with the previous one ("Allow for detachable instances") for possible redesign. Thanks, Yordan > Thanks! > Yordan > > >> >> -- Steve >>
diff --git a/src/ftracepy-utils.c b/src/ftracepy-utils.c index d061817..605f7eb 100644 --- a/src/ftracepy-utils.c +++ b/src/ftracepy-utils.c @@ -1570,16 +1570,18 @@ bool store_new_kprobe(const char *event) PyObject *PyFtrace_register_kprobe(PyObject *self, PyObject *args, PyObject *kwargs) { - static char *kwlist[] = {"event", "function", "probe", NULL}; + static char *kwlist[] = {"event", "function", "probe", "detached", NULL}; const char *event, *function, *probe; + int detached = false; if (!PyArg_ParseTupleAndKeywords(args, kwargs, - "sss", + "sss|p", kwlist, &event, &function, - &probe)) { + &probe, + &detached)) { return NULL; } @@ -1589,8 +1591,9 @@ PyObject *PyFtrace_register_kprobe(PyObject *self, PyObject *args, return NULL; } - if (!store_new_kprobe(event)) - return NULL; + if (!detached) + if (!store_new_kprobe(event)) + return NULL; Py_RETURN_NONE; } @@ -1598,16 +1601,18 @@ PyObject *PyFtrace_register_kprobe(PyObject *self, PyObject *args, PyObject *PyFtrace_register_kretprobe(PyObject *self, PyObject *args, PyObject *kwargs) { - static char *kwlist[] = {"event", "function", "probe", NULL}; + static char *kwlist[] = {"event", "function", "probe", "detached", NULL}; const char *event, *function, *probe = "$retval"; + int detached = false; if (!PyArg_ParseTupleAndKeywords(args, kwargs, - "ss|s", + "ss|sp", kwlist, &event, &function, - &probe)) { + &probe, + &detached)) { return NULL; } @@ -1617,8 +1622,9 @@ PyObject *PyFtrace_register_kretprobe(PyObject *self, PyObject *args, return NULL; } - if (!store_new_kprobe(event)) - return NULL; + if (!detached) + if (!store_new_kprobe(event)) + return NULL; Py_RETURN_NONE; } diff --git a/tests/1_unit/test_01_ftracepy_unit.py b/tests/1_unit/test_01_ftracepy_unit.py index 0d62da2..a7a6dec 100644 --- a/tests/1_unit/test_01_ftracepy_unit.py +++ b/tests/1_unit/test_01_ftracepy_unit.py @@ -479,6 +479,15 @@ class KprobeTestCase(unittest.TestCase): all_kprobes = ft.registered_kprobes() self.assertEqual(len(all_kprobes), 0) + ft.register_kprobe(event=evt1, function=evt1_func, + probe=evt1_prove, detached=True) + ft.unregister_kprobe(event='ALL') + all_kprobes = ft.registered_kprobes() + self.assertEqual(len(all_kprobes), 1) + ft.unregister_kprobe(event='ALL', force=True) + all_kprobes = ft.registered_kprobes() + self.assertEqual(len(all_kprobes), 0) + def test_enable_kprobe(self): evt1 = 'mkdir' diff --git a/tracecruncher/ft_utils.py b/tracecruncher/ft_utils.py index 8c245b1..a976885 100644 --- a/tracecruncher/ft_utils.py +++ b/tracecruncher/ft_utils.py @@ -171,12 +171,12 @@ class kprobe(kprobe_base): offset=offset, size=size) - def register(self): + def register(self, detached=False): """ Register this probe to Ftrace. """ probe = ' '.join('{!s}={!s}'.format(key,val) for (key, val) in self.fields.items()) - ft.register_kprobe(event=self.name, function=self.func, probe=probe); + ft.register_kprobe(event=self.name, function=self.func, probe=probe, detached=detached); self.evt_id = find_event_id(system=ft.tc_event_system(), event=self.name) @@ -203,8 +203,8 @@ class kretval_probe(kprobe_base): """ super().__init__(name, func) - def register(self): + def register(self, detached=False): """ Register this probe to Ftrace. """ - ft.register_kprobe(event=self.name, function=self.func); + ft.register_kprobe(event=self.name, function=self.func, detached=detached); self.evt_id = find_event_id(system=ft.tc_event_system(), event=self.name)
If a kprobe or kretprobe is created with "detached=True", the Python module is no longer responsible for destroying it when exiting. It is therefore up to the user free it explicitly, or to keep it active. Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com> --- src/ftracepy-utils.c | 26 ++++++++++++++++---------- tests/1_unit/test_01_ftracepy_unit.py | 9 +++++++++ tracecruncher/ft_utils.py | 8 ++++---- 3 files changed, 29 insertions(+), 14 deletions(-)