linux-kernel - [PATCH v3 0/5] x86/mm: fix cpu stuck issue in __change_page_attr_set

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

Message-Id: <1534814186-37067-1-git-send-email-bin.yang@intel.com>
Date:   Tue, 21 Aug 2018 01:16:21 +0000
From:   Bin Yang <bin.yang@...el.com>
To:     tglx@...utronix.de, mingo@...nel.org, hpa@...or.com,
        x86@...nel.org, linux-kernel@...r.kernel.org, peterz@...radead.org,
        dave.hansen@...el.com, mark.gross@...el.com, bin.yang@...el.com
Subject: [PATCH v3 0/5] x86/mm: fix cpu stuck issue in __change_page_attr_set_clr


One problem is found when optimizing the x86 kernel boot time, that sometimes
the free_initmem() will take about 600ms, which is way too much for fast boot.

When changing a 4K page attr inside the large page range,
__change_page_attr() will call try_preserve_large_page() to decide
to split the big page or not. And try_preserve_large_page() will
call static_protections() to check all 4K pages inside the large page
range one by one.

free_initmem()  <-- free N pages
  free_init_pages()
    set_memory_rw()
      change_page_attr_set()
        change_page_attr_set_clr()
	  __change_page_attr_set_clr()
	    __change_page_attr() <-- loop N times
	      try_preserve_large_page()
                static_protections() <-- worst case: loop (1G/4K = 262144) * N times

The problem is,  most of the  256K * N  times of call of static_proetections()
is _not_ necessary at all, as pointed out by Thomas Gleixner :

"The current code does the static_protection() check loop _before_ checking:

  1) Whether the new and the old pgprot are the same
  
  2) Whether the address range which needs to be changed is aligned to and
     covers the full large mapping

It does the static_protections() loop before #1 and #2 which can be
optimized."

The static_protections() check loop can be optimized to check for overlapping
ranges and then check explicitly for those without any looping.

Here are 5 patches for these optimizations:

Patch 1: check whether new pgprot is same as old pgprot first to avoid
	 unnecessary static_protection() checking.

Patch 2: check whether it is whole large page attr change first to avoid
	 unnecessary static_protection() checking.

Patch 3: add help function to check specific protection flags in range

Patch 4: Optimize the static_protection() by using overlap() check instead
	 of within()

Patch 5: Add a check for catching a case where the existing mapping is wrong
	 already

The approach and some of the comments came from Thomas's email example for how
to do this. Thanks again for Thomas's kind help.

Thanks,
Bin

Bin Yang (5):
  x86/mm: avoid redundant checking if pgprot has no change
  x86/mm: avoid static_protection() checking if not whole large page
    attr change
  x86/mm: add help function to check specific protection flags in range
  x86/mm: optimize static_protection() by using overlap()
  x86/mm: add WARN_ON_ONCE() for wrong large page mapping

 arch/x86/mm/pageattr.c | 127 +++++++++++++++++++++++++++++++++----------------
 1 file changed, 86 insertions(+), 41 deletions(-)

-- 
2.7.4