[patch 1/6] remove dead code in ia64_leave_kernel

[patch 1/6] remove dead code in ia64_leave_kernel

[Chen, Kenneth W]

The code around restoring user backing store in ia64_leave_kernel
is just plain dumb.  ia64_leave_kernel is used for non-syscall
exit path.  When entering the kernel from the user space, we
already preserved the user stack register frame by doing a
cover instruction.  on the way out, ia64_leave_kernel uncondition-
ally bypass a section of code by "br dont_preserve_current_frame"
(pNonSys should always be one in that path).  However, part of
code in the beginning of rbs_switch is used by ia64_leave_syscall
to calculate new "ndirty" bytes since in leave syscall, kernel
have to preserve the current frame and thus needing to re
calculate new ndirty size for loadrs.  Streamline that section of
code into ia64_leave_syscall and remove unneeded reading ar.bsp
in the leave kernel path.

Signed-off-by: Ken Chen <kenneth.w.chen@intel.com>

--- ./arch/ia64/kernel/entry.S.orig	2006-01-02 19:21:10.000000000 -0800
+++ ./arch/ia64/kernel/entry.S	2006-01-22 02:20:46.475278554 -0800
@@ -787,6 +787,13 @@ ENTRY(ia64_leave_syscall)
 
 	mov.m ar.ssd=r0			// M2   clear ar.ssd
 	mov f11ð			// F    clear f11
+	sub r16=r16,r18			// krbs = old bsp - size of dirty partition
+	cmp.ne p9,p0=r0,r0		// clear p9 to skip restore of cr.ifs
+	;;
+	sub r19=r19,r16			// calculate total byte size of dirty partition
+	add r18d,r18			// don't force in0-in7 into memory...
+	;;
+	shl r19=r19,16			// shift size of dirty partition into loadrs pos
 	br.cond.sptk.many rbs_switch	// B
 END(ia64_leave_syscall)
 
@@ -951,32 +958,17 @@ GLOBAL_ENTRY(ia64_leave_kernel)
 (pUStk)	st1 [r18]=r17		// restore current->thread.on_ustack
 	shr.u r18=r19,16	// get byte size of existing "dirty" partition
 	;;
-	mov r16=ar.bsp		// get existing backing store pointer
 	addl r17=THIS_CPU(ia64_phys_stacked_size_p8),r0
 	;;
 	ld4 r17=[r17]		// r17 = cpu_data->phys_stacked_size_p8
 (pKStk)	br.cond.dpnt skip_rbs_switch
 
+rbs_switch:
 	/*
 	 * Restore user backing store.
 	 *
 	 * NOTE: alloc, loadrs, and cover can't be predicated.
-	 */
-(pNonSys) br.cond.dpnt dont_preserve_current_frame
-	cover				// add current frame into dirty partition and set cr.ifs
-	;;
-	mov r19=ar.bsp			// get new backing store pointer
-rbs_switch:
-	sub r16=r16,r18			// krbs = old bsp - size of dirty partition
-	cmp.ne p9,p0=r0,r0		// clear p9 to skip restore of cr.ifs
-	;;
-	sub r19=r19,r16			// calculate total byte size of dirty partition
-	add r18d,r18			// don't force in0-in7 into memory...
-	;;
-	shl r19=r19,16			// shift size of dirty partition into loadrs position
-	;;
-dont_preserve_current_frame:
-	/*
+	 *
 	 * To prevent leaking bits between the kernel and user-space,
 	 * we must clear the stacked registers in the "invalid" partition here.
 	 * Not pretty, but at least it's fast (3.34 registers/cycle on Itanium,

FW: [patch 1/6] remove dead code in ia64_leave_kernel

[Chen, Kenneth W]

I just did a reply on the original mail.  Forwarding my response.

-----Original Message-----
From: Chen, Kenneth W [mailto:kenneth.w.chen@intel.com] 
Sent: Tuesday, January 31, 2006 11:42 AM
To: 'David Mosberger-Tang'
Subject: RE: [patch 1/6] remove dead code in ia64_leave_kernel

David Mosberger-Tang wrote on Tuesday, January 31, 2006 9:51 AM
> Be careful here.  Signal-delivery triggers some strange corner-cases. 
> I don't have this all in my head at the moment, but one tricky part is
> returning from a signal-handler, where you enter the kernel as a
> syscall, but leave it as a non-syscall (usually).  I trust that you
> already checked this case, but wanted to make sure the point isn't
> lost.

Thanks for the warning, I don't think I checked that corner case. I will
look at it again.


> Also, I believe with your patch, you'll end up consuming the read of
> ar.bsp which could adversely affect the latency of syscalls
> (especially when everything is cached).  Did you check on this?

Kernel currently consumes the read of ar.bsp, since it has to preserve
the current frame, and it does so by doing two ar.bsp read: once before
cover instruction, and once after to calculate new ndirty value to be
loaded into ar.rsc.  I'm not changing the order or location of these
ar.bsp reads in this patch.  The code that I moved are always executed
for syscall leave.

Having said that, I have a craze idea of optimize away two bsp reads
along with cover instruction in the syscall leave path.  I'm still
exploring whether that is achievable or not.  If we can do that, then
all that latency would go away along with the code that this patch moved.

- Ken

Re: FW: [patch 1/6] remove dead code in ia64_leave_kernel

[David Mosberger-Tang]

On 1/31/06, Chen, Kenneth W <kenneth.w.chen@intel.com> wrote:

> Having said that, I have a craze idea of optimize away two bsp reads
> along with cover instruction in the syscall leave path.  I'm still
> exploring whether that is achievable or not.  If we can do that, then
> all that latency would go away along with the code that this patch moved.

That would be nice, indeed.

  --david
--
Mosberger Consulting LLC, http://www.mosberger-consulting.com/