| Message ID | 20210712123242.223500-3-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:41 +0300 "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote: > @@ -600,6 +610,11 @@ PyObject *PyFtrace_create_instance(PyObject *self, PyObject *args, > } > > iw->ptr = instance; > + if (detached) { > + printf("detached instance: %s\n", name); Do we really need to print this? I would think this is up to the calling function to print this or not. -- Steve > + iw->detached = detached; > + } > +
On 23.07.21 г. 0:19, Steven Rostedt wrote: > On Mon, 12 Jul 2021 15:32:41 +0300 > "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote: > >> @@ -600,6 +610,11 @@ PyObject *PyFtrace_create_instance(PyObject *self, PyObject *args, >> } >> >> iw->ptr = instance; >> + if (detached) { >> + printf("detached instance: %s\n", name); > > Do we really need to print this? I would think this is up to the > calling function to print this or not. > I think there must be some notice that it is up to the user to take care about this new instance. This essentially detaches the instance from the garbage collection, which is quite something in the Python world. Maybe we can change the message with something more appropriate? Thanks! Yordan > -- Steve > > >> + iw->detached = detached; >> + } >> +
On Mon, 26 Jul 2021 12:04:51 +0300 "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote: > I think there must be some notice that it is up to the user to take care > about this new instance. This essentially detaches the instance from the > garbage collection, which is quite something in the Python world. > > Maybe we can change the message with something more appropriate? Can't the documentation on the use case be sufficient? I mean, if you have a script that does this, and you give this script to your customers, it would be annoying to have this message appear to your customers where they have no idea what it means. If anything, it should only be printed if you enable some sort of "debug level", like we started doing with the trace libraries. -- Steve
diff --git a/src/ftracepy-utils.c b/src/ftracepy-utils.c index 8f4b50c..d061817 100644 --- a/src/ftracepy-utils.c +++ b/src/ftracepy-utils.c @@ -421,6 +421,12 @@ static PyObject *tfs_list2py_list(char **list) struct instance_wrapper { struct tracefs_instance *ptr; + bool detached; + + /* + * This name will be used only for searching. The actual name of + * the instance is owned by the "tracefs_instance" object. + */ const char *name; }; @@ -451,10 +457,12 @@ void instance_wrapper_free(void *ptr) iw = ptr; if (iw->ptr) { - if (tracefs_instance_destroy(iw->ptr) < 0) - fprintf(stderr, - "\ntfs_error: Failed to destroy instance '%s'.\n", - get_instance_name(iw->ptr)); + if (!iw->detached) { + if (tracefs_instance_destroy(iw->ptr) < 0) + fprintf(stderr, + "\ntfs_error: Failed to destroy instance '%s'.\n", + get_instance_name(iw->ptr)); + } free(iw->ptr); } @@ -569,14 +577,16 @@ PyObject *PyFtrace_create_instance(PyObject *self, PyObject *args, struct tracefs_instance *instance; const char *name = NO_ARG; int tracing_on = true; + int detached = false; + static char *kwlist[] = {"name", "tracing_on", "detached", NULL}; - static char *kwlist[] = {"name", "tracing_on", NULL}; if (!PyArg_ParseTupleAndKeywords(args, kwargs, - "|sp", + "|spp", kwlist, &name, - &tracing_on)) { + &tracing_on, + &detached)) { return NULL; } @@ -600,6 +610,11 @@ PyObject *PyFtrace_create_instance(PyObject *self, PyObject *args, } iw->ptr = instance; + if (detached) { + printf("detached instance: %s\n", name); + iw->detached = detached; + } + iw_ptr = tsearch(iw, &instance_root, instance_compare); if (!iw_ptr || !(*iw_ptr) || !(*iw_ptr)->ptr || strcmp(tracefs_instance_get_name((*iw_ptr)->ptr), name) != 0) {
If the instance 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 | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-)