Skip to contentSkip to conversation

uml: fix uml-hang-on-2.6.9-host.patch

PATCH In review Bodo Stroesser 2 participants 2 messages Nov 12, 2004 – Nov 18, 2004 User-mode Linux (UML)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 uml: fix uml-hang-on-2.6.9-host.patch

From: Bodo Stroesser <[email protected]>

The patch needs to be corrected. The use of os_kill_ptraced_process() is restricted to the tracer-thread! No other process is allowed to do ptrace(PTRACE_KILL). So I had "dead" processes hanging in my host system until UML was shutdown. And I had panic()s, because processes were not stopped.
Thus, I implemented request_kill_ptraced_process(), which lets the tracer do the ptrace(PTRACE_KILL). Sorry, this method is slow, but I didn't see an other solution.
Also, I modified some other places to use the new call instead of os_kill_process(). But this didn't work on the first step. kill_off_processes_tt() couldn't kill the remaining processes on shutdown. This happens, because ptrace(PTRACE_KILL) doesn't really send a SIGKILL to the process, but writes SIGKILL to the exit_code. That does results in send_sig(SIGKILL) for processes being stopped on a ptrace event only, but not even for all of these events.
So I added an option to request_kill_ptraced_process() that allows to force an additional os_kill_process() to be executed before the os_kill_ptraced_process().
Summary: The behavior of 2.6.9 host is very uggly. You need to do kill() and ptrace(PTRACE_KILL) to be shure, a process will really exit. If the host is changed to be more consistent, we should change all this again.

---

include/asm-um/processor-generic.h +2 -1
@@ -54,7 +54,8 @@ struct thread_struct {
 		union {
 			struct {
 				int pid;
-			} fork, exec;
+				int mode;
+			} fork, exec, kill;
 			struct {
 				int (*proc)(void *);
 				void *arg;
arch/um/kernel/tt/include/mode-tt.h +1 -1
@@ -8,7 +8,7 @@
 
 #include "sysdep/ptrace.h"
 
-enum { OP_NONE, OP_EXEC, OP_FORK, OP_TRACE_ON, OP_REBOOT, OP_HALT, OP_CB };
+enum { OP_NONE, OP_EXEC, OP_FORK, OP_KILL, OP_TRACE_ON, OP_REBOOT, OP_HALT, OP_CB };
 
 extern int tracing_pid;
 
arch/um/kernel/tt/exec_user.c +2 -1
@@ -16,6 +16,7 @@
 #include "kern_util.h"
 #include "user.h"
 #include "ptrace_user.h"
+#include "os.h"
 
 void do_exec(int old_pid, int new_pid)
 {
@@ -36,7 +37,7 @@ void do_exec(int old_pid, int new_pid)
 		tracer_panic("do_exec failed to get registers - errno = %d",
 			     errno);
 
-	kill(old_pid, SIGKILL);
+	os_kill_ptraced_process(old_pid, 0);
 
 	if (ptrace(PTRACE_SETOPTIONS, new_pid, 0, (void *)PTRACE_O_TRACESYSGOOD) < 0)
 		tracer_panic("do_exec: PTRACE_SETOPTIONS failed, errno = %d", errno);
arch/um/kernel/tt/process_kern.c +24 -4
@@ -82,7 +82,7 @@ void *switch_to_tt(void *prev, void *nex
 	prev_sched = current->thread.prev_sched;
 	if((prev_sched->state == TASK_ZOMBIE) ||
 	   (prev_sched->state == TASK_DEAD))
-		os_kill_ptraced_process(prev_sched->thread.mode.tt.extern_pid, 1);
+		request_kill_ptraced_process( prev_sched->thread.mode.tt.extern_pid, 1);
 
 	/* This works around a nasty race with 'jail'.  If we are switching
 	 * between two threads of a threaded app and the incoming process
@@ -119,7 +119,7 @@ void release_thread_tt(struct task_struc
 	int pid = task->thread.mode.tt.extern_pid;
 
 	if(os_getpid() != pid)
-		os_kill_process(pid, 0);
+		request_kill_ptraced_process(pid, 0);
 }
 
 void exit_thread_tt(void)
@@ -330,10 +330,10 @@ void kill_off_processes_tt(void)
 	me = os_getpid();
         for_each_process(p){
 		if(p->thread.mode.tt.extern_pid != me)
-			os_kill_process(p->thread.mode.tt.extern_pid, 0);
+			request_kill_ptraced_process(p->thread.mode.tt.extern_pid, 2);
 	}
 	if(init_task.thread.mode.tt.extern_pid != me)
-		os_kill_process(init_task.thread.mode.tt.extern_pid, 0);
+		request_kill_ptraced_process(init_task.thread.mode.tt.extern_pid, 2);
 }
 
 void initial_thread_cb_tt(void (*proc)(void *), void *arg)
@@ -352,6 +352,20 @@ void initial_thread_cb_tt(void (*proc)(v
 	}
 }
 
+void request_kill_ptraced_process(int pid, int mode)
+{
+	/* The tracer has to do the kill, since killing must be
+	 * done with ptrace(PTRACE_KILL, pid), which is possible
+	 * from the father only! */
+	current->thread.request.op = OP_KILL;
+	current->thread.request.u.exec.pid = pid;
+	current->thread.request.u.exec.mode = mode;
+	os_usr1_process(os_getpid());
+	change_sig(SIGUSR1, 1);
+
+	change_sig(SIGUSR1, 0);
+}
+
 int do_proc_op(void *t, int proc_id)
 {
 	struct task_struct *task;
@@ -374,6 +388,12 @@ int do_proc_op(void *t, int proc_id)
 	case OP_FORK:
 		attach_process(thread->request.u.fork.pid);
 		break;
+	case OP_KILL:
+		if ( thread->request.u.kill.mode > 1 )
+			os_kill_process(thread->request.u.kill.pid, 0);
+		os_kill_ptraced_process(thread->request.u.kill.pid,
+		                        thread->request.u.kill.mode);
+		break;
 	case OP_CB:
 		(*thread->request.u.cb.proc)(thread->request.u.cb.arg);
 		break;
arch/um/kernel/tt/include/mode-tt.h +1 -0
@@ -20,6 +20,7 @@ extern void reboot_tt(void);
 extern void halt_tt(void);
 extern int is_tracer_winch(int pid, int fd, void *data);
 extern void kill_off_processes_tt(void);
+extern void request_kill_ptraced_process(int pid, int mode);
 
 #endif
 
arch/um/kernel/reboot.c +1 -1
@@ -22,7 +22,7 @@ static void kill_idlers(int me)
 	for(i = 0; i < sizeof(idle_threads)/sizeof(idle_threads[0]); i++){
 		p = idle_threads[i];
 		if((p != NULL) && (p->thread.mode.tt.extern_pid != me))
-			os_kill_process(p->thread.mode.tt.extern_pid, 0);
+			request_kill_ptraced_process(p->thread.mode.tt.extern_pid, 2);
 	}
 #endif
 }

-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

#1 lore raw
Quoted 9 lines · Bodo Stroesser

From: Bodo Stroesser <[email protected]>

The patch needs to be corrected. The use of os_kill_ptraced_process() is restricted to the tracer-thread! No other process is allowed to do ptrace(PTRACE_KILL). So I had "dead" processes hanging in my host system until UML was shutdown. And I had panic()s, because processes were not stopped.

Quoted 3 lines

Thus, I implemented request_kill_ptraced_process(), which lets the tracer do the ptrace(PTRACE_KILL). Sorry, this method is slow, but I didn't see an other solution.

Hmm, given how slow TT mode is, I think it may not be noticeable. Especially since the signal handling is asynchronous, the exited thread gets "scheduled to be killed when the signal is delivered". However, also look at Jeff Dike patch he's pushing for testing.

A doubt: IIRC, the queue keeps only one instance of each signal, right? Should not we switch all uses of SIGUSR* to real-time signals (which could also reduce latency, maybe)?

Quoted 7 lines

Also, I modified some other places to use the new call instead of os_kill_process(). But this didn't work on the first step. kill_off_processes_tt() couldn't kill the remaining processes on shutdown. This happens, because ptrace(PTRACE_KILL) doesn't really send a SIGKILL to the process, but writes SIGKILL to the exit_code.

Quoted 7 lines

That does results in send_sig(SIGKILL) for processes being stopped on a ptrace event only, but not even for all of these events.

So I added an option to request_kill_ptraced_process() that allows to force an additional os_kill_process() to be executed before the os_kill_ptraced_process().

About this: it's better to use symbolic constants for such usages, never to use numbers. And if you meet code snippets in UML were this is not applied, feel free to fix them.

Quoted 3 lines

Summary: The behavior of 2.6.9 host is very uggly. You need to do kill() and ptrace(PTRACE_KILL) to be shure, a process will really exit.

Hmm.... could you supply a testcase for this?

No, I've modified the Gerd Knorr testcase, replacing kill(SIGKILL) with ptrace(PTRACE_KILL), and the result was that PTRACE_KILL alone works, in the simplest scenario at least.

Quoted 2 lines

If the host is changed to be more consistent, we
should change all this again.

PTRACE_KILL should always work, on every kernel. If not, then we must fix that in mainline, and also

PTRACE_KILL not working would be a real bug, but I would rather check if the childs are stopped, and I would happily ask Roland McGrath to fix it and I'd include the fix in the SKAS patch (I'm worried it's getting too big, but so far, so good). We are facing, instead, a correct API change.

The problem is that we used SIGKILL, which should never have worked. I first thought Roland McGrath didn't realize this fact. Now I understand, instead, that the kernel should always disallow a ptraced process to be killed by someone which is not his parent, when it's stopped (when you reboot, you killall everything).

So, the behaviour needed by older UMLs will never be restored - I thought this was possible until now, but no more.

Signature · 14 lines
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729



-------------------------------------------------------
This SF.Net email is sponsored by: InterSystems CACHE
FREE OODBMS DOWNLOAD - A multidimensional database that combines
robust object and relational technologies, making it a perfect match
for Java, C++,COM, XML, ODBC and JDBC. www.intersystems.com/match8
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel