Bug 13884 - x86 CPA incorrect memtype reserving using set_pages_array_xx
Summary: x86 CPA incorrect memtype reserving using set_pages_array_xx
Status: CLOSED CODE_FIX
Alias: None
Product: Platform Specific/Hardware
Classification: Unclassified
Component: i386 (show other bugs)
Hardware: All Linux
: P1 high
Assignee: platform_i386
URL:
Keywords:
: 13667 (view as bug list)
Depends on:
Blocks: 13070
  Show dependency tree
 
Reported: 2009-07-31 13:39 UTC by Thomas Hellstrom
Modified: 2009-08-05 11:41 UTC (History)
3 users (show)

See Also:
Kernel Version: 2.6.31 rc series
Subsystem:
Regression: Yes
Bisected commit-id:


Attachments
Patch to fix cpa memtype set_pages_array_xx reserving (2.92 KB, patch)
2009-07-31 13:39 UTC, Thomas Hellstrom
Details | Diff

Description Thomas Hellstrom 2009-07-31 13:39:30 UTC
Created attachment 22552 [details]
Patch to fix cpa memtype set_pages_array_xx reserving

With the set_pages_array_xx() interface, the page virtual address is used to reserve memtype ranges instead of the page physical address. This causes incorrect ranges to be reserved with various symptoms, in particular false errors.

Furthermore, highmem pages were not handled correctly. CPA should ignore them. One can argue that it is an error to call set_pages_array_xx() with highmem pages, but usually the page arrays that are used as arguments to this interface contain a mix of highmem and lowmem pages.

Patch that seems to fix the problem attached.
Comment 1 Andrew Morton 2009-07-31 23:20:24 UTC
On Fri, 31 Jul 2009 13:39:31 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=13884

That seems fairly serious and might be 2.6.31 material (or earlier!)

Please don't submit patches via bugzilla - it mucks up everyone's
processes and tools.

Please resend the patch via email as per
Documentation/SubmittingPatches.  Suitable recipients are 

"H. Peter Anvin" <hpa@zytor.com>
Ingo Molnar <mingo@elte.hu>
Thomas Gleixner <tglx@linutronix.de>
Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
"Siddha, Suresh B" <suresh.b.siddha@intel.com>
linux-kernel@vger.kernel.org

thanks.
Comment 2 Venkatesh Pallipadi 2009-07-31 23:35:23 UTC
Yuk.... Using virtual address instead of physical. Thats second bug since yesterday related to PAT. I think I should not send any patches for next few days.

Regarding the fix, may be it is better to use __pa() like rest of set_memory_* routines are using.
Comment 3 Rafael J. Wysocki 2009-08-01 11:11:39 UTC
Caused by:

commit 9ae2847591c857bed44bc094b908b412bfa1b244
Author: venkatesh.pallipadi@intel.com <venkatesh.pallipadi@intel.com>
Date:   Thu Mar 19 14:51:14 2009 -0700

    x86, PAT: Add support for struct page pointer array in cpa set_clr

First-Bad-Commit : 9ae2847591c857bed44bc094b908b412bfa1b244

Notify-Also : Ingo Molnar <mingo@elte.hu>

Handled-By : Thomas Hellstrom <thellstrom@vmware.com>
Patch : http://bugzilla.kernel.org/attachment.cgi?id=22552
Comment 4 Rafael J. Wysocki 2009-08-01 12:22:20 UTC
*** Bug 13667 has been marked as a duplicate of this bug. ***
Comment 5 Thomas Hellstrom 2009-08-03 07:16:15 UTC
(In reply to comment #2)

> Regarding the fix, may be it is better to use __pa() like rest of
> set_memory_*
> routines are using.

Those interfaces already have the virtual address so it's easy to convert to physical using __pa().
However, when converting a struct page * to a physical address I think it's confusing to first convert to virtual and then to physical. Better to use page_to_pfn() IMHO. I don't feel strongly about it, though, so if you feel like changing it please do so. As for now, I'm posting the original patch.

Thanks,
Comment 6 Frans Pop 2009-08-04 07:32:04 UTC
I think this bug is also responsible for an issue I've been seeing with KDE with both .30 and .31.

Sometimes, when I start a second X session from KDE to log in as a different user, ksplash (the kde splash screen) will sometimes end in an endless loop and race the CPU.

I've not seen that happening anymore since I applied this patch. Please push it for both .30 and .31, thanks.
Comment 7 Ingo Molnar 2009-08-04 08:12:14 UTC
> I think this bug is also responsible for an issue I've been seeing 
> with KDE with both .30 and .31.
> 
> Sometimes, when I start a second X session from KDE to log in as a 
> different user, ksplash (the kde splash screen) will sometimes end 
> in an endless loop and race the CPU.
> 
> I've not seen that happening anymore since I applied this patch. 
> Please push it for both .30 and .31, thanks.

those affected APIs are not used on v2.6.30 at all.

I've marked the fixes for a .30 backport but it would still be nice 
to double check whether you have any additional graphics drivers in 
your .30 kernel, beyond what is in vanilla v2.6.30.

	Ingo
Comment 8 Frans Pop 2009-08-04 08:44:26 UTC
On Tuesday 04 August 2009, you wrote:
> those affected APIs are not used on v2.6.30 at all.

Hmm. OK.

> I've marked the fixes for a .30 backport but it would still be nice
> to double check whether you have any additional graphics drivers in
> your .30 kernel, beyond what is in vanilla v2.6.30.

I don't. The only "weird" thing about my video setup is that I use vesafb 
for my consoles. The rest is bog standard vanilla Intel drivers.

I may be wrong about this patch being involved; it may have been fixed by 
some other commit in .31. I just noticed that I suddenly no longer needed 
to kill the ksplash process regularly and that did roughly coincide 
with -rc5 or maybe -rc4 and applying this patch.

If needed I can try to verify, but as the problem did not occur 100% 
reliably that might be tricky and would cost a lot of time.
Comment 9 Ingo Molnar 2009-08-04 08:47:00 UTC
> If needed I can try to verify, but as the problem did not occur 
> 100% reliably that might be tricky and would cost a lot of time.

Not needed - just wanted to inquire in case there are some easy 
answers.

"No kernel bugs to report" is a desired state that should not 
require any extra work on your part! ;-)

	Ingo

Note You need to log in before you can comment on or make changes to this bug.