mtrr failure, 3.1-rc1

mtrr failure, 3.1-rc1

[Anders Eriksson]

My old P-II has 640MB ram, but the bios only sets up WC for the first 512, so i 
add this early in the boot process:
echo "base=0x20000000 size=0x8000000 type=write-back" > /proc/mtrr

For reasons I have forgot I also have to add 'disable_mtrr_check' to the 
command line. Works great.

As of merge dc43d9fa73d82083656fb9c02f4823bcdcfb9f91 This no longer works.
Adding that line to /proc/mtrr no longer has any effect (cat /proc/mtrr shows
only the reg00 entry).

Reverting that merge on top of rc3 brings things back to normal and /proc/mtrr 
shows:
reg00: base=0x000000000 (    0MB), size=  512MB, count=1: write-back
reg01: base=0x048000000 ( 1152MB), size=  128MB, count=1: write-combining
reg02: base=0x020000000 (  512MB), size=  128MB, count=2: write-back

Any ideas how to track this down futher? Would be good to get it fixed for 3.1

-Anders

Re: mtrr failure, 3.1-rc1

[Linus Torvalds]

Hmm. Some of those commits are marked for stable too.

I'm adding appropriate parties to the discussion (authors and
sign-offs from the five commits that that merge brings in), but it
would also be great if you could bisect exactly which commit broke for
you. You can make it quick by just doing

  git bisect start
  git bisect bad dc43d9fa73d82083656fb9c02f4823bcdcfb9f91
  git bisect good dc43d9fa73d82083656fb9c02f4823bcdcfb9f91^

and it should take just a couple of tests. I *assume* it's the
stop_machine changes by Tejun &co, but who knows..

Anders, is this a single-CPU system?

                            Linus

On Thu, Aug 25, 2011 at 12:03 AM, Anders Eriksson <aeriksson@fastmail.fm> wrote:
>
> My old P-II has 640MB ram, but the bios only sets up WC for the first 512, so i
> add this early in the boot process:
> echo "base=0x20000000 size=0x8000000 type=write-back" > /proc/mtrr
>
> For reasons I have forgot I also have to add 'disable_mtrr_check' to the
> command line. Works great.
>
> As of merge dc43d9fa73d82083656fb9c02f4823bcdcfb9f91 This no longer works.
> Adding that line to /proc/mtrr no longer has any effect (cat /proc/mtrr shows
> only the reg00 entry).
>
> Reverting that merge on top of rc3 brings things back to normal and /proc/mtrr
> shows:
> reg00: base=0x000000000 (    0MB), size=  512MB, count=1: write-back
> reg01: base=0x048000000 ( 1152MB), size=  128MB, count=1: write-combining
> reg02: base=0x020000000 (  512MB), size=  128MB, count=2: write-back
>
> Any ideas how to track this down futher? Would be good to get it fixed for 3.1
>
> -Anders
>
>
>

Re: mtrr failure, 3.1-rc1

[Tejun Heo]

Hello,

On Thu, Aug 25, 2011 at 08:55:12AM -0700, Linus Torvalds wrote:
> Hmm. Some of those commits are marked for stable too.
> 
> I'm adding appropriate parties to the discussion (authors and
> sign-offs from the five commits that that merge brings in), but it
> would also be great if you could bisect exactly which commit broke for
> you. You can make it quick by just doing
> 
>   git bisect start
>   git bisect bad dc43d9fa73d82083656fb9c02f4823bcdcfb9f91
>   git bisect good dc43d9fa73d82083656fb9c02f4823bcdcfb9f91^
> 
> and it should take just a couple of tests. I *assume* it's the
> stop_machine changes by Tejun &co, but who knows..
> 
> Anders, is this a single-CPU system?

Indeed, while removing custom rendezvous code and switching to
stop_machine, commit 192d8857427d "x86, mtrr: use stop_machine APIs
for doing MTRR rendezvous" completely dropped mtrr setting code on
!CONFIG_SMP.

Anders, if your kernel didn't have SMP turned on, can you please
verify the following patch fixes the problem?

Thanks.

diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index 08119a3..6b96110 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -149,7 +149,6 @@ struct set_mtrr_data {
  */
 static int mtrr_rendezvous_handler(void *info)
 {
-#ifdef CONFIG_SMP
 	struct set_mtrr_data *data = info;
 
 	/*
@@ -171,7 +170,6 @@ static int mtrr_rendezvous_handler(void *info)
 	} else if (mtrr_aps_delayed_init || !cpu_online(smp_processor_id())) {
 		mtrr_if->set_all();
 	}
-#endif
 	return 0;
 }
 

Re: mtrr failure, 3.1-rc1

[anders eriksson]

On 2011-08-25 18:46, Tejun Heo wrote:
> Indeed, while removing custom rendezvous code and switching to
> stop_machine, commit 192d8857427d "x86, mtrr: use stop_machine APIs
> for doing MTRR rendezvous" completely dropped mtrr setting code on
> !CONFIG_SMP.
>
> Anders, if your kernel didn't have SMP turned on, can you please
> verify the following patch fixes the problem?
Yes. It's single CPU compiled with !CONFIG_SMP. I'm compiling w/ this 
patch right now.
Takes a while on vintage equipment... ;-)

Re: mtrr failure, 3.1-rc1

[Suresh Siddha]

On Thu, 2011-08-25 at 09:46 -0700, Tejun Heo wrote:
> Hello,
> 
> On Thu, Aug 25, 2011 at 08:55:12AM -0700, Linus Torvalds wrote:
> > Hmm. Some of those commits are marked for stable too.
> > 
> > I'm adding appropriate parties to the discussion (authors and
> > sign-offs from the five commits that that merge brings in), but it
> > would also be great if you could bisect exactly which commit broke for
> > you. You can make it quick by just doing
> > 
> >   git bisect start
> >   git bisect bad dc43d9fa73d82083656fb9c02f4823bcdcfb9f91
> >   git bisect good dc43d9fa73d82083656fb9c02f4823bcdcfb9f91^
> > 
> > and it should take just a couple of tests. I *assume* it's the
> > stop_machine changes by Tejun &co, but who knows..
> > 
> > Anders, is this a single-CPU system?
> 
> Indeed, while removing custom rendezvous code and switching to
> stop_machine, commit 192d8857427d "x86, mtrr: use stop_machine APIs
> for doing MTRR rendezvous" completely dropped mtrr setting code on
> !CONFIG_SMP.

oops. My bad. Linus, Bad commit is not marked for stable.

I was able to reproduce the issue and tested the below patch. It looks
good.

Thanks.

Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>

> 
> Anders, if your kernel didn't have SMP turned on, can you please
> verify the following patch fixes the problem?
> 
> Thanks.
> 
> diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
> index 08119a3..6b96110 100644
> --- a/arch/x86/kernel/cpu/mtrr/main.c
> +++ b/arch/x86/kernel/cpu/mtrr/main.c
> @@ -149,7 +149,6 @@ struct set_mtrr_data {
>   */
>  static int mtrr_rendezvous_handler(void *info)
>  {
> -#ifdef CONFIG_SMP
>  	struct set_mtrr_data *data = info;
>  
>  	/*
> @@ -171,7 +170,6 @@ static int mtrr_rendezvous_handler(void *info)
>  	} else if (mtrr_aps_delayed_init || !cpu_online(smp_processor_id())) {
>  		mtrr_if->set_all();
>  	}
> -#endif
>  	return 0;
>  }
>  

Re: mtrr failure, 3.1-rc1

[Linus Torvalds]

On Thu, Aug 25, 2011 at 10:43 AM, Suresh Siddha
<suresh.b.siddha@intel.com> wrote:
>
> I was able to reproduce the issue and tested the below patch. It looks
> good.
>
> Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>

Tejun, sign-off?

                   Linus

[PATCH] mtrr: fix UP breakage caused during switch to stop_machine

[Tejun Heo]

While removing custom rendezvous code and switching to stop_machine,
commit 192d8857427d "x86, mtrr: use stop_machine APIs for doing MTRR
rendezvous" completely dropped mtrr setting code on !CONFIG_SMP
breaking MTRR settting on UP.  Fix it by removing the incorrect
CONFIG_SMP.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Anders Eriksson <aeriksson@fastmail.fm>
Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 arch/x86/kernel/cpu/mtrr/main.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index 08119a3..6b96110 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -149,7 +149,6 @@ struct set_mtrr_data {
  */
 static int mtrr_rendezvous_handler(void *info)
 {
-#ifdef CONFIG_SMP
 	struct set_mtrr_data *data = info;
 
 	/*
@@ -171,7 +170,6 @@ static int mtrr_rendezvous_handler(void *info)
 	} else if (mtrr_aps_delayed_init || !cpu_online(smp_processor_id())) {
 		mtrr_if->set_all();
 	}
-#endif
 	return 0;
 }
 

Re: [PATCH] mtrr: fix UP breakage caused during switch to stop_machine

[H. Peter Anvin]

On 08/25/2011 10:46 AM, Tejun Heo wrote:
> While removing custom rendezvous code and switching to stop_machine,
> commit 192d8857427d "x86, mtrr: use stop_machine APIs for doing MTRR
> rendezvous" completely dropped mtrr setting code on !CONFIG_SMP
> breaking MTRR settting on UP.  Fix it by removing the incorrect
> CONFIG_SMP.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: Anders Eriksson <aeriksson@fastmail.fm>
> Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>

Acked-by: H. Peter Anvin <hpa@zytor.com>

Linus, are you picking this up from email or should I put it in x86/urgent?

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] mtrr: fix UP breakage caused during switch to stop_machine

[Linus Torvalds]

On Thu, Aug 25, 2011 at 11:00 AM, H. Peter Anvin <hpa@zytor.com> wrote:
>
> Linus, are you picking this up from email or should I put it in x86/urgent?

I took it directly,

                    Linus

Re: mtrr failure, 3.1-rc1

[anders eriksson]

On 2011-08-25 18:46, Tejun Heo wrote:
> Anders, if your kernel didn't have SMP turned on, can you please
> verify the following patch fixes the problem?
>
> Thanks.
>
> diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
> index 08119a3..6b96110 100644
> --- a/arch/x86/kernel/cpu/mtrr/main.c
> +++ b/arch/x86/kernel/cpu/mtrr/main.c
> @@ -149,7 +149,6 @@ struct set_mtrr_data {
>    */
>   static int mtrr_rendezvous_handler(void *info)
>   {
> -#ifdef CONFIG_SMP
>   	struct set_mtrr_data *data = info;
>
>   	/*
> @@ -171,7 +170,6 @@ static int mtrr_rendezvous_handler(void *info)
>   	} else if (mtrr_aps_delayed_init || !cpu_online(smp_processor_id())) {
>   		mtrr_if->set_all();
>   	}
> -#endif
>   	return 0;
>   }
>
Tested. Works as expected. Thanks!

-A

Re: mtrr failure, 3.1-rc1

[Bob Tracy]

On Thu, Aug 25, 2011 at 09:03:35AM +0200, Anders Eriksson wrote:
> (mtrr issues)

Similar MTRR issue here with a K6-III/450, which I noticed when the X11
server was no longer able to allocate/configure a MTRR.  "cat /proc/mtrr"
shows nothing configured regardless of whether the X11 server is running.
I only see a diagnostic message to the effect of the MTRR not existing
when X11 exits.

Possibly significant: prior 3.X kernel versions would cause the X11
server to exit with a corrupted display after about 3-5 invocations.
Machine was otherwise usable, but had to reboot remotely to recover the
console display.  The "no MTRR" issue in 3.1-rc1 has thus far meant no
display corruption on X11 server exit: have run "startx" at least 10
times since booting 3.1-rc1.

-- 
------------------------------------------------------------------------
Bob Tracy          |  "Every normal man must be tempted at times to spit
rct@frus.com       |   upon his hands, hoist the black flag, and begin
                   |   slitting throats."	-- H.L. Mencken
------------------------------------------------------------------------