Re: RFC for patch to add task_{set,get}_name RPC
On Thu, May 09, 2013 at 07:20:39PM +0200, Samuel Thibault wrote: Thomas Schwinge, le Thu 09 May 2013 18:42:18 +0200, a écrit : Then, to what extent are the proposed new RPCs just a specialized variant of the generic port info RPC that we have been musing about, http://darnassus.sceen.net/~hurd-web/open_issues/translate_fd_or_port_to_file_name/, in particular the log from IRC, freenode, #hurd, 2013-03-07? To me it would make sense to follow the latter route, so be able to store with every generic port some bits of debugging/logging information Indeed. A potential problem with that approach is that, unlike the common case, where the object associated to a port is targetted, the port itself is the object here. The server managing it is then always the kernel, which might interfere with message passing. A simple solution would be to create raw system calls, but I'm not sure that's something we want to do as it can be handy to emulate kernel calls. Another solution is to restrict these RPCs to kernel objects only. -- Richard Braun
Re: RFC for patch to add task_{set,get}_name RPC
On Fri, May 10, 2013 at 05:07:52PM +0200, Richard Braun wrote: On Thu, May 09, 2013 at 07:20:39PM +0200, Samuel Thibault wrote: Thomas Schwinge, le Thu 09 May 2013 18:42:18 +0200, a écrit : Then, to what extent are the proposed new RPCs just a specialized variant of the generic port info RPC that we have been musing about, http://darnassus.sceen.net/~hurd-web/open_issues/translate_fd_or_port_to_file_name/, in particular the log from IRC, freenode, #hurd, 2013-03-07? To me it would make sense to follow the latter route, so be able to store with every generic port some bits of debugging/logging information Indeed. A potential problem with that approach is that, unlike the common case, where the object associated to a port is targetted, the port itself is the object here. The server managing it is then always the kernel, which might interfere with message passing. A simple solution would be to create raw system calls, but I'm not sure that's something we want to do as it can be handy to emulate kernel calls. Another solution is to restrict these RPCs to kernel objects only. Actually, ports are *always* kernel objects, so using a system call would be appropriate IMHO. -- Richard Braun
Re: RFC for patch to add task_{set,get}_name RPC
Samuel Thibault, le Thu 09 May 2013 01:55:56 +0200, a écrit : But the point is that I don't know which mapping I want to see. I just happen to notice from the kernel that a given task does a erroneous thing. From there, how to continue debugging without knowing which userland process was doing that? This is also the reason why the Linux kernel now prints foo[pid]: segfault at 1234 ip 1234 sp 1234 error 6 in foo[1234+1000] it helps a lot to know that a process actually has segfaulted, and which one, within the flurry of processes we now have in our systems. Samuel
Re: RFC for patch to add task_{set,get}_name RPC
Gentlemen, I don't want to get into the political or technical merits but here is an updated patch that works. I will let you decide what to do with it. :) Thanks, Barry diff --git a/include/mach/gnumach.defs b/include/mach/gnumach.defs index 7331334..b26be96 100644 --- a/include/mach/gnumach.defs +++ b/include/mach/gnumach.defs @@ -37,3 +37,17 @@ type vm_cache_statistics_data_t = struct[11] of integer_t; routine vm_cache_statistics( target_task : vm_task_t; out vm_cache_stats : vm_cache_statistics_data_t); + +/* + * Set the name of a task. + */ +routine task_set_name( + task: task_t; + name: task_name_t); + +/* + * Get the name of a task. + */ +routine task_get_name( + task: task_t; + out name: task_name_t); diff --git a/include/mach/mach_types.defs b/include/mach/mach_types.defs index 607d5d9..c913cbb 100644 --- a/include/mach/mach_types.defs +++ b/include/mach/mach_types.defs @@ -230,6 +230,8 @@ type emulation_vector_t = ^array[] of vm_offset_t; type rpc_signature_info_t = array[*:1024] of int; +type task_name_t = (MACH_MSG_TYPE_STRING, 8*32); + #ifKERNEL_SERVER simport kern/ipc_kobject.h; /* for null conversion */ simport kern/ipc_tt.h; /* for task/thread conversion */ diff --git a/include/mach/mach_types.h b/include/mach/mach_types.h index 8768482..5bb3a7b 100644 --- a/include/mach/mach_types.h +++ b/include/mach/mach_types.h @@ -43,6 +43,7 @@ #include mach/port.h #include mach/processor_info.h #include mach/task_info.h +#include mach/task_name.h #include mach/task_special_ports.h #include mach/thread_info.h #include mach/thread_special_ports.h diff --git a/include/mach/task_name.h b/include/mach/task_name.h new file mode 100644 index 000..d6306e2 --- /dev/null +++ b/include/mach/task_name.h @@ -0,0 +1,28 @@ +/* + * Copyright (C) 2013 Free Software Foundation, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, 675 Mass Ave, Cambridge, MA 02139, USA. + * + * Author: Barry deFreese. + */ + +#ifndef _TASK_NAME_H_ +#define _TASK_NAME_H_ + +#define TASK_NAME_SIZE 32 + +typedef char task_name_t[TASK_NAME_SIZE]; + +#endif /* _TASK_NAME_H_ */ diff --git a/kern/task.c b/kern/task.c index 114dd31..438037b 100644 --- a/kern/task.c +++ b/kern/task.c @@ -37,6 +37,7 @@ #include mach/vm_param.h #include mach/task_info.h #include mach/task_special_ports.h +#include mach/task_name.h #include ipc/ipc_space.h #include ipc/ipc_types.h #include kern/debug.h @@ -1212,3 +1213,33 @@ task_ras_control( #endif return ret; } + +kern_return_t +task_set_name ( + task_t task, + task_name_t name) +{ + + if (task == TASK_NULL) + return KERN_INVALID_ARGUMENT; + + strncpy(task-name, name, TASK_NAME_SIZE - 1); + task-name[TASK_NAME_SIZE - 1] = '\0'; + + return KERN_SUCCESS; +} + +kern_return_t +task_get_name ( + task_t task, + task_name_t name) +{ + + if (task == TASK_NULL) + return KERN_INVALID_ARGUMENT; + + strncpy(name, task-name, TASK_NAME_SIZE - 1); + name[TASK_NAME_SIZE - 1] = '\0'; + + return KERN_SUCCESS; +} diff --git a/kern/task.h b/kern/task.h index 9bfea57..842979a 100644 --- a/kern/task.h +++ b/kern/task.h @@ -39,6 +39,7 @@ #include mach/time_value.h #include mach/mach_param.h #include mach/task_info.h +#include mach/task_name.h #include kern/kern_types.h #include kern/lock.h #include kern/queue.h @@ -111,6 +112,8 @@ struct task { natural_t cow_faults; /* copy-on-write faults counter */ natural_t messages_sent; /* messages sent counter */ natural_t messages_received; /* messages received counter */ + + task_name_t name; /* Task name */ }; #define task_lock(task)simple_lock((task)-lock)
Re: RFC for patch to add task_{set,get}_name RPC
Hi! On Thu, 9 May 2013 01:55:56 +0200, Samuel Thibault samuel.thiba...@gnu.org wrote: Roland McGrath, le Wed 08 May 2013 16:47:24 -0700, a écrit : When it helps a huge lot to debug some things, it surely is a way to debug. I was able to debug quite a few spurious port deallocations as soon as I was able to print from the kernel which process was doing it. I don't see how to do the same kind of debugging through the proc server. You can implement some debug-only logging that shows you the mappings you want to see. But the point is that I don't know which mapping I want to see. I just happen to notice from the kernel that a given task does a erroneous thing. From there, how to continue debugging without knowing which userland process was doing that? I understand correctly that the microkernel is just the keeper but itself has no use for the information set with task_set_name. Thus, that information is free-form, for any kind of debugging/logging only (the intent being to set it to the executable's filename that constitutes a task). Then, to what extent are the proposed new RPCs just a specialized variant of the generic port info RPC that we have been musing about, http://darnassus.sceen.net/~hurd-web/open_issues/translate_fd_or_port_to_file_name/, in particular the log from IRC, freenode, #hurd, 2013-03-07? To me it would make sense to follow the latter route, so be able to store with every generic port some bits of debugging/logging information (perhaps literally, like the proposed 32 characters, or perhaps switch to dynamically allocated upon setting the information and released once the port itself is deallocated), and that could then be tied to some new --enable-debugging configure switch (or simply pinned to --enable-kdb), which, if not specified will turn these into MIG_BAD_ID stubs. Grüße, Thomas pgpo5E5PyFEKN.pgp Description: PGP signature
Re: RFC for patch to add task_{set,get}_name RPC
Thomas Schwinge, le Thu 09 May 2013 18:42:18 +0200, a écrit : Then, to what extent are the proposed new RPCs just a specialized variant of the generic port info RPC that we have been musing about, http://darnassus.sceen.net/~hurd-web/open_issues/translate_fd_or_port_to_file_name/, in particular the log from IRC, freenode, #hurd, 2013-03-07? To me it would make sense to follow the latter route, so be able to store with every generic port some bits of debugging/logging information Indeed. (perhaps literally, like the proposed 32 characters, or perhaps switch to dynamically allocated upon setting the information and released once the port itself is deallocated), Either would work, Barry could work on getting the static version working. and that could then be tied to some new --enable-debugging configure switch (or simply pinned to --enable-kdb), which, if not specified will turn these into MIG_BAD_ID stubs. Which is fine too. Samuel
RFC for patch to add task_{set,get}_name RPC
Hi folks, I am trying to get a test environment going to test this but in the meantime if any of you care to review, here is a patch to add task_set_name() and task_get_name() RPCs to gnumach. Thanks! Barry diff --git a/include/mach/gnumach.defs b/include/mach/gnumach.defs index 7331334..b26be96 100644 --- a/include/mach/gnumach.defs +++ b/include/mach/gnumach.defs @@ -37,3 +37,17 @@ type vm_cache_statistics_data_t = struct[11] of integer_t; routine vm_cache_statistics( target_task : vm_task_t; out vm_cache_stats : vm_cache_statistics_data_t); + +/* + * Set the name of a task. + */ +routine task_set_name( + task: task_t; + name: task_name_t); + +/* + * Get the name of a task. + */ +routine task_get_name( + task: task_t; + out name: task_name_t); diff --git a/include/mach/mach_types.defs b/include/mach/mach_types.defs index 607d5d9..c913cbb 100644 --- a/include/mach/mach_types.defs +++ b/include/mach/mach_types.defs @@ -230,6 +230,8 @@ type emulation_vector_t = ^array[] of vm_offset_t; type rpc_signature_info_t = array[*:1024] of int; +type task_name_t = (MACH_MSG_TYPE_STRING, 8*32); + #ifKERNEL_SERVER simport kern/ipc_kobject.h; /* for null conversion */ simport kern/ipc_tt.h; /* for task/thread conversion */ diff --git a/include/mach/mach_types.h b/include/mach/mach_types.h index 8768482..5bb3a7b 100644 --- a/include/mach/mach_types.h +++ b/include/mach/mach_types.h @@ -43,6 +43,7 @@ #include mach/port.h #include mach/processor_info.h #include mach/task_info.h +#include mach/task_name.h #include mach/task_special_ports.h #include mach/thread_info.h #include mach/thread_special_ports.h diff --git a/include/mach/task_name.h b/include/mach/task_name.h new file mode 100644 index 000..296bd9e --- /dev/null +++ b/include/mach/task_name.h @@ -0,0 +1,26 @@ +/* + * Copyright (C) 2013 Free Software Foundation, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, 675 Mass Ave, Cambridge, MA 02139, USA. + * + * Author: Barry deFreese. + */ + +#ifndef _TASK_NAME_H_ +#define _TASK_NAME_H_ + +typedef char task_name_t[32]; + +#endif /* _TASK_NAME_H_ */ diff --git a/kern/task.c b/kern/task.c index 114dd31..d8690c6 100644 --- a/kern/task.c +++ b/kern/task.c @@ -37,6 +37,7 @@ #include mach/vm_param.h #include mach/task_info.h #include mach/task_special_ports.h +#include mach/task_name.h #include ipc/ipc_space.h #include ipc/ipc_types.h #include kern/debug.h @@ -1212,3 +1213,33 @@ task_ras_control( #endif return ret; } + +kern_return_t +task_set_name ( + task_t task, + task_name_t name) +{ + + if (task == TASK_NULL) + return KERN_INVALID_ARGUMENT; + + strncpy(task-name, name, sizeof(task-name) -1); + task-name[sizeof(task-name) -1] = '\0'; + + return KERN_SUCCESS; +} + +kern_return_t +task_get_name ( + task_t task, + task_name_t name) +{ + + if (task == TASK_NULL) + return KERN_INVALID_ARGUMENT; + + strncpy(name, task-name, sizeof(task-name) - 1); + name[sizeof(name) -1] = '\0'; + + return KERN_SUCCESS; +} diff --git a/kern/task.h b/kern/task.h index 9bfea57..842979a 100644 --- a/kern/task.h +++ b/kern/task.h @@ -39,6 +39,7 @@ #include mach/time_value.h #include mach/mach_param.h #include mach/task_info.h +#include mach/task_name.h #include kern/kern_types.h #include kern/lock.h #include kern/queue.h @@ -111,6 +112,8 @@ struct task { natural_t cow_faults; /* copy-on-write faults counter */ natural_t messages_sent; /* messages sent counter */ natural_t messages_received; /* messages received counter */ + + task_name_t name; /* Task name */ }; #define task_lock(task)simple_lock((task)-lock)
Re: RFC for patch to add task_{set,get}_name RPC
What for? If you want something like this, why does it belong in the microkernel?
Re: RFC for patch to add task_{set,get}_name RPC
Roland McGrath, le Wed 08 May 2013 14:59:47 -0700, a écrit : What for? If you want something like this, why does it belong in the microkernel? So the kernel can tell what a task is in, e.g. show tasks command, in memory object statistics, etc. Without this it is hard to inspect the whole system state without divining what task is what process. Samuel
Re: RFC for patch to add task_{set,get}_name RPC
So the kernel can tell what a task is in, e.g. show tasks command, in memory object statistics, etc. Without this it is hard to inspect the whole system state without divining what task is what process. The proc server exists to provide that mapping.
Re: RFC for patch to add task_{set,get}_name RPC
Roland McGrath, le Wed 08 May 2013 15:34:59 -0700, a écrit : So the kernel can tell what a task is in, e.g. show tasks command, in memory object statistics, etc. Without this it is hard to inspect the whole system state without divining what task is what process. The proc server exists to provide that mapping. But we can't really ask the proc server from the kernel debugger. Samuel
Re: RFC for patch to add task_{set,get}_name RPC
But we can't really ask the proc server from the kernel debugger. There are lots of things you can't do from the kernel debugger. That doesn't mean that more state in the microkernel is the way to debug.
Re: RFC for patch to add task_{set,get}_name RPC
Roland McGrath, le Wed 08 May 2013 16:27:53 -0700, a écrit : But we can't really ask the proc server from the kernel debugger. There are lots of things you can't do from the kernel debugger. That doesn't mean that more state in the microkernel is the way to debug. When it helps a huge lot to debug some things, it surely is a way to debug. I was able to debug quite a few spurious port deallocations as soon as I was able to print from the kernel which process was doing it. I don't see how to do the same kind of debugging through the proc server. Samuel
Re: RFC for patch to add task_{set,get}_name RPC
When it helps a huge lot to debug some things, it surely is a way to debug. I was able to debug quite a few spurious port deallocations as soon as I was able to print from the kernel which process was doing it. I don't see how to do the same kind of debugging through the proc server. You can implement some debug-only logging that shows you the mappings you want to see.
Re: RFC for patch to add task_{set,get}_name RPC
Roland McGrath, le Wed 08 May 2013 16:47:24 -0700, a écrit : When it helps a huge lot to debug some things, it surely is a way to debug. I was able to debug quite a few spurious port deallocations as soon as I was able to print from the kernel which process was doing it. I don't see how to do the same kind of debugging through the proc server. You can implement some debug-only logging that shows you the mappings you want to see. But the point is that I don't know which mapping I want to see. I just happen to notice from the kernel that a given task does a erroneous thing. From there, how to continue debugging without knowing which userland process was doing that? Samuel