Consolidate calls to unmap_mapping_range in collapse_shmem

Consolidate calls to unmap_mapping_range in collapse_shmem

[Matthew Wilcox]

Is there a reason we call unmap_mapping_range() for a single page at a
time instead of the entire hugepage?  This is surely more efficient ...
but does it do something like increase the refcount on the page?

I suppose we might be able to skip all the calls to unmap_mapping_range()
if none of the pages are mapped, but surely anonymous pages are usually
mapped?

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 34bce5c308e3..2c686ba6a32b 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1396,17 +1396,11 @@ static void collapse_shmem(struct mm_struct *mm,
 			goto out_isolate_failed;
 		}
 
-		if (page_mapped(page))
-			unmap_mapping_range(mapping, index << PAGE_SHIFT,
-					PAGE_SIZE, 0);
-
 		spin_lock_irq(&mapping->tree_lock);
 
 		slot = radix_tree_lookup_slot(&mapping->page_tree, index);
 		VM_BUG_ON_PAGE(page != radix_tree_deref_slot_protected(slot,
 					&mapping->tree_lock), page);
-		VM_BUG_ON_PAGE(page_mapped(page), page);
-
 		/*
 		 * The page is expected to have page_count() == 3:
 		 *  - we hold a pin on it;
@@ -1472,6 +1466,9 @@ static void collapse_shmem(struct mm_struct *mm,
 		unsigned long flags;
 		struct zone *zone = page_zone(new_page);
 
+		unmap_mapping_range(mapping, start << PAGE_SHIFT,
+					HPAGE_PMD_SIZE, 0);
+
 		/*
 		 * Replacing old pages with new one has succeed, now we need to
 		 * copy the content and free old pages.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: Consolidate calls to unmap_mapping_range in collapse_shmem

[Kirill A. Shutemov]

On Tue, Mar 28, 2017 at 07:15:03PM -0700, Matthew Wilcox wrote:
> 
> Is there a reason we call unmap_mapping_range() for a single page at a
> time instead of the entire hugepage?  This is surely more efficient ...
> but does it do something like increase the refcount on the page?

Yes, mapcount holds refcount on the page. So page_ref_freeze() will fail
with proposed change.

> I suppose we might be able to skip all the calls to unmap_mapping_range()
> if none of the pages are mapped, but surely anonymous pages are usually
> mapped?

The valid optimization I *think* would be to call unmap_mapping_range()
for whole huge page range before iterating over individual pages. This way
we would only hit page_mapped() case if there was race with page fault.

P.S. proper To/Cc would helped me to notice it. :)

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>