public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Bin Yang <bin.yang@intel.com>
To: tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com,
	x86@kernel.org, bin.yang@intel.com, linux-kernel@vger.kernel.org
Subject: [PATCH v2] x86/mm: fix cpu stuck issue in __change_page_attr_set_clr
Date: Thu,  5 Jul 2018 05:47:16 +0000	[thread overview]
Message-ID: <1530769636-26603-1-git-send-email-bin.yang@intel.com> (raw)

When changing a 4K page attr inside 1G/2M 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. The check loop is very inefficient. In worst case,
static_protections() will be called for 1G/4K (262144) times.

This issue can be triggered by free_initmem() during kernel boot.
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 262144 * N times

Instead of checking all pages, it can only check one page for same
overlapping range. This patch enhances static_protections() to return the
page num that all following pages are in the same overlapping range with
same protection flags. It can reduce the check loop from 262144 to less
than 10 times.

Signed-off-by: Bin Yang <bin.yang@intel.com>
---
 arch/x86/mm/pageattr.c | 72 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 58 insertions(+), 14 deletions(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 3bded76e..dee23ef 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -292,17 +292,27 @@ static void cpa_flush_array(unsigned long *start, int numpages, int cache,
  * checks and fixes these known static required protection bits.
  */
 static inline pgprot_t static_protections(pgprot_t prot, unsigned long address,
-				   unsigned long pfn)
+				   unsigned long pfn, unsigned long *page_num)
 {
 	pgprot_t forbidden = __pgprot(0);
+	unsigned long tmp;
+	unsigned long num = PUD_PAGE_SIZE >> PAGE_SHIFT;
 
 	/*
 	 * The BIOS area between 640k and 1Mb needs to be executable for
 	 * PCI BIOS based config access (CONFIG_PCI_GOBIOS) support.
 	 */
 #ifdef CONFIG_PCI_BIOS
-	if (pcibios_enabled && within(pfn, BIOS_BEGIN >> PAGE_SHIFT, BIOS_END >> PAGE_SHIFT))
-		pgprot_val(forbidden) |= _PAGE_NX;
+	if (pcibios_enabled) {
+		tmp = (BIOS_BEGIN >> PAGE_SHIFT) > pfn ?
+			(BIOS_BEGIN >> PAGE_SHIFT) - pfn : ULONG_MAX;
+		if (within(pfn, BIOS_BEGIN >> PAGE_SHIFT,
+		    BIOS_END >> PAGE_SHIFT)) {
+			pgprot_val(forbidden) |= _PAGE_NX;
+			tmp = (BIOS_END >> PAGE_SHIFT) - pfn;
+		}
+		num = num > tmp ? tmp : num;
+	}
 #endif
 
 	/*
@@ -310,18 +320,30 @@ static inline pgprot_t static_protections(pgprot_t prot, unsigned long address,
 	 * Does not cover __inittext since that is gone later on. On
 	 * 64bit we do not enforce !NX on the low mapping
 	 */
-	if (within(address, (unsigned long)_text, (unsigned long)_etext))
+	tmp = (unsigned long)_text > address ?
+		((unsigned long)_text - address) >> PAGE_SHIFT : ULONG_MAX;
+	if (within(address, (unsigned long)_text, (unsigned long)_etext)) {
 		pgprot_val(forbidden) |= _PAGE_NX;
+		tmp = ((unsigned long)_etext - address) >> PAGE_SHIFT;
+	}
+	num = num > tmp ? tmp : num;
 
 	/*
 	 * The .rodata section needs to be read-only. Using the pfn
 	 * catches all aliases.  This also includes __ro_after_init,
 	 * so do not enforce until kernel_set_to_readonly is true.
 	 */
-	if (kernel_set_to_readonly &&
-	    within(pfn, __pa_symbol(__start_rodata) >> PAGE_SHIFT,
-		   __pa_symbol(__end_rodata) >> PAGE_SHIFT))
-		pgprot_val(forbidden) |= _PAGE_RW;
+	if (kernel_set_to_readonly) {
+		tmp = (__pa_symbol(__start_rodata) >> PAGE_SHIFT) > pfn ?
+			(__pa_symbol(__start_rodata) >> PAGE_SHIFT) - pfn :
+			ULONG_MAX;
+		if (within(pfn, __pa_symbol(__start_rodata) >> PAGE_SHIFT,
+		    __pa_symbol(__end_rodata) >> PAGE_SHIFT)) {
+			pgprot_val(forbidden) |= _PAGE_RW;
+			tmp = (__pa_symbol(__end_rodata) >> PAGE_SHIFT) - pfn;
+		}
+		num = num > tmp ? tmp : num;
+	}
 
 #if defined(CONFIG_X86_64)
 	/*
@@ -333,11 +355,15 @@ static inline pgprot_t static_protections(pgprot_t prot, unsigned long address,
 	 * This will preserve the large page mappings for kernel text/data
 	 * at no extra cost.
 	 */
+	tmp = kernel_set_to_readonly && (unsigned long)_text > address ?
+		((unsigned long)_text - address) >> PAGE_SHIFT : ULONG_MAX;
 	if (kernel_set_to_readonly &&
 	    within(address, (unsigned long)_text,
 		   (unsigned long)__end_rodata_hpage_align)) {
 		unsigned int level;
 
+		tmp = ((unsigned long)__end_rodata_hpage_align
+				- address) >> PAGE_SHIFT;
 		/*
 		 * Don't enforce the !RW mapping for the kernel text mapping,
 		 * if the current mapping is already using small page mapping.
@@ -355,13 +381,28 @@ static inline pgprot_t static_protections(pgprot_t prot, unsigned long address,
 		 * text mapping logic will help Linux Xen parvirt guest boot
 		 * as well.
 		 */
-		if (lookup_address(address, &level) && (level != PG_LEVEL_4K))
+		if (lookup_address(address, &level) && (level != PG_LEVEL_4K)) {
+			unsigned long psize = page_level_size(level);
+			unsigned long pmask = page_level_mask(level);
+			unsigned long nextpage_addr =
+				(address + psize) & pmask;
+			unsigned long numpages =
+				(nextpage_addr - address) >> PAGE_SHIFT;
+
 			pgprot_val(forbidden) |= _PAGE_RW;
+			tmp = tmp > numpages ? numpages : tmp;
+		}
 	}
+	num = kernel_set_to_readonly && num > tmp ? tmp : num;
 #endif
 
 	prot = __pgprot(pgprot_val(prot) & ~pgprot_val(forbidden));
 
+	if (num == 0)
+		num = 1;
+	if (page_num)
+		*page_num = num;
+
 	return prot;
 }
 
@@ -552,7 +593,8 @@ static int
 try_preserve_large_page(pte_t *kpte, unsigned long address,
 			struct cpa_data *cpa)
 {
-	unsigned long nextpage_addr, numpages, pmask, psize, addr, pfn, old_pfn;
+	unsigned long nextpage_addr, numpages, pmask, psize, pnum,
+		      addr, pfn, old_pfn;
 	pte_t new_pte, old_pte, *tmp;
 	pgprot_t old_prot, new_prot, req_prot;
 	int i, do_split = 1;
@@ -625,7 +667,7 @@ try_preserve_large_page(pte_t *kpte, unsigned long address,
 	pfn = old_pfn + ((address & (psize - 1)) >> PAGE_SHIFT);
 	cpa->pfn = pfn;
 
-	new_prot = static_protections(req_prot, address, pfn);
+	new_prot = static_protections(req_prot, address, pfn, NULL);
 
 	/*
 	 * We need to check the full range, whether
@@ -634,8 +676,10 @@ try_preserve_large_page(pte_t *kpte, unsigned long address,
 	 */
 	addr = address & pmask;
 	pfn = old_pfn;
-	for (i = 0; i < (psize >> PAGE_SHIFT); i++, addr += PAGE_SIZE, pfn++) {
-		pgprot_t chk_prot = static_protections(req_prot, addr, pfn);
+	for (i = 0; i < (psize >> PAGE_SHIFT);
+	    i += pnum, addr += PAGE_SIZE * pnum, pfn += pnum) {
+		pgprot_t chk_prot =
+			static_protections(req_prot, addr, pfn, &pnum);
 
 		if (pgprot_val(chk_prot) != pgprot_val(new_prot))
 			goto out_unlock;
@@ -1246,7 +1290,7 @@ static int __change_page_attr(struct cpa_data *cpa, int primary)
 		pgprot_val(new_prot) &= ~pgprot_val(cpa->mask_clr);
 		pgprot_val(new_prot) |= pgprot_val(cpa->mask_set);
 
-		new_prot = static_protections(new_prot, address, pfn);
+		new_prot = static_protections(new_prot, address, pfn, NULL);
 
 		new_prot = pgprot_clear_protnone_bits(new_prot);
 
-- 
2.7.4


             reply	other threads:[~2018-07-05  5:47 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-05  5:47 Bin Yang [this message]
2018-07-05  6:28 ` [PATCH v2] x86/mm: fix cpu stuck issue in __change_page_attr_set_clr Thomas Gleixner
2018-07-05 22:20 ` Dave Hansen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1530769636-26603-1-git-send-email-bin.yang@intel.com \
    --to=bin.yang@intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox