linux-kernel - [PATCH v3 1/5] x86/mm: avoid redundant checking if pgprot has no change

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: <1534814186-37067-2-git-send-email-bin.yang@intel.com>
Date:   Tue, 21 Aug 2018 01:16:22 +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 1/5] x86/mm: avoid redundant checking if pgprot has no change

In try_preserve_large_page(), the check for
pgprot_val(new_prot) == pgprot_val(old_port) can definitely
be done at first to avoid redundant checking.

The approach and some of the comments came from Thomas Gleixner's
email example for how to do this

Suggested-by: Thomas Gleixner <tglx@...utronix.de>
Signed-off-by: Bin Yang <bin.yang@...el.com>
---
 arch/x86/mm/pageattr.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 8d6c34f..68613fd 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -629,6 +629,22 @@ try_preserve_large_page(pte_t *kpte, unsigned long address,
 	new_prot = static_protections(req_prot, address, pfn);
 
 	/*
+	 * The static_protections() is used to check specific protection flags
+	 * for certain areas of memory. The old pgprot should be checked already
+	 * when it was applied before. If it's not, then this is a bug in some
+	 * other code and needs to be fixed there.
+	 *
+	 * If new pgprot is same as old pgprot, return directly without any
+	 * additional checking. The following static_protections() checking is
+	 * pointless if pgprot has no change. It can avoid the redundant
+	 * checking and optimize the performance of large page split checking.
+	 */
+	if (pgprot_val(new_prot) == pgprot_val(old_prot)) {
+		do_split = 0;
+		goto out_unlock;
+	}
+
+	/*
 	 * We need to check the full range, whether
 	 * static_protection() requires a different pgprot for one of
 	 * the pages in the range we try to preserve:
@@ -642,14 +658,6 @@ try_preserve_large_page(pte_t *kpte, unsigned long address,
 			goto out_unlock;
 	}
 
-	/*
-	 * If there are no changes, return. maxpages has been updated
-	 * above:
-	 */
-	if (pgprot_val(new_prot) == pgprot_val(old_prot)) {
-		do_split = 0;
-		goto out_unlock;
-	}
 
 	/*
 	 * We need to change the attributes. Check, whether we can
-- 
2.7.4