Unless someone branches to shootdown_now.....

--Terry

At 05:43 PM 8/12/2005 +0200, Frank van der Linden wrote:
>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