linux-kernel - Re: [PATCH] x86: split e820 reserved entries record to late v3

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <86802c440808281802w7994d7cdjcd1f36fe3381ac76@mail.gmail.com>
Date:	Thu, 28 Aug 2008 18:02:48 -0700
From:	"Yinghai Lu" <yhlu.kernel@...il.com>
To:	"Linus Torvalds" <torvalds@...ux-foundation.org>
Cc:	"Ingo Molnar" <mingo@...e.hu>,
	"Thomas Gleixner" <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	"Andrew Morton" <akpm@...ux-foundation.org>,
	"Jesse Barnes" <jbarnes@...tuousgeek.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86: split e820 reserved entries record to late v3

On Thu, Aug 28, 2008 at 5:53 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
>
> On Thu, 28 Aug 2008, Yinghai Lu wrote:
>>  /*
>>   * Mark e820 reserved areas as busy for the resource manager.
>>   */
>> +struct resource __initdata *e820_res;
>
> Btw, make this one "static" - I don't think anybody outside of this file
> should ever use it anyway.

yes

>
> And this:
>
>> +             if (e820.map[i].type != E820_RESERVED || res->start < (1ULL<<20))
>> +                     insert_resource(&iomem_resource, res);
>
> is probably worthy of a comment.

how about
/*
 * don't register the region that could be conflicted with pci device
BAR resource
 * and insert them later.
 */


>
> And finally:
>
>> +void __init e820_reserve_resources_late(void)
>> +{
>> +     int i;
>> +     struct resource *res;
>> +
>> +     res = e820_res;
>> +     for (i = 0; i < e820.nr_map; i++) {
>> +             if (e820.map[i].type == E820_RESERVED && res->start >= (1ULL<<20))
>> +                     insert_resource(&iomem_resource, res);
>
> instead of duplicating the conditional and always having to worry about it
> matching, how about just making it
>
>                if (!res->parent)
>                        insert_resource(&iomem_resource, res);
>
> which basically says that "if it's not already inserted, try to re-insert
> it now".
there could be one is all zero, when 32bit/!64_RESOURCE.
so could change it to
                if (!res->parent && res->end)
                       insert_resource(&iomem_resource, res);

YH
--
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/