| Message ID | 20220518225355.784371-3-ebiederm@xmission.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | ptrace: cleanups and calling do_cldstop with only siglock | expand |
On Wed, May 18, 2022 at 05:53:42PM -0500, Eric W. Biederman wrote: > kdb has a bug that when using the ps command to display a list of > processes, if a process is being debugged the debugger as the parent > process. > > This is silly, and I expect it never comes up in ptractice. As there ^^^^^^^^^ Lol, love the new word :-)
Peter Zijlstra <peterz@infradead.org> writes: > On Wed, May 18, 2022 at 05:53:42PM -0500, Eric W. Biederman wrote: >> kdb has a bug that when using the ps command to display a list of >> processes, if a process is being debugged the debugger as the parent >> process. >> >> This is silly, and I expect it never comes up in ptractice. As there > ^^^^^^^^^ > > Lol, love the new word :-) It wasn't intentional but now I just might have to keep it. Eric
Hi, On Wed, May 18, 2022 at 3:54 PM Eric W. Biederman <ebiederm@xmission.com> wrote: > > kdb has a bug that when using the ps command to display a list of > processes, if a process is being debugged the debugger as the parent > process. > > This is silly, and I expect it never comes up in ptractice. As there > is very little point in using gdb and kdb simultaneously. Update the > code to use real_parent so that it is clear kdb does not want to > display a debugger as the parent of a process. So I would tend to defer to Daniel, but I'm not convinced that the behavior you describe for kdb today _is_ actually silly. If I was in kdb and I was listing processes, I might actually want to see that a process's parent was set to gdb. Presumably that would tell me extra information that might be relevant to my debug session. Personally, I'd rather add an extra piece of information into the list showing the real parent if it's not the same as the parent. Then you're not throwing away information. -Doug
Doug Anderson <dianders@chromium.org> writes: > Hi, > > On Wed, May 18, 2022 at 3:54 PM Eric W. Biederman <ebiederm@xmission.com> wrote: >> >> kdb has a bug that when using the ps command to display a list of >> processes, if a process is being debugged the debugger as the parent >> process. >> >> This is silly, and I expect it never comes up in ptractice. As there >> is very little point in using gdb and kdb simultaneously. Update the >> code to use real_parent so that it is clear kdb does not want to >> display a debugger as the parent of a process. > > So I would tend to defer to Daniel, but I'm not convinced that the > behavior you describe for kdb today _is_ actually silly. > > If I was in kdb and I was listing processes, I might actually want to > see that a process's parent was set to gdb. Presumably that would tell > me extra information that might be relevant to my debug session. > > Personally, I'd rather add an extra piece of information into the list > showing the real parent if it's not the same as the parent. Then > you're not throwing away information. The name of the field is confusing for anyone who isn't intimate with the implementation details. The function getppid returns tsk->real_parent->tgid. If kdb wants information of what the tracer is that is fine, but I recommend putting that information in another field. Given that the original description says give the information that ps gives my sense is that kdb is currently wrong. Especially as it does not give you the actual parentage anywhere. I can certainly be convinced, but I do want some clarity. It looks very attractive to rename task->parent to task->ptracer and leave the field NULL when there is no tracer. Eric
Hi, On Thu, May 19, 2022 at 4:49 PM Eric W. Biederman <ebiederm@xmission.com> wrote: > > Doug Anderson <dianders@chromium.org> writes: > > > Hi, > > > > On Wed, May 18, 2022 at 3:54 PM Eric W. Biederman <ebiederm@xmission.com> wrote: > >> > >> kdb has a bug that when using the ps command to display a list of > >> processes, if a process is being debugged the debugger as the parent > >> process. > >> > >> This is silly, and I expect it never comes up in ptractice. As there > >> is very little point in using gdb and kdb simultaneously. Update the > >> code to use real_parent so that it is clear kdb does not want to > >> display a debugger as the parent of a process. > > > > So I would tend to defer to Daniel, but I'm not convinced that the > > behavior you describe for kdb today _is_ actually silly. > > > > If I was in kdb and I was listing processes, I might actually want to > > see that a process's parent was set to gdb. Presumably that would tell > > me extra information that might be relevant to my debug session. > > > > Personally, I'd rather add an extra piece of information into the list > > showing the real parent if it's not the same as the parent. Then > > you're not throwing away information. > > The name of the field is confusing for anyone who isn't intimate with > the implementation details. The function getppid returns > tsk->real_parent->tgid. > > If kdb wants information of what the tracer is that is fine, but I > recommend putting that information in another field. > > Given that the original description says give the information that ps > gives my sense is that kdb is currently wrong. Especially as it does > not give you the actual parentage anywhere. > > I can certainly be convinced, but I do want some clarity. It looks very > attractive to rename task->parent to task->ptracer and leave the field > NULL when there is no tracer. Fair enough. You can consider my objection rescinded. Presumably, though, you're hoping for an Ack for your patch and you plan to take it with the rest of the series. That's going to need to come from Daniel anyway as he is the actual maintainer. I'm just the peanut gallery. ;-) -Doug
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c index 0852a537dad4..db49f1026eaa 100644 --- a/kernel/debug/kdb/kdb_main.c +++ b/kernel/debug/kdb/kdb_main.c @@ -2306,7 +2306,7 @@ void kdb_ps1(const struct task_struct *p) cpu = kdb_process_cpu(p); kdb_printf("0x%px %8d %8d %d %4d %c 0x%px %c%s\n", - (void *)p, p->pid, p->parent->pid, + (void *)p, p->pid, p->real_parent->pid, kdb_task_has_cpu(p), kdb_process_cpu(p), kdb_task_state_char(p), (void *)(&p->thread),
kdb has a bug that when using the ps command to display a list of processes, if a process is being debugged the debugger as the parent process. This is silly, and I expect it never comes up in ptractice. As there is very little point in using gdb and kdb simultaneously. Update the code to use real_parent so that it is clear kdb does not want to display a debugger as the parent of a process. Cc: Jason Wessel <jason.wessel@windriver.com> Cc: Daniel Thompson <daniel.thompson@linaro.org> Cc: Douglas Anderson <dianders@chromium.org> Fixes: 5d5314d6795f ("kdb: core for kgdb back end (1 of 2)" Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> --- kernel/debug/kdb/kdb_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)