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