Re: RFC for patch to add task_{set,get}_name RPC

2013-05-10 Thread Richard Braun
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

2013-05-10 Thread Richard Braun
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

2013-05-09 Thread Samuel Thibault
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

2013-05-09 Thread Barry deFreese
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

2013-05-09 Thread Thomas Schwinge
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

2013-05-09 Thread Samuel Thibault
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

2013-05-08 Thread Barry deFreese
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

2013-05-08 Thread Roland McGrath
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

2013-05-08 Thread Samuel Thibault
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

2013-05-08 Thread Roland McGrath
 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

2013-05-08 Thread Samuel Thibault
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

2013-05-08 Thread Roland McGrath
 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

2013-05-08 Thread Samuel Thibault
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

2013-05-08 Thread Roland McGrath
 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

2013-05-08 Thread Samuel Thibault
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