[PATCH 2/19] UML - Tidy stub management code

[PATCH 2/19] UML - Tidy stub management code

[Jeff Dike]

Restructure the stub management code to make it simpler.

syscall_stub_done is extracted from do_syscall_stub.

The counters are gone since I never looked at them.

The common code in run_syscall_stub and syscall_stub_data is extracted
into flush_syscalls.

Signed-off-by: Jeff Dike <jdike@linux.intel.com>
---
 arch/um/os-Linux/skas/mem.c |  162 +++++++++++++++++++++++---------------------
 1 file changed, 85 insertions(+), 77 deletions(-)

Index: linux-2.6-git/arch/um/os-Linux/skas/mem.c
===================================================================
--- linux-2.6-git.orig/arch/um/os-Linux/skas/mem.c	2008-04-25 11:36:59.000000000 -0400
+++ linux-2.6-git/arch/um/os-Linux/skas/mem.c	2008-04-25 11:39:07.000000000 -0400
@@ -40,35 +40,69 @@ static unsigned long syscall_regs[MAX_RE
 
 static int __init init_syscall_regs(void)
 {
+	unsigned long *stub_entry;
+
 	get_safe_registers(syscall_regs);
+	stub_entry = &batch_syscall_stub;
+
 	syscall_regs[REGS_IP_INDEX] = STUB_CODE +
-		((unsigned long) &batch_syscall_stub -
+		((unsigned long) stub_entry -
 		 (unsigned long) &__syscall_stub_start);
 	return 0;
 }
 
 __initcall(init_syscall_regs);
 
-extern int proc_mm;
+static int syscall_stub_done(unsigned long stack)
+{
+	unsigned long *syscall, *data, offset;
+	int ret, n;
+
+	/*
+	 * When the stub stops, we find the following values on the
+	 * beginning of the stack:
+	 * (long) return_value
+	 * (long) offset to failed sycall data (0 if no error)
+	 */
+	ret = *((unsigned long *) stack);
+	offset = *((unsigned long *) stack + 1);
+	if (offset == 0)
+		return 0;
 
-int single_count = 0;
-int multi_count = 0;
-int multi_op_count = 0;
+	data = (unsigned long *)(stack + offset - STUB_DATA);
+	printk(UM_KERN_ERR "syscall_stub_done : ret = %d, offset = %ld, "
+	       "data = %p\n", ret, offset, data);
+	syscall = (unsigned long *)((unsigned long)data + data[0]);
+	printk(UM_KERN_ERR "syscall_stub_done : syscall %ld failed, "
+	       "return value = 0x%x, expected return value = 0x%lx\n",
+	       syscall[0], ret, syscall[7]);
+	printk(UM_KERN_ERR "    syscall parameters: "
+	       "0x%lx 0x%lx 0x%lx 0x%lx 0x%lx 0x%lx\n",
+	       syscall[1], syscall[2], syscall[3],
+	       syscall[4], syscall[5], syscall[6]);
+	for (n = 1; n < data[0]/sizeof(long); n++) {
+		if (n == 1)
+			printk(UM_KERN_ERR "    additional syscall "
+			       "data:");
+		if (n % 4 == 1)
+			printk(UM_KERN_CONT "\n" UM_KERN_ERR "      ");
+		printk(UM_KERN_CONT "  0x%lx", data[n]);
+	}
+	if (n > 1)
+		printk("\n");
 
-static inline long do_syscall_stub(struct mm_id * mm_idp, void **addr)
+	return ret;
+}
+
+static long do_syscall_stub(struct mm_id *mm_idp, void **addr)
 {
-	int n, i;
-	long ret, offset;
-	unsigned long * data;
-	unsigned long * syscall;
-	int err, pid = mm_idp->u.pid;
+	long ret;
+	int n, i, err, pid = mm_idp->u.pid;
 
 	if (proc_mm)
 		/* FIXME: Need to look up userspace_pid by cpu */
 		pid = userspace_pid[0];
 
-	multi_count++;
-
 	n = ptrace_setregs(pid, syscall_regs);
 	if (n < 0) {
 		printk(UM_KERN_ERR "Registers - \n");
@@ -85,52 +119,39 @@ static inline long do_syscall_stub(struc
 
 	wait_stub_done(pid);
 
-	/*
-	 * When the stub stops, we find the following values on the
-	 * beginning of the stack:
-	 * (long )return_value
-	 * (long )offset to failed sycall-data (0, if no error)
-	 */
-	ret = *((unsigned long *) mm_idp->stack);
-	offset = *((unsigned long *) mm_idp->stack + 1);
-	if (offset) {
-		data = (unsigned long *)(mm_idp->stack + offset - STUB_DATA);
-		printk(UM_KERN_ERR "do_syscall_stub : ret = %ld, offset = %ld, "
-		       "data = %p\n", ret, offset, data);
-		syscall = (unsigned long *)((unsigned long)data + data[0]);
-		printk(UM_KERN_ERR "do_syscall_stub: syscall %ld failed, "
-		       "return value = 0x%lx, expected return value = 0x%lx\n",
-		       syscall[0], ret, syscall[7]);
-		printk(UM_KERN_ERR "    syscall parameters: "
-		       "0x%lx 0x%lx 0x%lx 0x%lx 0x%lx 0x%lx\n",
-		       syscall[1], syscall[2], syscall[3],
-		       syscall[4], syscall[5], syscall[6]);
-		for (n = 1; n < data[0]/sizeof(long); n++) {
-			if (n == 1)
-				printk(UM_KERN_ERR "    additional syscall "
-				       "data:");
-			if (n % 4 == 1)
-				printk("\n" UM_KERN_ERR "      ");
-			printk("  0x%lx", data[n]);
-		}
-		if (n > 1)
-			printk("\n");
-	}
-	else ret = 0;
+	ret = syscall_stub_done(mm_idp->stack);
 
 	*addr = check_init_stack(mm_idp, NULL);
 
 	return ret;
 }
 
-long run_syscall_stub(struct mm_id * mm_idp, int syscall,
+static int flush_syscalls(struct mm_id *mm_idp, void **addr, int extra)
+{
+	unsigned long *stack = check_init_stack(mm_idp, *addr);
+	int current, end;
+
+	current = ((unsigned long) stack) & ~UM_KERN_PAGE_MASK;
+	end = UM_KERN_PAGE_SIZE;
+
+	if (current + (10 + extra) * sizeof(long) < end)
+		return 0;
+
+	return do_syscall_stub(mm_idp, addr);
+}
+
+long run_syscall_stub(struct mm_id *mm_idp, int syscall,
 		      unsigned long *args, long expected, void **addr,
 		      int done)
 {
-	unsigned long *stack = check_init_stack(mm_idp, *addr);
+	unsigned long *stack;
+	int ret;
 
-	if (done && *addr == NULL)
-		single_count++;
+	ret = flush_syscalls(mm_idp, addr, 0);
+	if (ret)
+		return ret;
+
+	stack = check_init_stack(mm_idp, *addr);
 
 	*stack += sizeof(long);
 	stack += *stack / sizeof(long);
@@ -144,45 +165,32 @@ long run_syscall_stub(struct mm_id * mm_
 	*stack++ = args[5];
 	*stack++ = expected;
 	*stack = 0;
-	multi_op_count++;
 
-	if (!done && ((((unsigned long) stack) & ~UM_KERN_PAGE_MASK) <
-		     UM_KERN_PAGE_SIZE - 10 * sizeof(long))) {
-		*addr = stack;
-		return 0;
-	}
+	if (done)
+		return do_syscall_stub(mm_idp, addr);
 
-	return do_syscall_stub(mm_idp, addr);
+	*addr = stack;
+
+	return 0;
 }
 
-long syscall_stub_data(struct mm_id * mm_idp,
-		       unsigned long *data, int data_count,
-		       void **addr, void **stub_addr)
+long syscall_stub_data(struct mm_id *mm_idp, unsigned long *data,
+		       int data_count, void **addr, void **stub_addr)
 {
 	unsigned long *stack;
-	int ret = 0;
+	int ret;
 
-	/*
-	 * If *addr still is uninitialized, it *must* contain NULL.
-	 * Thus in this case do_syscall_stub correctly won't be called.
-	 */
-	if ((((unsigned long) *addr) & ~UM_KERN_PAGE_MASK) >=
-	   UM_KERN_PAGE_SIZE - (10 + data_count) * sizeof(long)) {
-		ret = do_syscall_stub(mm_idp, addr);
-		/* in case of error, don't overwrite data on stack */
-		if (ret)
-			return ret;
-	}
+	ret = flush_syscalls(mm_idp, addr, data_count);
+	if (ret)
+		return ret;
 
 	stack = check_init_stack(mm_idp, *addr);
-	*addr = stack;
-
-	*stack = data_count * sizeof(long);
+	*stack++ = data_count * sizeof(long);
 
-	memcpy(stack + 1, data, data_count * sizeof(long));
+	memcpy(stack, data, data_count * sizeof(long));
 
-	*stub_addr = (void *)(((unsigned long)(stack + 1) &
-			       ~UM_KERN_PAGE_MASK) + STUB_DATA);
+	*stub_addr = (void *)(((unsigned long) stack & ~UM_KERN_PAGE_MASK) +
+			      STUB_DATA);
 
 	return 0;
 }

Re: [PATCH 2/19] UML - Tidy stub management code

[WANG Cong]

From: Jeff Dike <jdike@addtoit.com>
Date: Fri, 25 Apr 2008 13:56:06 -0400
> Restructure the stub management code to make it simpler.
> 
> syscall_stub_done is extracted from do_syscall_stub.
> 
> The counters are gone since I never looked at them.
> 
> The common code in run_syscall_stub and syscall_stub_data is extracted
> into flush_syscalls.
> 
> Signed-off-by: Jeff Dike <jdike@linux.intel.com>
> ---
>  arch/um/os-Linux/skas/mem.c |  162 +++++++++++++++++++++++---------------------
>  1 file changed, 85 insertions(+), 77 deletions(-)
> 
> Index: linux-2.6-git/arch/um/os-Linux/skas/mem.c
> ===================================================================
> --- linux-2.6-git.orig/arch/um/os-Linux/skas/mem.c	2008-04-25 11:36:59.000000000 -0400
> +++ linux-2.6-git/arch/um/os-Linux/skas/mem.c	2008-04-25 11:39:07.000000000 -0400
> @@ -40,35 +40,69 @@ static unsigned long syscall_regs[MAX_RE
>  
>  static int __init init_syscall_regs(void)
>  {
> +	unsigned long *stub_entry;
> +
>  	get_safe_registers(syscall_regs);
> +	stub_entry = &batch_syscall_stub;
> +
>  	syscall_regs[REGS_IP_INDEX] = STUB_CODE +
> -		((unsigned long) &batch_syscall_stub -
> +		((unsigned long) stub_entry -
>  		 (unsigned long) &__syscall_stub_start);
>  	return 0;
>  }
>  
>  __initcall(init_syscall_regs);
>  
> -extern int proc_mm;
> +static int syscall_stub_done(unsigned long stack)
> +{
> +	unsigned long *syscall, *data, offset;
> +	int ret, n;
> +
> +	/*
> +	 * When the stub stops, we find the following values on the
> +	 * beginning of the stack:
> +	 * (long) return_value
> +	 * (long) offset to failed sycall data (0 if no error)
> +	 */
> +	ret = *((unsigned long *) stack);


I am afraid the value will be truncated on 64bit machine, since
'ret' is 'int' while 'stack' points to an 'unsigned long'.

Is this expected?

Thanks.

Re: [PATCH 2/19] UML - Tidy stub management code

[Jeff Dike]

On Sat, Apr 26, 2008 at 06:01:12PM +0800, WANG Cong wrote:
> > +	/*
> > +	 * When the stub stops, we find the following values on the
> > +	 * beginning of the stack:
> > +	 * (long) return_value
> > +	 * (long) offset to failed sycall data (0 if no error)
> > +	 */
> > +	ret = *((unsigned long *) stack);
> 
> 
> I am afraid the value will be truncated on 64bit machine, since
> 'ret' is 'int' while 'stack' points to an 'unsigned long'.
> 
> Is this expected?

That's dubious, but I don't think it breaks anything.  On LE, *(int *)
and *(long *) will give you the same answer, and the value here is an
error code, which fits into 32 bits.

      	    	       Jeff

-- 
Work email - jdike at linux dot intel dot com

Re: [PATCH 2/19] UML - Tidy stub management code

[WANG Cong]

On Mon, 28 Apr 2008, Jeff Dike wrote:

> On Sat, Apr 26, 2008 at 06:01:12PM +0800, WANG Cong wrote:
>> > +	/*
>> > +	 * When the stub stops, we find the following values on the
>> > +	 * beginning of the stack:
>> > +	 * (long) return_value
>> > +	 * (long) offset to failed sycall data (0 if no error)
>> > +	 */
>> > +	ret = *((unsigned long *) stack);
>> 
>> 
>> I am afraid the value will be truncated on 64bit machine, since
>> 'ret' is 'int' while 'stack' points to an 'unsigned long'.
>> 
>> Is this expected?
>
> That's dubious, but I don't think it breaks anything.  On LE, *(int *)
> and *(long *) will give you the same answer, and the value here is an
> error code, which fits into 32 bits.

So it's safe here.

OK. Thanks!

--
Hi, I'm a .signature virus, please copy/paste me to help me spread
all over the world.