Skip to contentSkip to conversation

forget_pte()

PATCH William Lee Irwin III 2 participants 4 messages Jun 1, 2002 – Jun 3, 2002

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas:
# ChangeSet 1.656 -> 1.657
# mm/memory.c 1.70 -> 1.71
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/06/01 [email protected] 1.657
# memory.c:
# Fix stale comment around forget_pte() and change to a #define in order to get accurate line numbers for BUG(), also using BUG_ON().
# --------------------------------------------
#

mm/memory.c +5 -8
diff -Nru a/mm/memory.c b/mm/memory.c
@@ -309,15 +309,12 @@
 }
 
 /*
- * Return indicates whether a page was freed so caller can adjust rss
+ * bug check to be sure pte's are unmapped when no longer used
  */
-static inline void forget_pte(pte_t page)
-{
-	if (!pte_none(page)) {
-		printk("forget_pte: old mapping existed!\n");
-		BUG();
-	}
-}
+#define forget_pte(pte)			\
+	do {				\
+		BUG_ON(!pte_none(pte));	\
+	} while (0)
 
 static void zap_pte_range(mmu_gather_t *tlb, pmd_t * pmd, unsigned long address, unsigned long size)
 {
#1 lore raw

In message <[email protected]> you write:

Quoted 11 lines

diff -Nru a/mm/memory.c b/mm/memory.c

mm/memory.c +1 -2
@@ -309,15 +309,12 @@
 }
 
 /*
- * Return indicates whether a page was freed so caller can adjust rss
+ * bug check to be sure pte's are unmapped when no longer used
  */
-static inline void forget_pte(pte_t page)

mm/memory.c line 317

Quoted 10 lines
mm/memory.c +4 -6
-{
-	if (!pte_none(page)) {
-		printk("forget_pte: old mapping existed!\n");
-		BUG();
-	}
-}
+#define forget_pte(pte)			\
+	do {				\
+		BUG_ON(!pte_none(pte));	\
+	} while (0)

Hmm... it's only used in two places, and the name is entirely misleading. I think it might be neater to replace those two occurances with:

	/* PTEs must be unmapped */
	BUG_ON(!pte_none(pte));

Cheers,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

#2 lore raw
In reply to William Lee Irwin III
Quoted 8 lines · Rusty Russell

Hmm... it's only used in two places, and the name is entirely misleading. I think it might be neater to replace those two occurances with:

	/* PTEs must be unmapped */
	BUG_ON(!pte_none(pte));

Cheers,
Rusty.

I'll send a patch replacing it with that open-coded at the call sites in short order, then.

#3 lore raw
In reply to William Lee Irwin III

On Sat, Jun 01, 2002 at 09:40:02AM -0700, William Lee Irwin III wrote:
...

patch updated to incorporate rusty's suggestion:

===== mm/memory.c 1.70 vs edited =====

mm/memory.c +4 -13
@@ -310,17 +310,6 @@
 	return -ENOMEM;
 }
 
-/*
- * Return indicates whether a page was freed so caller can adjust rss
- */
-static inline void forget_pte(pte_t page)
-{
-	if (!pte_none(page)) {
-		printk("forget_pte: old mapping existed!\n");
-		BUG();
-	}
-}
-
 static void zap_pte_range(mmu_gather_t *tlb, pmd_t * pmd, unsigned long address, unsigned long size)
 {
 	unsigned long offset;
@@ -779,7 +768,8 @@
 		pte_t zero_pte = pte_wrprotect(mk_pte(ZERO_PAGE(address), prot));
 		pte_t oldpage = ptep_get_and_clear(pte);
 		set_pte(pte, zero_pte);
-		forget_pte(oldpage);
+		/* PTE's must be unmapped */
+		BUG_ON(!pte_none(oldpage));
 		address += PAGE_SIZE;
 		pte++;
 	} while (address && (address < end));
@@ -857,7 +847,8 @@
 
 		if (!pfn_valid(pfn) || PageReserved(pfn_to_page(pfn)))
  			set_pte(pte, pfn_pte(pfn, prot));
-		forget_pte(oldpage);
+		/* PTE's must be unmapped */
+		BUG_ON(!pte_none(oldpage));
 		address += PAGE_SIZE;
 		pfn++;
 		pte++;