linux-kernel - Re: [PATCH RESEND 2/3] x86, mm: Update min_pfn_mapped in add_pfn_range

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]

Date:	Mon, 2 Sep 2013 23:34:52 -0700
From:	Yinghai Lu <yinghai@...nel.org>
To:	Tang Chen <tangchen@...fujitsu.com>
Cc:	Ingo Molnar <mingo@...hat.com>, "H. Peter Anvin" <hpa@...or.com>,
	Pekka Enberg <penberg@...nel.org>,
	Jacob Shin <jacob.shin@....com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Len Brown <lenb@...nel.org>, "Rafael J. Wysocki" <rjw@...k.pl>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
	"the arch/x86 maintainers" <x86@...nel.org>
Subject: Re: [PATCH RESEND 2/3] x86, mm: Update min_pfn_mapped in add_pfn_range_mapped().

On Mon, Sep 2, 2013 at 10:38 PM, Tang Chen <tangchen@...fujitsu.com> wrote:
> On 09/03/2013 10:48 AM, Yinghai Lu wrote:
>>
>> On Mon, Sep 2, 2013 at 6:06 PM, Tang Chen<tangchen@...fujitsu.com>  wrote:
>>>
>>> Hi Yinghai,
>>>
>>> On 09/03/2013 02:41 AM, Yinghai Lu wrote:
>>
>>
>>> How about change the "for (from low to high)" in
>>> init_range_memory_mapping()
>>> to
>>> "for_rev(from high to low)" ?
>>> Then we can update min_pfn_mapped in add_pfn_range_mapped().
>>>
>>> And also, the outer loop is from high to low, we can change the inner
>>> loop
>>> to be from high
>>> to low too.
>>
>>
>> No. there is other reason for doing local from low to high.
>>
>> kernel_physical_mapping_init() could clear some mapping near the end
>> of PUG/PMD entries but not the head.
>
>
> Thanks for your explanation. But sorry, I'd like to understand it more
> clearly.
>
> Are you talking about the following code ?
>         phys_pud_init()
>         {
>                 if (addr >= end) {
>                         if (!after_bootmem &&
>                             !e820_any_mapped(addr & PUD_MASK, next,
> E820_RAM) &&
>                             !e820_any_mapped(addr & PUD_MASK, next,
> E820_RESERVED_KERN))
>                                 set_pud(pud, __pud(0));
>                         continue;
>                 }
>         }
> It will clear the PUD/PMD out of range.
>
>
> But,
>
> init_mem_mapping()
> {
>     while (from high to low) {
>         init_range_memory_mapping()
>         {
>             for (from low to high) {
> /* I'm saying changing this loop */
>                 init_memory_mapping()
>                 {
>                     for () {
> /* Not this one */
>                         kernel_physical_mapping_init();
>                     }
>                     add_pfn_range_mapped();
>                 }
>             }
>         }
>     }
> }
>
> I'm saying changing the outer loop in init_range_memory_mapping(), not the
> one in init_memory_mapping().
> I think it is OK to call init_memory_mapping() with any order. The loop is
> out of init_memory_mapping(), right ?
>
> In init_memory_mapping(), it is still from low to high. But when the
> kernel_physical_mapping_init() finished,
> we can update min_pfn_mapped in add_pfn_range_mapped() because the outer
> loop is from high to low.
>
> Am I missing something here ?  Please tell me.

Yes, that looks ok,

but will make the code more hard to understand, aka more dependency.

the only purpose for min_pfn_mapped is for control allocation for
alloc_low_pages.

so put it's updating in init_mem_mapping is clear and less twisting.

also in my patchset that put page table in local node, min_pfn_mapped
is replaced by
local_min_pfn_mapped per node.

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