2014-10-09 08:04:29

by Daniel Wagner

[permalink] [raw]
Subject: [PATCH v0] scripts/gdb: Use a generator instead of iterator for task list

I am trying to debug something here and gave your patches a spin.
I found a small issue when trying to access threads instead only
processes.

See you next week in Duesseldorf!

Daniel Wagner (1):
scripts/gdb: Use a generator instead of iterator for task list

scripts/gdb/linux/tasks.py | 50 +++++++++++++++++++---------------------------
1 file changed, 20 insertions(+), 30 deletions(-)

--
1.9.3


2014-10-09 08:04:19

by Daniel Wagner

[permalink] [raw]
Subject: [PATCH v0] scripts/gdb: Use a generator instead of iterator for task list

The iterator does not return any task_struct from the thread_group list
because the first condition in the 'if not t or ...' will only be the
first time None.

Instead of keeping track of the state ourself in the next() function,
we fall back using Python's generator.

Signed-off-by: Daniel Wagner <[email protected]>
Cc: Jan Kiszka <[email protected]>
---
scripts/gdb/linux/tasks.py | 50 +++++++++++++++++++---------------------------
1 file changed, 20 insertions(+), 30 deletions(-)

diff --git a/scripts/gdb/linux/tasks.py b/scripts/gdb/linux/tasks.py
index 0008e75..e2037d9 100644
--- a/scripts/gdb/linux/tasks.py
+++ b/scripts/gdb/linux/tasks.py
@@ -18,38 +18,28 @@ from linux import utils

task_type = utils.CachedType("struct task_struct")

-
-class TaskList:
- def __init__(self):
- global task_type
- self.task_ptr_type = task_type.get_type().pointer()
- self.init_task = gdb.parse_and_eval("init_task")
- self.curr_group = self.init_task.address
- self.curr_task = None
-
- def __iter__(self):
- return self
-
- def __next__(self):
- t = self.curr_task
- if not t or t == self.curr_group:
- self.curr_group = \
- utils.container_of(self.curr_group['tasks']['next'],
- self.task_ptr_type, "tasks")
- if self.curr_group == self.init_task.address:
- raise StopIteration
- t = self.curr_task = self.curr_group
- else:
- self.curr_task = \
- utils.container_of(t['thread_group']['next'],
- self.task_ptr_type, "thread_group")
- return t
-
- def next(self):
- return self.__next__()
+def task_lists():
+ global task_type
+ task_ptr_type = task_type.get_type().pointer()
+ init_task = gdb.parse_and_eval("init_task").address
+ t = g = init_task
+
+ while True:
+ while True:
+ yield t
+
+ t = utils.container_of(t['thread_group']['next'],
+ task_ptr_type, "thread_group")
+ if t == g:
+ break
+
+ t = g = utils.container_of(g['tasks']['next'],
+ task_ptr_type, "tasks")
+ if t == init_task:
+ return

def get_task_by_pid(pid):
- for task in TaskList():
+ for task in task_lists():
if int(task['pid']) == pid:
return task
return None
--
1.9.3

2014-10-16 12:31:33

by Jan Kiszka

[permalink] [raw]
Subject: Re: [PATCH v0] scripts/gdb: Use a generator instead of iterator for task list

On 2014-10-09 10:03, Daniel Wagner wrote:
> The iterator does not return any task_struct from the thread_group list
> because the first condition in the 'if not t or ...' will only be the
> first time None.
>
> Instead of keeping track of the state ourself in the next() function,
> we fall back using Python's generator.
>
> Signed-off-by: Daniel Wagner <[email protected]>
> Cc: Jan Kiszka <[email protected]>
> ---
> scripts/gdb/linux/tasks.py | 50 +++++++++++++++++++---------------------------
> 1 file changed, 20 insertions(+), 30 deletions(-)
>
> diff --git a/scripts/gdb/linux/tasks.py b/scripts/gdb/linux/tasks.py
> index 0008e75..e2037d9 100644
> --- a/scripts/gdb/linux/tasks.py
> +++ b/scripts/gdb/linux/tasks.py
> @@ -18,38 +18,28 @@ from linux import utils
>
> task_type = utils.CachedType("struct task_struct")
>
> -
> -class TaskList:
> - def __init__(self):
> - global task_type
> - self.task_ptr_type = task_type.get_type().pointer()
> - self.init_task = gdb.parse_and_eval("init_task")
> - self.curr_group = self.init_task.address
> - self.curr_task = None
> -
> - def __iter__(self):
> - return self
> -
> - def __next__(self):
> - t = self.curr_task
> - if not t or t == self.curr_group:
> - self.curr_group = \
> - utils.container_of(self.curr_group['tasks']['next'],
> - self.task_ptr_type, "tasks")
> - if self.curr_group == self.init_task.address:
> - raise StopIteration
> - t = self.curr_task = self.curr_group
> - else:
> - self.curr_task = \
> - utils.container_of(t['thread_group']['next'],
> - self.task_ptr_type, "thread_group")
> - return t
> -
> - def next(self):
> - return self.__next__()
> +def task_lists():
> + global task_type
> + task_ptr_type = task_type.get_type().pointer()
> + init_task = gdb.parse_and_eval("init_task").address
> + t = g = init_task
> +
> + while True:
> + while True:
> + yield t
> +
> + t = utils.container_of(t['thread_group']['next'],
> + task_ptr_type, "thread_group")
> + if t == g:
> + break
> +
> + t = g = utils.container_of(g['tasks']['next'],
> + task_ptr_type, "tasks")
> + if t == init_task:
> + return
>
> def get_task_by_pid(pid):
> - for task in TaskList():
> + for task in task_lists():
> if int(task['pid']) == pid:
> return task
> return None
>

Thanks for the fix (and the nice simplification), added to my queue.
I'll repost the whole series ASAP.

Jan

--
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux