linux-kernel - Re: [PATCH 06/19] x86, mm: setup page table in top-down

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAE9FiQX-JBHvjqLyjutKfBUp2DQKEwVMGEkj7Wi6yijAERhP6w@mail.gmail.com>
Date:	Fri, 19 Oct 2012 09:41:22 -0700
From:	Yinghai Lu <yinghai@...nel.org>
To:	Stefano Stabellini <stefano.stabellini@...citrix.com>
Cc:	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...e.hu>,
	"H. Peter Anvin" <hpa@...or.com>, Jacob Shin <jacob.shin@....com>,
	Tejun Heo <tj@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 06/19] x86, mm: setup page table in top-down

On Fri, Oct 19, 2012 at 9:24 AM, Stefano Stabellini
<stefano.stabellini@...citrix.com> wrote:
> On Thu, 18 Oct 2012, Yinghai Lu wrote:
>> Get pgt_buf early from BRK, and use it to map PMD_SIZE from top at first.
>> then use mapped pages to map more range below, and keep looping until
>> all pages get mapped.
>>
>> alloc_low_page will use page from BRK at first, after that buff is used up,
>> will use memblock to find and reserve page for page table usage.
>>
>> At last we could get rid of calculation and find early pgt related code.
>>
>> -v2: update to after fix_xen change,
>>      also use MACRO for initial pgt_buf size and add comments with it.
>> -v3: skip big reserved range in memblock.reserved near end.
>> -v4: don't need fix_xen change now.
>>
>> Suggested-by: "H. Peter Anvin" <hpa@...or.com>
>> Signed-off-by: Yinghai Lu <yinghai@...nel.org>
>
> The series is starting to get in good shape!
> I tested it on a 2G and an 8G VM and it seems to be working fine.

domU on 32bit and 64bit?

>
>
> The most important thing to do now is testing it on different machines
> (with and without xen) and writing better commit messages.
> We can help you with the testing but you really need to write better
> docs.

I tested on native one on 32bit, and 64bit.

Also xen dom0 32bit and 64bit.

Changelog is always problem now. Looks like everyone is complaining about that.

Actually I already tried my best on that, really don't know what to do next.

>
> In particular you should state in clear letters that alloc_low_page is
> always going to return a page that is already mapped. You should write
> it both in the commit message and in the code as a comment.
> This is particularly important because it is going to become part of the
> interface between the common x86 code and the other x86 subsystems like
> Xen.

alloc_low_page() is used in arch/x86/mm/init*.c. How come it becomes
interface to
other subsystem?

I'm not sure if we really need that comment in code for that:
---
the page that alloc_low_page return is directed mapped already, could
use virtual address
to access it.
---

>
> Also, it wouldn't hurt to explain the overall design at the beginning of
> the series: I shouldn't have to read your code to understand what you
> are doing. I should read the description of the patch, understand what
> you are doing, then go and check if the code actually does what you say
> it does.

Actually I really don't like to read too long change log in commit.
Changelog should be concise and precise.
code change is more straightforward to me.

Thanks

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/