MTRR driver model support broken on SMP.

MTRR driver model support broken on SMP.

[Nigel Cunningham]

Hi all.

MTRR driver support is broken on SMP because it calls smp functions with
interrupts disabled, but interrupts should be disabled because it is
called via device_power_up.

Badness in smp_call_function at arch/i386/kernel/smp.c:565
 [<c0107f88>] dump_stack+0x1e/0x20
 [<c0116927>] smp_call_function+0x12b/0x137
 [<c011063f>] set_mtrr+0x67/0x121
 [<c0110c9b>] mtrr_restore+0x4f/0x73
 [<c0219c0f>] sysdev_resume+0x6f/0xf0
 [<c021d591>] device_power_up+0x8/0xf
 [<f882d6f9>] suspend_drivers_resume+0x1a/0x49 [suspend_core]
 [<f882e2ea>] do_suspend2_resume_2+0xf6/0x11d [suspend_core]
 [<c02577f6>] do_suspend2_lowlevel+0x716/0x71c
 [<f882dd6d>] save_image+0x1b8/0x252 [suspend_core]
 [<f882f357>] do_activate+0x934/0x973 [suspend_core]
 [<c013cf83>] software_suspend_pending+0x75/0x77
 [<c013cfdc>] proc_software_suspend_pending+0x8/0xd
 [<c0191fc9>] proc_file_write+0x3b/0x3d
 [<c015fe3a>] vfs_write+0xa2/0x10e
 [<c015ff42>] sys_write+0x3f/0x5d
 [<c01070e1>] sysenter_past_esp+0x52/0x71

Regards,

Nigel

Re: MTRR driver model support broken on SMP.

[Zwane Mwaikambo]

On Wed, 4 Aug 2004, Nigel Cunningham wrote:

> MTRR driver support is broken on SMP because it calls smp functions with
> interrupts disabled, but interrupts should be disabled because it is
> called via device_power_up.
>
> Badness in smp_call_function at arch/i386/kernel/smp.c:565
>  [<c0107f88>] dump_stack+0x1e/0x20
>  [<c0116927>] smp_call_function+0x12b/0x137
>  [<c011063f>] set_mtrr+0x67/0x121
>  [<c0110c9b>] mtrr_restore+0x4f/0x73
>  [<c0219c0f>] sysdev_resume+0x6f/0xf0
>  [<c021d591>] device_power_up+0x8/0xf

Looking at this i'm really curious as to whether we need to bother at all,
can you remove the mtrr restore code and then compare /proc/mtrr before
and after suspending.

Re: MTRR driver model support broken on SMP.

[Nigel Cunningham]

Hi.

On Wed, 2004-08-04 at 15:23, Zwane Mwaikambo wrote:
> On Wed, 4 Aug 2004, Nigel Cunningham wrote:
> 
> > MTRR driver support is broken on SMP because it calls smp functions with
> > interrupts disabled, but interrupts should be disabled because it is
> > called via device_power_up.
> >
> > Badness in smp_call_function at arch/i386/kernel/smp.c:565
> >  [<c0107f88>] dump_stack+0x1e/0x20
> >  [<c0116927>] smp_call_function+0x12b/0x137
> >  [<c011063f>] set_mtrr+0x67/0x121
> >  [<c0110c9b>] mtrr_restore+0x4f/0x73
> >  [<c0219c0f>] sysdev_resume+0x6f/0xf0
> >  [<c021d591>] device_power_up+0x8/0xf
> 
> Looking at this i'm really curious as to whether we need to bother at all,
> can you remove the mtrr restore code and then compare /proc/mtrr before
> and after suspending.

I haven't had problems but do remember 2.4 users who had trouble with X
before code to save and restore mtrrs was added.

Regards,

Nigel

Re: MTRR driver model support broken on SMP.

[Zwane Mwaikambo]

On Wed, 4 Aug 2004, Nigel Cunningham wrote:

> > Looking at this i'm really curious as to whether we need to bother at all,
> > can you remove the mtrr restore code and then compare /proc/mtrr before
> > and after suspending.
>
> I haven't had problems but do remember 2.4 users who had trouble with X
> before code to save and restore mtrrs was added.

Ahh yes, X11 will create an additional entry on startup whilst the
boot MTRR settings don't have it.

Re: MTRR driver model support broken on SMP.

[Nigel Cunningham]

Hi.

On Wed, 2004-08-04 at 15:31, Zwane Mwaikambo wrote:
> On Wed, 4 Aug 2004, Nigel Cunningham wrote:
> 
> > > Looking at this i'm really curious as to whether we need to bother at all,
> > > can you remove the mtrr restore code and then compare /proc/mtrr before
> > > and after suspending.
> >
> > I haven't had problems but do remember 2.4 users who had trouble with X
> > before code to save and restore mtrrs was added.
> 
> Ahh yes, X11 will create an additional entry on startup whilst the
> boot MTRR settings don't have it.

Okay. So the question then is how to get them restored. I don't
understand much about the driver model, but it seems to me that all we
should need it get to mtrr save/restore done from the
drivers_suspend/resume calls, which do have interrupts enabled. But how
to achieve that...

Nigel

Re: MTRR driver model support broken on SMP.

[Zwane Mwaikambo]

On Wed, 4 Aug 2004, Nigel Cunningham wrote:

> Okay. So the question then is how to get them restored. I don't
> understand much about the driver model, but it seems to me that all we
> should need it get to mtrr save/restore done from the
> drivers_suspend/resume calls, which do have interrupts enabled. But how
> to achieve that...

Indeed, i can't see an easy way without writing it up as a "normal"
driver. It'd be nice if we could avoid adding more infrastructure, i'll
give it some more thought.

Re: MTRR driver model support broken on SMP.

[Nigel Cunningham]

Hi.

On Thu, 2004-08-05 at 07:15, Zwane Mwaikambo wrote:
> On Wed, 4 Aug 2004, Nigel Cunningham wrote:
> 
> > Okay. So the question then is how to get them restored. I don't
> > understand much about the driver model, but it seems to me that all we
> > should need it get to mtrr save/restore done from the

Wow. That's atrocious typing. Think more, type slower :>

> > drivers_suspend/resume calls, which do have interrupts enabled. But how
> > to achieve that...
> 
> Indeed, i can't see an easy way without writing it up as a "normal"
> driver. It'd be nice if we could avoid adding more infrastructure, i'll
> give it some more thought.

Me too. I have another patch I'm finishing off, then I'll see if I can
come up with one for this.

Nigel