To: richard@sleepie.demon.co.uk
Cc: linux-m68k@lists.linux-m68k.org
Subject: Re: L68K: kernel_thread()
References: <358E14C9.D57B72A0@net-tel.co.uk> <vyzemwh6aid.fsf@issan.informatik.uni-dortmund.de> <358E2BC8.C7C8DAB@net-tel.co.uk>
X-Yow: If I am elected no one will ever have to do their laundry again!
From: Andreas Schwab <schwab@issan.informatik.uni-dortmund.de>
Date: 22 Jun 1998 13:44:58 +0200
Sender: owner-linux-m68k@phil.uni-sb.de

Richard Hirst <richard.hirst@net-tel.co.uk> writes:

|> > |>            : "d0", "d2");
|> > |>
|> > |>         set_fs (fs);
|> > |>         return retval;
|> > |> }
|> > |>
|> > |>
|> > |> Shouldn't that __asm__ declare %d0 and %d1 as inputs, and %d1
|> > |> as used?
|> > 
|> > They are already, and %d1 isn't really changed as far as gcc is concerned,
|> > because the second trap doesn't return.
|> 
|> I don't see where %d1 is declared as an input to the __asm__

Here it is:

|> > |>              "r" (arg), "a" (fn), "d" (clone_arg), "r" (current)
                                          ^^^^^^^^^^^^^^^

|> 
|> > |> I added debug code to set_fs(), which included a function call,
|> > |> and the %d0 value from the __asm__ was trashed before being
|> > |> used in the return.
|> > 
|> > You must of course save %d0/%d1/%a0/%a1 around the function call, since
|> > they are call-clobbered.
|> 
|> Of course.  I was just surprised to find I couldn't modify the definition
|> of set_fs() to include a function call without going through the whole
|> source looking for places it was used first to check for assumptions
|> about register usage.

Ok, point taken.  The register variables should be declared and used as
close as possible to the asm, and retval should be copied into another
variable that is then returned instead.  Please try this patch:

--- linux/include/asm-m68k/unistd.h.~1~	Mon Jun 22 13:38:34 1998
+++ linux/include/asm-m68k/unistd.h	Mon Jun 22 13:39:54 1998
@@ -329,13 +329,16 @@
  */
 static inline pid_t kernel_thread(int (*fn)(void *), void * arg, unsigned long flags)
 {
-	register long retval __asm__ ("d0") = __NR_clone;
-	register long clone_arg __asm__ ("d1") = flags | CLONE_VM;
+	pid_t pid;
 	mm_segment_t fs;
 
 	fs = get_fs();
 	set_fs (KERNEL_DS);
 
+	{
+	register long retval __asm__ ("d0");
+	register long clone_arg __asm__ ("d1") = flags | CLONE_VM;
+
 	__asm__ __volatile__
 	  ("clrl %%d2\n\t"
 	   "trap #0\n\t"		/* Linux/m68k system call */
@@ -352,9 +355,11 @@
 	   : "0" (__NR_clone), "i" (__NR_exit),
 	     "r" (arg), "a" (fn), "d" (clone_arg), "r" (current)
 	   : "d0", "d2");
+	pid = retval;
+	}
 
 	set_fs (fs);
-	return retval;
+	return pid;
 }
 
 static inline pid_t wait(int * wait_stat)

Andreas.