[PATCH 2.4.23-pre8] Remove broken prefetching in free_one_pgd()

[PATCH 2.4.23-pre8] Remove broken prefetching in free_one_pgd()

[Luck, Tony]

This patch was accepted into 2.5.55, attributed to "davej@uk".

This code will prefetch from beyond the end of the page table
being cleared ... which is clearly a bad thing if the page table
in question is allocated from the last page of memory (or precedes
a hole on a discontig mem system).

-Tony Luck

diff -ru linux-2.4.23-pre8/mm/memory.c fix/mm/memory.c
--- linux-2.4.23-pre8/mm/memory.c	Fri Oct 24 13:37:23 2003
+++ fix/mm/memory.c	Fri Oct 24 13:40:47 2003
@@ -120,10 +120,8 @@
 	}
 	pmd = pmd_offset(dir, 0);
 	pgd_clear(dir);
-	for (j = 0; j < PTRS_PER_PMD ; j++) {
-		prefetchw(pmd+j+(PREFETCH_STRIDE/16));
+	for (j = 0; j < PTRS_PER_PMD ; j++)
 		free_one_pmd(pmd+j);
-	}
 	pmd_free(pmd);
 }
 

Re: [PATCH 2.4.23-pre8] Remove broken prefetching in free_one_pgd()

[David Mosberger]

>>>>> On Fri, 24 Oct 2003 13:56:31 -0700, "Luck, Tony" <tony.luck@intel.com> said:

  Tony> This patch was accepted into 2.5.55, attributed to "davej@uk".
  Tony> This code will prefetch from beyond the end of the page table
  Tony> being cleared ... which is clearly a bad thing if the page
  Tony> table in question is allocated from the last page of memory
  Tony> (or precedes a hole on a discontig mem system).

Different arches behave differently, though.  In the case of ia64,
it'a always safe to prefetch (even with lfetch.fault).

	--david

RE: [PATCH 2.4.23-pre8] Remove broken prefetching in free_one_pgd()

[Luck, Tony]

>   Tony> This patch was accepted into 2.5.55, attributed to "davej@uk".
>   Tony> This code will prefetch from beyond the end of the page table
>   Tony> being cleared ... which is clearly a bad thing if the page
>   Tony> table in question is allocated from the last page of memory
>   Tony> (or precedes a hole on a discontig mem system).
> 
> Different arches behave differently, though.  In the case of ia64,
> it'a always safe to prefetch (even with lfetch.fault).

Not quite always ... this was how I found the efi trim.bottom bug, since
Linux had allocated a pgd at 0xa00000-16k, and the lfetch that reached
out beyond the end of the page to the uncacheable address 0xa00000 took
an MCA.

A pgd in the last page of a granule that is followed by an uncacheable
address would do the same with lfetch.fault, wouldn't it?

-Tony

RE: [PATCH 2.4.23-pre8] Remove broken prefetching in free_one_pgd()

[David Mosberger]

>>>>> On Fri, 24 Oct 2003 15:16:59 -0700, "Luck, Tony" <tony.luck@intel.com> said:

  Tony> This patch was accepted into 2.5.55, attributed to "davej@uk".
  Tony> This code will prefetch from beyond the end of the page table
  Tony> being cleared ... which is clearly a bad thing if the page
  Tony> table in question is allocated from the last page of memory
  Tony> (or precedes a hole on a discontig mem system).

  >> Different arches behave differently, though.  In the case of ia64,
  >> it'a always safe to prefetch (even with lfetch.fault).

  Tony> Not quite always ... this was how I found the efi trim.bottom
  Tony> bug, since Linux had allocated a pgd at 0xa00000-16k, and the
  Tony> lfetch that reached out beyond the end of the page to the
  Tony> uncacheable address 0xa00000 took an MCA.

But don't confuse cause and effect!  The MCA was caused by a bad TLB
entry.  The lfetch only triggered the latent bug (as might have a
instruction-prefetch).

  Tony> A pgd in the last page of a granule that is followed by an uncacheable
  Tony> address would do the same with lfetch.fault, wouldn't it?

No, lfetch to uncacheable translations have no effect.

	--david

Re: [PATCH 2.4.23-pre8] Remove broken prefetching in free_one_pgd()

[Pavel Machek]

Hi!

> This patch was accepted into 2.5.55, attributed to "davej@uk".

Dave Jones?n

> This code will prefetch from beyond the end of the page table
> being cleared ... which is clearly a bad thing if the page table
> in question is allocated from the last page of memory (or precedes
> a hole on a discontig mem system).

Prefetching random addresses should be safe... Well for 2.4 we
probably want to play it safe and kill it, but I guess quite a few
pieces rely on prefretch(NULL) doing nothing...


> diff -ru linux-2.4.23-pre8/mm/memory.c fix/mm/memory.c
> --- linux-2.4.23-pre8/mm/memory.c	Fri Oct 24 13:37:23 2003
> +++ fix/mm/memory.c	Fri Oct 24 13:40:47 2003
> @@ -120,10 +120,8 @@
>  	}
>  	pmd = pmd_offset(dir, 0);
>  	pgd_clear(dir);
> -	for (j = 0; j < PTRS_PER_PMD ; j++) {
> -		prefetchw(pmd+j+(PREFETCH_STRIDE/16));
> +	for (j = 0; j < PTRS_PER_PMD ; j++)
>  		free_one_pmd(pmd+j);
> -	}
>  	pmd_free(pmd);
>  }


-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [PATCH 2.4.23-pre8] Remove broken prefetching in free_one_pgd()

[Dave Jones]

On Sat, Oct 25, 2003 at 10:10:10PM +0200, Pavel Machek wrote:

 > > This patch was accepted into 2.5.55, attributed to "davej@uk".
 > Dave Jones?n

Yep.

 > > This code will prefetch from beyond the end of the page table
 > > being cleared ... which is clearly a bad thing if the page table
 > > in question is allocated from the last page of memory (or precedes
 > > a hole on a discontig mem system).
 > Prefetching random addresses should be safe...

It isn't on some CPUs.  Early athlons go bang when you prefetch
past the end of RAM into unmapped memory for eg.

		Dave

-- 
 Dave Jones     http://www.codemonkey.org.uk

Re: [PATCH 2.4.23-pre8] Remove broken prefetching in free_one_pgd()

[Pavel Machek]

Hi!

>  > > This code will prefetch from beyond the end of the page table
>  > > being cleared ... which is clearly a bad thing if the page table
>  > > in question is allocated from the last page of memory (or precedes
>  > > a hole on a discontig mem system).
>  > Prefetching random addresses should be safe...
> 
> It isn't on some CPUs.  Early athlons go bang when you prefetch
> past the end of RAM into unmapped memory for eg.

Well, broken HW :-(, but andi's patch should fix up for that.

								Pavel

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [PATCH 2.4.23-pre8] Remove broken prefetching in free_one_pgd()

[Dave Jones]

On Mon, Oct 27, 2003 at 10:52:59AM +0100, Pavel Machek wrote:

 > > It isn't on some CPUs.  Early athlons go bang when you prefetch
 > > past the end of RAM into unmapped memory for eg.
 > 
 > Well, broken HW :-(

There's a lot of it out there. If we didn't try to work around
some of it, we'd end up running on less < 10% of machines out there 8-)

> but andi's patch should fix up for that.

Different bug.

		Dave

-- 
 Dave Jones     http://www.codemonkey.org.uk

Re: [PATCH 2.4.23-pre8] Remove broken prefetching in free_one_pgd()

[Bjorn Helgaas]

On Friday 24 October 2003 4:21 pm, David Mosberger wrote:
> >>>>> On Fri, 24 Oct 2003 15:16:59 -0700, "Luck, Tony" <tony.luck@intel.com> said:
>   >> Different arches behave differently, though.  In the case of ia64,
>   >> it'a always safe to prefetch (even with lfetch.fault).
> 
>   Tony> Not quite always ... this was how I found the efi trim.bottom
>   Tony> bug, since Linux had allocated a pgd at 0xa00000-16k, and the
>   Tony> lfetch that reached out beyond the end of the page to the
>   Tony> uncacheable address 0xa00000 took an MCA.
> 
> But don't confuse cause and effect!  The MCA was caused by a bad TLB
> entry.  The lfetch only triggered the latent bug (as might have a
> instruction-prefetch).

I'm assuming that the EFI memory map trim fixes prevent the bad
TLB entry, and hence, the prefetching patch is not required by ia64
in 2.4.  Tony, let me know if otherwise.

Bjorn

RE: [PATCH 2.4.23-pre8] Remove broken prefetching in free_one_pgd()

[Luck, Tony]

> On Friday 24 October 2003 4:21 pm, David Mosberger wrote:
> > >>>>> On Fri, 24 Oct 2003 15:16:59 -0700, "Luck, Tony" 
> <tony.luck@intel.com> said:
> >   >> Different arches behave differently, though.  In the 
> case of ia64,
> >   >> it'a always safe to prefetch (even with lfetch.fault).
> > 
> >   Tony> Not quite always ... this was how I found the efi 
> trim.bottom
> >   Tony> bug, since Linux had allocated a pgd at 
> 0xa00000-16k, and the
> >   Tony> lfetch that reached out beyond the end of the page to the
> >   Tony> uncacheable address 0xa00000 took an MCA.
> > 
> > But don't confuse cause and effect!  The MCA was caused by a bad TLB
> > entry.  The lfetch only triggered the latent bug (as might have a
> > instruction-prefetch).
> 
> I'm assuming that the EFI memory map trim fixes prevent the bad
> TLB entry, and hence, the prefetching patch is not required by ia64
> in 2.4.  Tony, let me know if otherwise.

If EFI trim is doing its job (and the current version now seems
to be handling all cases correctly), then you should no longer
be able to have a TLB entry erroneously marking an uncacheable
area of memory for cacheable access ... so you can keep the prefetch
for ia64 (David pointed out that dropping this prefetch has a
severe negative impact on lmbench fork+execve test).

-Tony