[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <49EA5BB2.90603@kernel.org>
Date: Sat, 18 Apr 2009 16:01:06 -0700
From: Yinghai Lu <yinghai@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
CC: Ingo Molnar <mingo@...e.hu>,
Jesse Barnes <jbarnes@...tuousgeek.org>,
"H. Peter Anvin" <hpa@...or.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Thomas Gleixner <tglx@...utronix.de>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
linux-pci@...r.kernel.org, yannick.roehlly@...e.fr
Subject: Re: [PATCH] x86/pci: make pci_mem_start to be aligned only -v4
Yinghai Lu wrote:
> Linus Torvalds wrote:
>> On Sat, 18 Apr 2009, Yinghai Lu wrote:
>>> except need to change
>>>> + reserve_region_with_split(&iomem_resource, start, end, "RAM buffer");
>>> ==> > + reserve_region_with_split(&iomem_resource, start, end - 1, "RAM buffer");
>> Yes, I sent out a later email pointing that out.
>>
>>> it will make sure dynmical allocating code will not use those range.
>>>
>>> and could make e820_setup_gap much simple.
>> ACK. In fact:
>>
>>> Index: linux-2.6/arch/x86/kernel/e820.c
>>> ===================================================================
>>> --- linux-2.6.orig/arch/x86/kernel/e820.c
>>> +++ linux-2.6/arch/x86/kernel/e820.c
>>> @@ -635,14 +635,12 @@ __init void e820_setup_gap(void)
>>> #endif
>>>
>>> /*
>>> - * See how much we want to round up: start off with
>>> - * rounding to the next 1MB area.
>>> + * e820_reserve_resources_late will protect stolen RAM
>>> + * so just round it to 1M
>>> */
>>> round = 0x100000;
>>> - while ((gapsize >> 4) > round)
>>> - round += round;
>>> - /* Fun with two's complement */
>>> - pci_mem_start = (gapstart + round) & -round;
>>> +
>>> + pci_mem_start = roundup(gapstart, round);
>> You can just remove "round" entirely. It's no longer a variable, it's just
>> an odd way of saying 1M ;)
>>
>>> Ingo, can you put those two patches in tip?
>> I would suggest that we first change "reserve_region_with_split()" to not
>> recurse into the region.
>>
>> That function isn't used by anything else (we ended up using
>> "expand_to_fit()" instead in the one place that migth have used it), and
>> now th eone caller we do have would not want the recursion - if there
>> already exists a resource at the top level, we want to just avoid it.
>>
>> This - again TOTALLY UNTESTED - patch removes the "recurse into conflicts"
>> code. Comments? Testing?
>>
>> Linus
>> ---
>> kernel/resource.c | 46 ++++++++++++----------------------------------
>> 1 files changed, 12 insertions(+), 34 deletions(-)
>>
>> diff --git a/kernel/resource.c b/kernel/resource.c
>> index fd5d7d5..ac5f3a3 100644
>> --- a/kernel/resource.c
>> +++ b/kernel/resource.c
>> @@ -533,43 +533,21 @@ static void __init __reserve_region_with_split(struct resource *root,
>> res->end = end;
>> res->flags = IORESOURCE_BUSY;
>>
>> - for (;;) {
>> - conflict = __request_resource(parent, res);
>> - if (!conflict)
>> - break;
>> - if (conflict != parent) {
>> - parent = conflict;
>> - if (!(conflict->flags & IORESOURCE_BUSY))
>> - continue;
>> - }
>> -
>> - /* Uhhuh, that didn't work out.. */
>> - kfree(res);
>> - res = NULL;
>> - break;
>> - }
>> -
>> - if (!res) {
>> - /* failed, split and try again */
>> -
>> - /* conflict covered whole area */
>> - if (conflict->start <= start && conflict->end >= end)
>> - return;
>> + conflict = __request_resource(parent, res);
>> + if (!conflict)
>> + return;
>>
>> - if (conflict->start > start)
>> - __reserve_region_with_split(root, start, conflict->start-1, name);
>> - if (!(conflict->flags & IORESOURCE_BUSY)) {
>> - resource_size_t common_start, common_end;
>> + /* failed, split and try again */
>> + kfree(res);
>>
>> - common_start = max(conflict->start, start);
>> - common_end = min(conflict->end, end);
>> - if (common_start < common_end)
>> - __reserve_region_with_split(root, common_start, common_end, name);
>> - }
>> - if (conflict->end < end)
>> - __reserve_region_with_split(root, conflict->end+1, end, name);
>> - }
>> + /* conflict covered whole area */
>> + if (conflict->start <= start && conflict->end >= end)
>> + return;
>>
>> + if (conflict->start > start)
>> + __reserve_region_with_split(root, start, conflict->start-1, name);
>> + if (conflict->end < end)
>> + __reserve_region_with_split(root, conflict->end+1, end, name);
>> }
>>
>> void __init reserve_region_with_split(struct resource *root,
>
> with
> [ 0.000000] BIOS-provided physical RAM map:
> [ 0.000000] BIOS-e820: 0000000000000100 - 0000000000097400 (usable)
> [ 0.000000] BIOS-e820: 0000000000097400 - 00000000000a0000 (reserved)
> [ 0.000000] BIOS-e820: 0000000000100000 - 00000000b7fa0000 (usable)
> [ 0.000000] BIOS-e820: 00000000b7fae000 - 00000000b7fb0000 (usable)
> [ 0.000000] BIOS-e820: 00000000b7fb0000 - 00000000b7fbe000 (ACPI data)
> [ 0.000000] BIOS-e820: 00000000b7fbe000 - 00000000b7ff0000 (ACPI NVS)
> [ 0.000000] BIOS-e820: 00000000b7ff0000 - 00000000b8000000 (reserved)
> [ 0.000000] BIOS-e820: 00000000e0000000 - 00000000f0000000 (reserved)
> [ 0.000000] BIOS-e820: 00000000fec00000 - 00000000fec01000 (reserved)
> [ 0.000000] BIOS-e820: 00000000fee00000 - 00000000fef00000 (reserved)
> [ 0.000000] BIOS-e820: 00000000ff700000 - 0000000100000000 (reserved)
> [ 0.000000] BIOS-e820: 0000000100000000 - 0000002048000000 (usable)
>
> got
>
> 00000100-000973ff : System RAM
> 00097400-0009ffff : reserved
> 000a0000-000bffff : PCI Bus #00
> 000c0000-000cffff : pnp 00:0c
> 000e0000-000fffff : pnp 00:0c
> 00100000-b7f9ffff : System RAM
> 00200000-00c68f6b : Kernel code
> 00c68f6c-01332f7f : Kernel data
> 015a6000-01fcaa57 : Kernel bss
> 20000000-23ffffff : GART
> b7fa0000-b7fadfff : RAM buffer
> b7fae000-b7faffff : System RAM
> b7fb0000-b7fbdfff : ACPI Tables
> b7fbe000-b7feffff : ACPI Non-volatile Storage
> b7ff0000-b7ffffff : reserved
> ...
>
without your patch got
00000100-000973ff : System RAM
00097400-0009ffff : reserved
000a0000-000bffff : PCI Bus #00
000c0000-000cffff : pnp 00:0c
000e0000-000fffff : pnp 00:0c
00100000-b7f9ffff : System RAM
00200000-00c68f6b : Kernel code
00c68f6c-01332f7f : Kernel data
015a6000-01fcaa57 : Kernel bss
20000000-23ffffff : GART
b7fa0000-b7fadfff : RAM buffer
b7fae000-b7faffff : System RAM
b7fb0000-b7fbdfff : ACPI Tables
b7fbe000-b7feffff : ACPI Non-volatile Storage
b7ff0000-b7ffffff : reserved
b7ff0000-b7ffffff : RAM buffer
b8000000-beffffff : PCI Bus #00
bf000000-bfffffff : PCI Bus #80
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists