2008-01-31 07:38:00

by Huang, Ying

[permalink] [raw]
Subject: [PATCH 2/5] x86: c_p_a clflush_cache_range fix

Because in i386 early boot stage, boot_cpu_data may be not available,
which makes clflush_cach_range() into infinite loop, which is called
by change_page_attr(). This patch fixes this by providing a default
clflush_size of 64. But the better method may be providing a
early_identify_cpu() for i386.

Signed-off-by: Huang Ying <[email protected]>

---
arch/x86/mm/pageattr.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -36,11 +36,14 @@ within(unsigned long addr, unsigned long
*/
void clflush_cache_range(void *vaddr, unsigned int size)
{
+ int clflush_size;
void *vend = vaddr + size - 1;

mb();

- for (; vaddr < vend; vaddr += boot_cpu_data.x86_clflush_size)
+ /* In boot stage, boot_cpu_data may be not available */
+ clflush_size = boot_cpu_data.x86_clflush_size ? : 64;
+ for (; vaddr < vend; vaddr += clflush_size)
clflush(vaddr);
/*
* Flush any possible final partial cacheline:


2008-01-31 07:42:10

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH 2/5] x86: c_p_a clflush_cache_range fix

On Thursday 31 January 2008 08:36:02 Huang, Ying wrote:
> Because in i386 early boot stage, boot_cpu_data may be not available,
> which makes clflush_cach_range() into infinite loop, which is called
> by change_page_attr(). This patch fixes this by providing a default
> clflush_size of 64. But the better method may be providing a
> early_identify_cpu() for i386.

There already is one. arch/x86/kernel/cpu/common.c:early_cpu_detect
Just set it to 64 there.

-Andi

2008-01-31 08:28:30

by Huang, Ying

[permalink] [raw]
Subject: Re: [PATCH 2/5] x86: c_p_a clflush_cache_range fix

On Thu, 2008-01-31 at 08:40 +0100, Andi Kleen wrote:
> On Thursday 31 January 2008 08:36:02 Huang, Ying wrote:
> > Because in i386 early boot stage, boot_cpu_data may be not available,
> > which makes clflush_cach_range() into infinite loop, which is called
> > by change_page_attr(). This patch fixes this by providing a default
> > clflush_size of 64. But the better method may be providing a
> > early_identify_cpu() for i386.
>
> There already is one. arch/x86/kernel/cpu/common.c:early_cpu_detect
> Just set it to 64 there.

Thanks. The following is a new patch based on your reminding.

--------------------------------------------------------------------

Because in i386 early boot stage, boot_cpu_data may be not available,
which makes clflush_cach_range() into infinite loop, which is called
by change_page_attr(). This patch fixes this by setting
boot_cpu_data.x86_clflush_size in early_cpu_detect().

Signed-off-by: Huang Ying <[email protected]>

---
arch/x86/kernel/cpu/common.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -274,8 +274,10 @@ void __init cpu_detect(struct cpuinfo_x8
if (c->x86 >= 0x6)
c->x86_model += ((tfms >> 16) & 0xF) << 4;
c->x86_mask = tfms & 15;
- if (cap0 & (1<<19))
+ if (cap0 & (1<<19)) {
c->x86_cache_alignment = ((misc >> 8) & 0xff) * 8;
+ c->x86_clflush_size = ((misc >> 8) & 0xff) * 8;
+ }
}
}
static void __cpuinit early_get_cap(struct cpuinfo_x86 *c)
@@ -317,6 +319,7 @@ static void __init early_cpu_detect(void
struct cpuinfo_x86 *c = &boot_cpu_data;

c->x86_cache_alignment = 32;
+ c->x86_clflush_size = 32;

if (!have_cpuid_p())
return;

2008-01-31 12:08:58

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/5] x86: c_p_a clflush_cache_range fix


* Huang, Ying <[email protected]> wrote:

> +++ b/arch/x86/kernel/cpu/common.c
> @@ -274,8 +274,10 @@ void __init cpu_detect(struct cpuinfo_x8
> if (c->x86 >= 0x6)
> c->x86_model += ((tfms >> 16) & 0xF) << 4;
> c->x86_mask = tfms & 15;
> - if (cap0 & (1<<19))
> + if (cap0 & (1<<19)) {
> c->x86_cache_alignment = ((misc >> 8) & 0xff) * 8;
> + c->x86_clflush_size = ((misc >> 8) & 0xff) * 8;
> + }

thanks!

> @@ -317,6 +319,7 @@ static void __init early_cpu_detect(void
> struct cpuinfo_x86 *c = &boot_cpu_data;
>
> c->x86_cache_alignment = 32;
> + c->x86_clflush_size = 32;

i suspect 32 is a good lower limit (it's not a big issue to do too
finegrained flushes, and CLFLUSH is valid with arbitrary alignment) -
and this will be overriden with 64 later on.

Ingo