linux-kernel - Re: [PATCH 22/26] x86/mm: add sync_global_pgds() for configuration with 5-level paging

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: <20170313072227.GB28726@gmail.com>
Date:   Mon, 13 Mar 2017 08:22:27 +0100
From:   Ingo Molnar <mingo@...nel.org>
To:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>, x86@...nel.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Arnd Bergmann <arnd@...db.de>,
        "H. Peter Anvin" <hpa@...or.com>, Andi Kleen <ak@...ux.intel.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Andy Lutomirski <luto@...capital.net>,
        Michal Hocko <mhocko@...e.com>, linux-arch@...r.kernel.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 22/26] x86/mm: add sync_global_pgds() for configuration
 with 5-level paging

* Kirill A. Shutemov <kirill.shutemov@...ux.intel.com> wrote:

> This basically restores slightly modified version of original
> sync_global_pgds() which we had before foldedl p4d was introduced.

Please read your changelogs, I saw several typos/grammar mistakes in earlier 
patches. The one here is:

	s/foldedl/folded

> +	for (address = start; address <= end && address >= start;
> +			address += PGDIR_SIZE) {

Please don't address col80 checkpatch warnings by breaking the line in such an 
ugly way! Find another method, or just leave it slightly longer than 80 cols.

This one could probably be solved by:

	s/address/addr

... which is the canonical variable name for such iterations anyway.

> +			/* the pgt_lock only for Xen */

Please use whole sentences in comments, and please capitalize them properly.

I.e. here:

			/* We acquire the pgt_lock only for Xen: */

> +				BUG_ON(pgd_page_vaddr(*pgd)
> +						!= pgd_page_vaddr(*pgd_ref));

Ugly col80 artifact ...

Please review the rest of the series for similar patterns as well, and please only 
post 5-10 patches in the next submission - we'll review and apply them step by 
step.

Thanks,

	Ingo