Subject: Re: CVS commit: src/sys/arch/i386/i386
To: YAMAMOTO Takashi <yamt@netbsd.org>
From: Frank van der Linden <fvdl@netbsd.org>
List: port-i386
Date: 08/12/2005 17:43:29
On Fri, Aug 12, 2005 at 10:04:24AM +0000, YAMAMOTO Takashi wrote:
> 
> Module Name:	src
> Committed By:	yamt
> Date:		Fri Aug 12 10:04:24 UTC 2005
> 
> Modified Files:
> 	src/sys/arch/i386/i386: pmap.c
> 
> Log Message:
> pmap_enter: fix an uninitialized variable bug which can cause
> "TLB IPI rendezvous failed".

This change has no effect. The diff is:

==========

Index: src/sys/arch/i386/i386/pmap.c
diff -c src/sys/arch/i386/i386/pmap.c:1.183 src/sys/arch/i386/i386/pmap.c:1.184
*** src/sys/arch/i386/i386/pmap.c:1.183	Sun May 29 15:58:33 2005
--- src/sys/arch/i386/i386/pmap.c	Fri Aug 12 10:04:24 2005
***************
*** 3421,3430 ****
  	/* Update page attributes if needed */
  	if ((opte & (PG_V | PG_U)) == (PG_V | PG_U)) {
  #if defined(MULTIPROCESSOR)
! 		int32_t cpumask = 0;
  #endif
  shootdown_now:
  #if defined(MULTIPROCESSOR)
  		pmap_tlb_shootdown(pmap, va, opte, &cpumask);
  		pmap_tlb_shootnow(cpumask);
  #else
--- 3421,3432 ----
  	/* Update page attributes if needed */
  	if ((opte & (PG_V | PG_U)) == (PG_V | PG_U)) {
  #if defined(MULTIPROCESSOR)
! 		int32_t cpumask;
  #endif
  shootdown_now:
  #if defined(MULTIPROCESSOR)
+ 		cpumask = 0;
+ 
  		pmap_tlb_shootdown(pmap, va, opte, &cpumask);
  		pmap_tlb_shootnow(cpumask);
  #else

==========

As you can see, the code still does the same thing. Just to make sure that it
wasn't a compiler bug, I compared the assembly output (with stabs and labels
stripped):

=======

*** old2	Fri Aug 12 17:31:25 2005
--- new2	Fri Aug 12 17:31:19 2005
***************
*** 4340,4351 ****
  	call	Xspllower
  	addl	$16, %esp
  	movl	$0, (%ecx)
- 	movl	$0, -72(%ebp)
  	leal	-72(%ebp), %eax
  	pushl	%eax
  	pushl	-76(%ebp)
  	pushl	12(%ebp)
  	pushl	8(%ebp)
  	call	pmap_tlb_shootdown
  	popl	%eax
  	pushl	-72(%ebp)
--- 4340,4351 ----
  	call	Xspllower
  	addl	$16, %esp
  	movl	$0, (%ecx)
  	leal	-72(%ebp), %eax
  	pushl	%eax
  	pushl	-76(%ebp)
  	pushl	12(%ebp)
  	pushl	8(%ebp)
+ 	movl	$0, -72(%ebp)
  	call	pmap_tlb_shootdown
  	popl	%eax
  	pushl	-72(%ebp)

==========

For some reason, it moved the 0 assignment down a few instructions, but
there's no difference there; cpumask is always correctly set to 0 before
a pointer to it is passed to pmap_tlb_shootdown.

Since this is a null change, it's probably better to revert it or to
change the CVS comment.

- Frank