[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 2.5.63] Make objrmap mapcount non-atomic




--On Monday, March 03, 2003 14:15:18 -0800 Andrew Morton <akpm@digeo.com>
wrote:

> Well why not make mapcount an "int" and move the places where it is
> modified inside pte_chain_lock()?
> 
> That does not increase the number of atomic operations, and it makes me
> stop wondering if this:
> 
>                 if (atomic_read(&page->pte.mapcount) == 0)
>                         inc_page_state(nr_mapped);
> 
> is racy ;)

That would be entirely too easy a solution :)

You're entirely right, of course.  Here's the patch that makes it an int
instead of atomic, with the appropriate locking.

Dave

======================================================================
Dave McCracken          IBM Linux Base Kernel Team      1-512-838-3059
dmccr@us.ibm.com                                        T/L   678-3059
--- 2.5.63-objrmap/./include/linux/mm.h	2003-02-27 15:58:34.000000000 -0600
+++ 2.5.63-objfix/./include/linux/mm.h	2003-03-03 16:26:21.000000000 -0600
@@ -172,7 +172,7 @@
 		struct pte_chain *chain;/* Reverse pte mapping pointer.
 					 * protected by PG_chainlock */
 		pte_addr_t direct;
-		atomic_t mapcount;
+		int mapcount;
 	} pte;
 	unsigned long private;		/* mapping-private opaque data */
 
--- 2.5.63-objrmap/./mm/rmap.c	2003-02-28 14:19:10.000000000 -0600
+++ 2.5.63-objfix/./mm/rmap.c	2003-03-03 20:08:43.000000000 -0600
@@ -144,7 +144,7 @@
 	struct vm_area_struct *vma;
 	int referenced = 0;
 
-	if (atomic_read(&page->pte.mapcount) == 0)
+	if (!page->pte.mapcount)
 		return 0;
 
 	if (!mapping)
@@ -243,19 +243,20 @@
 	if (!pfn_valid(page_to_pfn(page)) || PageReserved(page))
 		return pte_chain;
 
+	pte_chain_lock(page);
+
 	if (!PageAnon(page)) {
 		if (!page->mapping)
 			BUG();
 		if (PageSwapCache(page))
 			BUG();
-		if (atomic_read(&page->pte.mapcount) == 0)
+		if (!page->pte.mapcount)
 			inc_page_state(nr_mapped);
-		atomic_inc(&page->pte.mapcount);
+		page->pte.mapcount++;
+		pte_chain_unlock(page);
 		return pte_chain;
 	}
 
-	pte_chain_lock(page);
-
 #ifdef DEBUG_RMAP
 	/*
 	 * This stuff needs help to get up to highmem speed.
@@ -342,20 +343,22 @@
 	if (!page_mapped(page))
 		return;		/* remap_page_range() from a driver? */
 
+	pte_chain_lock(page);
+
 	if (!PageAnon(page)) {
 		if (!page->mapping)
 			BUG();
 		if (PageSwapCache(page))
 			BUG();
-		if (atomic_read(&page->pte.mapcount) == 0)
+		if (!page->pte.mapcount)
 			BUG();
-		if (atomic_dec_and_test(&page->pte.mapcount))
+		page->pte.mapcount--;
+		if (!page->pte.mapcount)
 			dec_page_state(nr_mapped);
+		pte_chain_unlock(page);
 		return;
 	}
 
-	pte_chain_lock(page);
-
 	if (PageDirect(page)) {
 		if (page->pte.direct == pte_paddr) {
 			page->pte.direct = 0;
@@ -471,11 +474,11 @@
 	if (pte_dirty(pteval))
 		set_page_dirty(page);
 
-	if (atomic_read(&page->pte.mapcount) == 0)
+	if (!page->pte.mapcount)
 		BUG();
 
 	mm->rss--;
-	atomic_dec(&page->pte.mapcount);
+	page->pte.mapcount--;
 	page_cache_release(page);
 
 out_unmap:
@@ -516,7 +519,7 @@
 			goto out;
 	}
 
-	if (atomic_read(&page->pte.mapcount) != 0)
+	if (page->pte.mapcount)
 		BUG();
 
 out: