[PATCH] trace-cruncher: Add API to set tracing CPU affinity

[PATCH] trace-cruncher: Add API to set tracing CPU affinity

[Steven Rostedt]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Add a set_affinity API that can let the user set what CPUs to enable
tracing on.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
This depends on the new libtracefs API found here:

  https://patchwork.kernel.org/project/linux-trace-devel/patch/20211217180450.12d3524b@gandalf.local.home/

This can be worked out later if you have comments. And I'll keep
the VMWare tag, as I did write while still working at VMware. I figured
that my last patch as a VMware employee would be my first patch
to trace-cruncher ;-)

 examples/test-affinity.py |  5 ++++
 src/ftracepy-utils.c      | 58 +++++++++++++++++++++++++++++++++++++++
 src/ftracepy-utils.h      |  3 ++
 src/ftracepy.c            |  5 ++++
 4 files changed, 71 insertions(+)
 create mode 100755 examples/test-affinity.py

diff --git a/examples/test-affinity.py b/examples/test-affinity.py
new file mode 100755
index 0000000..1619931
--- /dev/null
+++ b/examples/test-affinity.py
@@ -0,0 +1,5 @@
+#!/usr/bin/env python3
+
+import tracecruncher.ftracepy as ft
+
+ft.set_affinity(cpus=["0-1","3"]);
diff --git a/src/ftracepy-utils.c b/src/ftracepy-utils.c
index a735d88..ab39cf1 100644
--- a/src/ftracepy-utils.c
+++ b/src/ftracepy-utils.c
@@ -2394,6 +2394,64 @@ PyObject *PyFtrace_hist(PyObject *self, PyObject *args,
 	return NULL;
 }
 
+PyObject *PyFtrace_set_affinity(PyObject *self, PyObject *args,
+						PyObject *kwargs)
+{
+	struct tracefs_instance *instance;
+	static char *kwlist[] = {"cpus", "instance", NULL};
+	PyObject *py_cpus;
+	PyObject *py_inst = NULL;
+	const char *cpu_str;
+	struct trace_seq seq;
+	int ret;
+
+	if (!PyArg_ParseTupleAndKeywords(args,
+					 kwargs,
+					 "O|O",
+					 kwlist,
+					 &py_cpus,
+					 &py_inst)) {
+		return NULL;
+	}
+
+	trace_seq_init(&seq);
+
+	if (PyUnicode_Check(py_cpus)) {
+		cpu_str = (const char *)PyUnicode_DATA(py_cpus);
+		if (trace_seq_puts(&seq, cpu_str) < 0)
+			goto err_seq;
+	} else if (PyList_CheckExact(py_cpus)) {
+		int i, n = PyList_Size(py_cpus);
+
+		for (i = 0; i < n; ++i) {
+			cpu_str = str_from_list(py_cpus, i);
+			if (i)
+				trace_seq_putc(&seq, ',');
+			if (trace_seq_puts(&seq, cpu_str) < 0)
+				goto err_seq;
+		}
+	}
+
+	if (!get_optional_instance(py_inst, &instance))
+		goto err;
+
+	trace_seq_terminate(&seq);
+	ret = tracefs_instance_set_affinity(instance, seq.buffer);
+	if (ret < 0) {
+		PyErr_SetString(TRACECRUNCHER_ERROR,
+				"Invalid \"cpus\" argument.");
+		goto err;
+	}
+
+	Py_RETURN_NONE;
+err_seq:
+	PyErr_SetString(TRACECRUNCHER_ERROR,
+			"Internal processing string error.");
+err:
+	trace_seq_destroy(&seq);
+	return NULL;
+}
+
 PyObject *PyFtrace_set_ftrace_loglevel(PyObject *self, PyObject *args,
 						       PyObject *kwargs)
 {
diff --git a/src/ftracepy-utils.h b/src/ftracepy-utils.h
index d09c8bf..67f67ab 100644
--- a/src/ftracepy-utils.h
+++ b/src/ftracepy-utils.h
@@ -208,6 +208,9 @@ PyObject *PyFtrace_register_kretprobe(PyObject *self, PyObject *args,
 PyObject *PyFtrace_hist(PyObject *self, PyObject *args,
 					PyObject *kwargs);
 
+PyObject *PyFtrace_set_affinity(PyObject *self, PyObject *args,
+						PyObject *kwargs);
+
 PyObject *PyFtrace_set_ftrace_loglevel(PyObject *self, PyObject *args,
 						       PyObject *kwargs);
 
diff --git a/src/ftracepy.c b/src/ftracepy.c
index b270b71..634f357 100644
--- a/src/ftracepy.c
+++ b/src/ftracepy.c
@@ -377,6 +377,11 @@ static PyMethodDef ftracepy_methods[] = {
 	 METH_VARARGS | METH_KEYWORDS,
 	 "Define a histogram."
 	},
+	{"set_affinity",
+	 (PyCFunction) PyFtrace_set_affinity,
+	 METH_VARARGS | METH_KEYWORDS,
+	 "Find an existing ftrace instance."
+	},
 	{"set_ftrace_loglevel",
 	 (PyCFunction) PyFtrace_set_ftrace_loglevel,
 	 METH_VARARGS | METH_KEYWORDS,
-- 
2.31.1

Re: [PATCH] trace-cruncher: Add API to set tracing CPU affinity

[Yordan Karadzhov]

Hi Steven

On 18.12.21 г. 1:26 ч., Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> 
> Add a set_affinity API that can let the user set what CPUs to enable
> tracing on.

For the sake of completeness we will need APIs to 'clear' and 'get' the CPU affinity as well.

> 
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> This depends on the new libtracefs API found here:
> 
>    https://patchwork.kernel.org/project/linux-trace-devel/patch/20211217180450.12d3524b@gandalf.local.home/
> 
> This can be worked out later if you have comments. And I'll keep
> the VMWare tag, as I did write while still working at VMware. I figured
> that my last patch as a VMware employee would be my first patch
> to trace-cruncher ;-)

This is really symbolic and means a lot for me!

> 
>   examples/test-affinity.py |  5 ++++
>   src/ftracepy-utils.c      | 58 +++++++++++++++++++++++++++++++++++++++
>   src/ftracepy-utils.h      |  3 ++
>   src/ftracepy.c            |  5 ++++
>   4 files changed, 71 insertions(+)
>   create mode 100755 examples/test-affinity.py
> 
> diff --git a/examples/test-affinity.py b/examples/test-affinity.py
> new file mode 100755
> index 0000000..1619931
> --- /dev/null
> +++ b/examples/test-affinity.py
> @@ -0,0 +1,5 @@
> +#!/usr/bin/env python3
> +
> +import tracecruncher.ftracepy as ft
> +
> +ft.set_affinity(cpus=["0-1","3"]);
> diff --git a/src/ftracepy-utils.c b/src/ftracepy-utils.c
> index a735d88..ab39cf1 100644
> --- a/src/ftracepy-utils.c
> +++ b/src/ftracepy-utils.c
> @@ -2394,6 +2394,64 @@ PyObject *PyFtrace_hist(PyObject *self, PyObject *args,
>   	return NULL;
>   }
>   
> +PyObject *PyFtrace_set_affinity(PyObject *self, PyObject *args,
> +						PyObject *kwargs)
> +{
> +	struct tracefs_instance *instance;
> +	static char *kwlist[] = {"cpus", "instance", NULL};
> +	PyObject *py_cpus;
> +	PyObject *py_inst = NULL;
> +	const char *cpu_str;
> +	struct trace_seq seq;
> +	int ret;
> +
> +	if (!PyArg_ParseTupleAndKeywords(args,
> +					 kwargs,
> +					 "O|O",
> +					 kwlist,
> +					 &py_cpus,
> +					 &py_inst)) {
> +		return NULL;
> +	}
> +
> +	trace_seq_init(&seq);

There is a global trace_seq object that can be used here. Also you have to check for error.
Perhaps having:

	if (!init_print_seq())
		return NULL;

> +
> +	if (PyUnicode_Check(py_cpus)) {
> +		cpu_str = (const char *)PyUnicode_DATA(py_cpus);
> +		if (trace_seq_puts(&seq, cpu_str) < 0)
> +			goto err_seq;
> +	} else if (PyList_CheckExact(py_cpus)) {
> +		int i, n = PyList_Size(py_cpus);
> +
> +		for (i = 0; i < n; ++i) {
> +			cpu_str = str_from_list(py_cpus, i);
> +			if (i)
> +				trace_seq_putc(&seq, ',');
> +			if (trace_seq_puts(&seq, cpu_str) < 0)
> +				goto err_seq;
> +		}
> +	}
If py_cpus is neither PyUnicode nor PyList, we have to print error and return NULL.

> +
> +	if (!get_optional_instance(py_inst, &instance))
> +		goto err;
> +
> +	trace_seq_terminate(&seq);
> +	ret = tracefs_instance_set_affinity(instance, seq.buffer);
> +	if (ret < 0) {
> +		PyErr_SetString(TRACECRUNCHER_ERROR,
> +				"Invalid \"cpus\" argument.");
> +		goto err;
> +	}
> +
> +	Py_RETURN_NONE;
> +err_seq:
> +	PyErr_SetString(TRACECRUNCHER_ERROR,
> +			"Internal processing string error.");
> +err:
> +	trace_seq_destroy(&seq);
> +	return NULL;
> +}
> +
>   PyObject *PyFtrace_set_ftrace_loglevel(PyObject *self, PyObject *args,
>   						       PyObject *kwargs)
>   {
> diff --git a/src/ftracepy-utils.h b/src/ftracepy-utils.h
> index d09c8bf..67f67ab 100644
> --- a/src/ftracepy-utils.h
> +++ b/src/ftracepy-utils.h
> @@ -208,6 +208,9 @@ PyObject *PyFtrace_register_kretprobe(PyObject *self, PyObject *args,
>   PyObject *PyFtrace_hist(PyObject *self, PyObject *args,
>   					PyObject *kwargs);
>   
> +PyObject *PyFtrace_set_affinity(PyObject *self, PyObject *args,
> +						PyObject *kwargs);
> +
>   PyObject *PyFtrace_set_ftrace_loglevel(PyObject *self, PyObject *args,
>   						       PyObject *kwargs);
>   
> diff --git a/src/ftracepy.c b/src/ftracepy.c
> index b270b71..634f357 100644
> --- a/src/ftracepy.c
> +++ b/src/ftracepy.c
> @@ -377,6 +377,11 @@ static PyMethodDef ftracepy_methods[] = {
>   	 METH_VARARGS | METH_KEYWORDS,
>   	 "Define a histogram."
>   	},
> +	{"set_affinity",
> +	 (PyCFunction) PyFtrace_set_affinity,
> +	 METH_VARARGS | METH_KEYWORDS,
> +	 "Find an existing ftrace instance."
A short description of the method has to be provided here.

Thanks!
Yordan


> +	},
>   	{"set_ftrace_loglevel",
>   	 (PyCFunction) PyFtrace_set_ftrace_loglevel,
>   	 METH_VARARGS | METH_KEYWORDS,
> 

Re: [PATCH] trace-cruncher: Add API to set tracing CPU affinity

[Steven Rostedt]

On Mon, 10 Jan 2022 15:32:16 +0200
Yordan Karadzhov <y.karadz@gmail.com> wrote:

> Hi Steven
> 
> On 18.12.21 г. 1:26 ч., Steven Rostedt wrote:
> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> > 
> > Add a set_affinity API that can let the user set what CPUs to enable
> > tracing on.  
> 
> For the sake of completeness we will need APIs to 'clear' and 'get' the CPU affinity as well.

For "clear" you mean to avoid a CPU(s), not clear the mask, right?

> 
> > 
> > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> > ---
> > This depends on the new libtracefs API found here:
> > 
> >    https://patchwork.kernel.org/project/linux-trace-devel/patch/20211217180450.12d3524b@gandalf.local.home/
> > 
> > This can be worked out later if you have comments. And I'll keep
> > the VMWare tag, as I did write while still working at VMware. I figured
> > that my last patch as a VMware employee would be my first patch
> > to trace-cruncher ;-)  
> 
> This is really symbolic and means a lot for me!

I thought you may appreciate it ;-)

> 
> > 
> >   examples/test-affinity.py |  5 ++++
> >   src/ftracepy-utils.c      | 58 +++++++++++++++++++++++++++++++++++++++
> >   src/ftracepy-utils.h      |  3 ++
> >   src/ftracepy.c            |  5 ++++
> >   4 files changed, 71 insertions(+)
> >   create mode 100755 examples/test-affinity.py
> > 
> > diff --git a/examples/test-affinity.py b/examples/test-affinity.py
> > new file mode 100755
> > index 0000000..1619931
> > --- /dev/null
> > +++ b/examples/test-affinity.py
> > @@ -0,0 +1,5 @@
> > +#!/usr/bin/env python3
> > +
> > +import tracecruncher.ftracepy as ft
> > +
> > +ft.set_affinity(cpus=["0-1","3"]);
> > diff --git a/src/ftracepy-utils.c b/src/ftracepy-utils.c
> > index a735d88..ab39cf1 100644
> > --- a/src/ftracepy-utils.c
> > +++ b/src/ftracepy-utils.c
> > @@ -2394,6 +2394,64 @@ PyObject *PyFtrace_hist(PyObject *self, PyObject *args,
> >   	return NULL;
> >   }
> >   
> > +PyObject *PyFtrace_set_affinity(PyObject *self, PyObject *args,
> > +						PyObject *kwargs)
> > +{
> > +	struct tracefs_instance *instance;
> > +	static char *kwlist[] = {"cpus", "instance", NULL};
> > +	PyObject *py_cpus;
> > +	PyObject *py_inst = NULL;
> > +	const char *cpu_str;
> > +	struct trace_seq seq;
> > +	int ret;
> > +
> > +	if (!PyArg_ParseTupleAndKeywords(args,
> > +					 kwargs,
> > +					 "O|O",
> > +					 kwlist,
> > +					 &py_cpus,
> > +					 &py_inst)) {
> > +		return NULL;
> > +	}
> > +
> > +	trace_seq_init(&seq);  
> 
> There is a global trace_seq object that can be used here. Also you have to check for error.
> Perhaps having:


Don't we need mutex protection if we use a global object?



> 
> 	if (!init_print_seq())
> 		return NULL;

Sure. I left it out as trace_seq has an internal state that won't (at least
it should not) allow anything to be added if it didn't allocate or
something else failed. Thus, you could do all the work and just test at the
end.

> 
> > +
> > +	if (PyUnicode_Check(py_cpus)) {
> > +		cpu_str = (const char *)PyUnicode_DATA(py_cpus);
> > +		if (trace_seq_puts(&seq, cpu_str) < 0)
> > +			goto err_seq;
> > +	} else if (PyList_CheckExact(py_cpus)) {
> > +		int i, n = PyList_Size(py_cpus);
> > +
> > +		for (i = 0; i < n; ++i) {
> > +			cpu_str = str_from_list(py_cpus, i);
> > +			if (i)
> > +				trace_seq_putc(&seq, ',');
> > +			if (trace_seq_puts(&seq, cpu_str) < 0)
> > +				goto err_seq;
> > +		}
> > +	}  
> If py_cpus is neither PyUnicode nor PyList, we have to print error and return NULL.

Yeah, I wasn't sure how much python would detect this. That can be added.

> 
> > +
> > +	if (!get_optional_instance(py_inst, &instance))
> > +		goto err;
> > +
> > +	trace_seq_terminate(&seq);
> > +	ret = tracefs_instance_set_affinity(instance, seq.buffer);
> > +	if (ret < 0) {
> > +		PyErr_SetString(TRACECRUNCHER_ERROR,
> > +				"Invalid \"cpus\" argument.");
> > +		goto err;
> > +	}
> > +
> > +	Py_RETURN_NONE;
> > +err_seq:
> > +	PyErr_SetString(TRACECRUNCHER_ERROR,
> > +			"Internal processing string error.");
> > +err:
> > +	trace_seq_destroy(&seq);
> > +	return NULL;
> > +}
> > +
> >   PyObject *PyFtrace_set_ftrace_loglevel(PyObject *self, PyObject *args,
> >   						       PyObject *kwargs)
> >   {
> > diff --git a/src/ftracepy-utils.h b/src/ftracepy-utils.h
> > index d09c8bf..67f67ab 100644
> > --- a/src/ftracepy-utils.h
> > +++ b/src/ftracepy-utils.h
> > @@ -208,6 +208,9 @@ PyObject *PyFtrace_register_kretprobe(PyObject *self, PyObject *args,
> >   PyObject *PyFtrace_hist(PyObject *self, PyObject *args,
> >   					PyObject *kwargs);
> >   
> > +PyObject *PyFtrace_set_affinity(PyObject *self, PyObject *args,
> > +						PyObject *kwargs);
> > +
> >   PyObject *PyFtrace_set_ftrace_loglevel(PyObject *self, PyObject *args,
> >   						       PyObject *kwargs);
> >   
> > diff --git a/src/ftracepy.c b/src/ftracepy.c
> > index b270b71..634f357 100644
> > --- a/src/ftracepy.c
> > +++ b/src/ftracepy.c
> > @@ -377,6 +377,11 @@ static PyMethodDef ftracepy_methods[] = {
> >   	 METH_VARARGS | METH_KEYWORDS,
> >   	 "Define a histogram."
> >   	},
> > +	{"set_affinity",
> > +	 (PyCFunction) PyFtrace_set_affinity,
> > +	 METH_VARARGS | METH_KEYWORDS,
> > +	 "Find an existing ftrace instance."  
> A short description of the method has to be provided here.

OK, will do in v2.

Cheers,

-- Steve

Re: [PATCH] trace-cruncher: Add API to set tracing CPU affinity

[Yordan Karadzhov]

On 10.01.22 г. 17:20 ч., Steven Rostedt wrote:
> On Mon, 10 Jan 2022 15:32:16 +0200
> Yordan Karadzhov <y.karadz@gmail.com> wrote:
> 
>> Hi Steven
>>
>> On 18.12.21 г. 1:26 ч., Steven Rostedt wrote:
>>> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>>>
>>> Add a set_affinity API that can let the user set what CPUs to enable
>>> tracing on.
>>
>> For the sake of completeness we will need APIs to 'clear' and 'get' the CPU affinity as well.
> 
> For "clear" you mean to avoid a CPU(s), not clear the mask, right?

I mean clear the mask. This will be a very simple method. The only argument will be the instance. And inside the method 
you just call 'tracefs_instance_file_clear()'. The 'get' method can return the bit mask of the CPUs.

> 
>>
>>>
>>> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>>> ---
>>> This depends on the new libtracefs API found here:
>>>
>>>     https://patchwork.kernel.org/project/linux-trace-devel/patch/20211217180450.12d3524b@gandalf.local.home/
>>>
>>> This can be worked out later if you have comments. And I'll keep
>>> the VMWare tag, as I did write while still working at VMware. I figured
>>> that my last patch as a VMware employee would be my first patch
>>> to trace-cruncher ;-)
>>
>> This is really symbolic and means a lot for me!
> 
> I thought you may appreciate it ;-)
I really do appreciate this :-)

> 
>>
>>>
>>>    examples/test-affinity.py |  5 ++++
>>>    src/ftracepy-utils.c      | 58 +++++++++++++++++++++++++++++++++++++++
>>>    src/ftracepy-utils.h      |  3 ++
>>>    src/ftracepy.c            |  5 ++++
>>>    4 files changed, 71 insertions(+)
>>>    create mode 100755 examples/test-affinity.py
>>>
>>> diff --git a/examples/test-affinity.py b/examples/test-affinity.py
>>> new file mode 100755
>>> index 0000000..1619931
>>> --- /dev/null
>>> +++ b/examples/test-affinity.py
>>> @@ -0,0 +1,5 @@
>>> +#!/usr/bin/env python3
>>> +
>>> +import tracecruncher.ftracepy as ft
>>> +
>>> +ft.set_affinity(cpus=["0-1","3"]);
>>> diff --git a/src/ftracepy-utils.c b/src/ftracepy-utils.c
>>> index a735d88..ab39cf1 100644
>>> --- a/src/ftracepy-utils.c
>>> +++ b/src/ftracepy-utils.c
>>> @@ -2394,6 +2394,64 @@ PyObject *PyFtrace_hist(PyObject *self, PyObject *args,
>>>    	return NULL;
>>>    }
>>>    
>>> +PyObject *PyFtrace_set_affinity(PyObject *self, PyObject *args,
>>> +						PyObject *kwargs)
>>> +{
>>> +	struct tracefs_instance *instance;
>>> +	static char *kwlist[] = {"cpus", "instance", NULL};
>>> +	PyObject *py_cpus;
>>> +	PyObject *py_inst = NULL;
>>> +	const char *cpu_str;
>>> +	struct trace_seq seq;
>>> +	int ret;
>>> +
>>> +	if (!PyArg_ParseTupleAndKeywords(args,
>>> +					 kwargs,
>>> +					 "O|O",
>>> +					 kwlist,
>>> +					 &py_cpus,
>>> +					 &py_inst)) {
>>> +		return NULL;
>>> +	}
>>> +
>>> +	trace_seq_init(&seq);
>>
>> There is a global trace_seq object that can be used here. Also you have to check for error.
>> Perhaps having:
> 
> 
> Don't we need mutex protection if we use a global object?

As far as I know Python is intrinsically single threaded.
Only one thread can execute Python code at once.
Multiprocessing is the only way to parallelize the execution.

> 
> 
> 
>>
>> 	if (!init_print_seq())
>> 		return NULL;
> 
> Sure. I left it out as trace_seq has an internal state that won't (at least
> it should not) allow anything to be added if it didn't allocate or
> something else failed. Thus, you could do all the work and just test at the
> end.
> 
>>
>>> +
>>> +	if (PyUnicode_Check(py_cpus)) {
>>> +		cpu_str = (const char *)PyUnicode_DATA(py_cpus);
>>> +		if (trace_seq_puts(&seq, cpu_str) < 0)
>>> +			goto err_seq;
>>> +	} else if (PyList_CheckExact(py_cpus)) {
>>> +		int i, n = PyList_Size(py_cpus);
>>> +
>>> +		for (i = 0; i < n; ++i) {
>>> +			cpu_str = str_from_list(py_cpus, i);
>>> +			if (i)
>>> +				trace_seq_putc(&seq, ',');
>>> +			if (trace_seq_puts(&seq, cpu_str) < 0)
>>> +				goto err_seq;
>>> +		}
>>> +	}
>> If py_cpus is neither PyUnicode nor PyList, we have to print error and return NULL.
> 
> Yeah, I wasn't sure how much python would detect this. That can be added.

The C code here is everything that will happen when you call the method in Python.
No black magic ;-)

Actually there is one magical use case and this is when the method returns NULL.
In this case Python will do for you the error propagation and the cleanup.

> 
>>
>>> +
>>> +	if (!get_optional_instance(py_inst, &instance))
>>> +		goto err;
>>> +
>>> +	trace_seq_terminate(&seq);
>>> +	ret = tracefs_instance_set_affinity(instance, seq.buffer);
>>> +	if (ret < 0) {
>>> +		PyErr_SetString(TRACECRUNCHER_ERROR,
>>> +				"Invalid \"cpus\" argument.");
>>> +		goto err;
>>> +	}
>>> +
>>> +	Py_RETURN_NONE;
>>> +err_seq:
>>> +	PyErr_SetString(TRACECRUNCHER_ERROR,
>>> +			"Internal processing string error.");
>>> +err:
>>> +	trace_seq_destroy(&seq);
>>> +	return NULL;
>>> +}
>>> +
>>>    PyObject *PyFtrace_set_ftrace_loglevel(PyObject *self, PyObject *args,
>>>    						       PyObject *kwargs)
>>>    {
>>> diff --git a/src/ftracepy-utils.h b/src/ftracepy-utils.h
>>> index d09c8bf..67f67ab 100644
>>> --- a/src/ftracepy-utils.h
>>> +++ b/src/ftracepy-utils.h
>>> @@ -208,6 +208,9 @@ PyObject *PyFtrace_register_kretprobe(PyObject *self, PyObject *args,
>>>    PyObject *PyFtrace_hist(PyObject *self, PyObject *args,
>>>    					PyObject *kwargs);
>>>    
>>> +PyObject *PyFtrace_set_affinity(PyObject *self, PyObject *args,
>>> +						PyObject *kwargs);
>>> +
>>>    PyObject *PyFtrace_set_ftrace_loglevel(PyObject *self, PyObject *args,
>>>    						       PyObject *kwargs);
>>>    
>>> diff --git a/src/ftracepy.c b/src/ftracepy.c
>>> index b270b71..634f357 100644
>>> --- a/src/ftracepy.c
>>> +++ b/src/ftracepy.c
>>> @@ -377,6 +377,11 @@ static PyMethodDef ftracepy_methods[] = {
>>>    	 METH_VARARGS | METH_KEYWORDS,
>>>    	 "Define a histogram."
>>>    	},
>>> +	{"set_affinity",
>>> +	 (PyCFunction) PyFtrace_set_affinity,
>>> +	 METH_VARARGS | METH_KEYWORDS,
>>> +	 "Find an existing ftrace instance."
>> A short description of the method has to be provided here.
> 
> OK, will do in v2.

Thanks!
Yordan


> 
> Cheers,
> 
> -- Steve
> 

Re: [PATCH] trace-cruncher: Add API to set tracing CPU affinity

[Steven Rostedt]

On Tue, 11 Jan 2022 15:07:06 +0200
Yordan Karadzhov <y.karadz@gmail.com> wrote:

> On 10.01.22 г. 17:20 ч., Steven Rostedt wrote:
> > On Mon, 10 Jan 2022 15:32:16 +0200
> > Yordan Karadzhov <y.karadz@gmail.com> wrote:
> >   
> >> Hi Steven
> >>
> >> On 18.12.21 г. 1:26 ч., Steven Rostedt wrote:  
> >>> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> >>>
> >>> Add a set_affinity API that can let the user set what CPUs to enable
> >>> tracing on.  
> >>
> >> For the sake of completeness we will need APIs to 'clear' and 'get' the CPU affinity as well.  
> > 
> > For "clear" you mean to avoid a CPU(s), not clear the mask, right?  
> 
> I mean clear the mask. This will be a very simple method. The only argument will be the instance. And inside the method 
> you just call 'tracefs_instance_file_clear()'. The 'get' method can return the bit mask of the CPUs.
> 

But why would anyone clear it? It doesn't make any sense. This mask is
the CPUs that tracing is allowed on. By clearing it, you basically
stopped all tracing. If anything, it would confuse people. I honestly
do not know of a single use case (besides testing the tracing
infrastructure) for clearing the entire mask.

It would be similar to clearing the affinity mask for a task or
interrupt. If anything, that could harm the system. I'm not even sure
it's allowed (it would fail if tried).

Now, I could see clearing specific CPUs, and that would be useful. Say
you want to trace the entire system, but you dedicated a CPU for the
tracing application. It would make sense to clear the CPUs that your
tracing application is on and leave all others intact. That way the
tracing application doesn't affect the results as much.


> >>>    
> >>> +PyObject *PyFtrace_set_affinity(PyObject *self, PyObject *args,
> >>> +						PyObject *kwargs)
> >>> +{
> >>> +	struct tracefs_instance *instance;
> >>> +	static char *kwlist[] = {"cpus", "instance", NULL};
> >>> +	PyObject *py_cpus;
> >>> +	PyObject *py_inst = NULL;
> >>> +	const char *cpu_str;
> >>> +	struct trace_seq seq;
> >>> +	int ret;
> >>> +
> >>> +	if (!PyArg_ParseTupleAndKeywords(args,
> >>> +					 kwargs,
> >>> +					 "O|O",
> >>> +					 kwlist,
> >>> +					 &py_cpus,
> >>> +					 &py_inst)) {
> >>> +		return NULL;
> >>> +	}
> >>> +
> >>> +	trace_seq_init(&seq);  
> >>
> >> There is a global trace_seq object that can be used here. Also you have to check for error.
> >> Perhaps having:  
> > 
> > 
> > Don't we need mutex protection if we use a global object?  
> 
> As far as I know Python is intrinsically single threaded.
> Only one thread can execute Python code at once.

Are you sure about that? A quick search produced this:

  https://realpython.com/intro-to-python-threading/

> Multiprocessing is the only way to parallelize the execution.
> 
> > 
> > 
> >   
> >>
> >> 	if (!init_print_seq())
> >> 		return NULL;  
> > 
> > Sure. I left it out as trace_seq has an internal state that won't (at least
> > it should not) allow anything to be added if it didn't allocate or
> > something else failed. Thus, you could do all the work and just test at the
> > end.
> >   
> >>  
> >>> +
> >>> +	if (PyUnicode_Check(py_cpus)) {
> >>> +		cpu_str = (const char *)PyUnicode_DATA(py_cpus);
> >>> +		if (trace_seq_puts(&seq, cpu_str) < 0)
> >>> +			goto err_seq;
> >>> +	} else if (PyList_CheckExact(py_cpus)) {
> >>> +		int i, n = PyList_Size(py_cpus);
> >>> +
> >>> +		for (i = 0; i < n; ++i) {
> >>> +			cpu_str = str_from_list(py_cpus, i);
> >>> +			if (i)
> >>> +				trace_seq_putc(&seq, ',');
> >>> +			if (trace_seq_puts(&seq, cpu_str) < 0)
> >>> +				goto err_seq;
> >>> +		}
> >>> +	}  
> >> If py_cpus is neither PyUnicode nor PyList, we have to print error and return NULL.  
> > 
> > Yeah, I wasn't sure how much python would detect this. That can be added.  
> 
> The C code here is everything that will happen when you call the method in Python.
> No black magic ;-)

OK.

-- Steve

> 
> Actually there is one magical use case and this is when the method returns NULL.
> In this case Python will do for you the error propagation and the cleanup.
> 
> >  

Re: [PATCH] trace-cruncher: Add API to set tracing CPU affinity

[Yordan Karadzhov]

On 11.01.22 г. 16:00 ч., Steven Rostedt wrote:
> On Tue, 11 Jan 2022 15:07:06 +0200
> Yordan Karadzhov<y.karadz@gmail.com>  wrote:
> 
>> On 10.01.22 г. 17:20 ч., Steven Rostedt wrote:
>>> On Mon, 10 Jan 2022 15:32:16 +0200
>>> Yordan Karadzhov<y.karadz@gmail.com>  wrote:
>>>    
>>>> Hi Steven
>>>>
>>>> On 18.12.21 г. 1:26 ч., Steven Rostedt wrote:
>>>>> From: "Steven Rostedt (VMware)"<rostedt@goodmis.org>
>>>>>
>>>>> Add a set_affinity API that can let the user set what CPUs to enable
>>>>> tracing on.
>>>> For the sake of completeness we will need APIs to 'clear' and 'get' the CPU affinity as well.
>>> For "clear" you mean to avoid a CPU(s), not clear the mask, right?
>> I mean clear the mask. This will be a very simple method. The only argument will be the instance. And inside the method
>> you just call 'tracefs_instance_file_clear()'. The 'get' method can return the bit mask of the CPUs.
>>
> But why would anyone clear it? It doesn't make any sense. This mask is
> the CPUs that tracing is allowed on. By clearing it, you basically
> stopped all tracing. If anything, it would confuse people. I honestly
> do not know of a single use case (besides testing the tracing
> infrastructure) for clearing the entire mask.
> 
> It would be similar to clearing the affinity mask for a task or
> interrupt. If anything, that could harm the system. I'm not even sure
> it's allowed (it would fail if tried).
> 
> Now, I could see clearing specific CPUs, and that would be useful. Say
> you want to trace the entire system, but you dedicated a CPU for the
> tracing application. It would make sense to clear the CPUs that your
> tracing application is on and leave all others intact. That way the
> tracing application doesn't affect the results as much.
> 

Sorry about my confusion, you are right.

Clearing the mask or setting all bits to 0 makes no sense. What I wanted to have is the exact opposite. I want an API 
that restores the default configuration (sets all bits to 1).
Maybe 'reset' is a better name in this case.

Thanks!
Yordan

Re: [PATCH] trace-cruncher: Add API to set tracing CPU affinity

[Yordan Karadzhov]

On 11.01.22 г. 16:00 ч., Steven Rostedt wrote:
>>>>> +PyObject *PyFtrace_set_affinity(PyObject *self, PyObject *args,
>>>>> +						PyObject *kwargs)
>>>>> +{
>>>>> +	struct tracefs_instance *instance;
>>>>> +	static char *kwlist[] = {"cpus", "instance", NULL};
>>>>> +	PyObject *py_cpus;
>>>>> +	PyObject *py_inst = NULL;
>>>>> +	const char *cpu_str;
>>>>> +	struct trace_seq seq;
>>>>> +	int ret;
>>>>> +
>>>>> +	if (!PyArg_ParseTupleAndKeywords(args,
>>>>> +					 kwargs,
>>>>> +					 "O|O",
>>>>> +					 kwlist,
>>>>> +					 &py_cpus,
>>>>> +					 &py_inst)) {
>>>>> +		return NULL;
>>>>> +	}
>>>>> +
>>>>> +	trace_seq_init(&seq);
>>>> There is a global trace_seq object that can be used here. Also you have to check for error.
>>>> Perhaps having:
>>>
>>> Don't we need mutex protection if we use a global object?
>> As far as I know Python is intrinsically single threaded.
>> Only one thread can execute Python code at once.
> Are you sure about that? A quick search produced this:
> 
>    https://realpython.com/intro-to-python-threading/
> 

Hmm, I am not 100% sure, but I still think this is the case.
In your link, if you go down a bit you have

"If you’re not sure if you want to use Python threading, asyncio, or multiprocessing, then you can check out Speed Up 
Your Python Program With Concurrency."

where " Speed Up Your Python Program With Concurrency" is a link to here:
https://realpython.com/python-concurrency/

and here if you scroll down to the first table, you will see that multiprocessing is the only real way to run Python on 
multiple CPUs.

You can have a look also here
https://docs.python.org/3/library/threading.html

"In CPython, due to the Global Interpreter Lock, only one thread can execute Python code at once (even though certain 
performance-oriented libraries might overcome this limitation). If you want your application to make better use of the 
computational resources of multi-core machines, you are advised to use multiprocessing or 
concurrent.futures.ProcessPoolExecutor. However, threading is still an appropriate model if you want to run multiple 
I/O-bound tasks simultaneously."

Thanks!
Y.


>> Multiprocessing is the only way to parallelize the execution.
>>
>>>
>>>    
>>>> 	if (!init_print_seq())
>>>> 		return NULL;

Re: [PATCH] trace-cruncher: Add API to set tracing CPU affinity

[Steven Rostedt]

On Tue, 11 Jan 2022 16:38:01 +0200
Yordan Karadzhov <y.karadz@gmail.com> wrote:

> Hmm, I am not 100% sure, but I still think this is the case.
> In your link, if you go down a bit you have
> 
> "If you’re not sure if you want to use Python threading, asyncio, or multiprocessing, then you can check out Speed Up 
> Your Python Program With Concurrency."
> 
> where " Speed Up Your Python Program With Concurrency" is a link to here:
> https://realpython.com/python-concurrency/
> 
> and here if you scroll down to the first table, you will see that multiprocessing is the only real way to run Python on 
> multiple CPUs.
> 
> You can have a look also here
> https://docs.python.org/3/library/threading.html
> 
> "In CPython, due to the Global Interpreter Lock, only one thread can execute Python code at once (even though certain 
> performance-oriented libraries might overcome this limitation). If you want your application to make better use of the 
> computational resources of multi-core machines, you are advised to use multiprocessing or 
> concurrent.futures.ProcessPoolExecutor. However, threading is still an appropriate model if you want to run multiple 
> I/O-bound tasks simultaneously."

Well, it doesn't affect the API so we can always change it in the future.

-- Steve