parisc: keep track of last shared mmap'ed address

PATCH Helge Deller 2 participants 2 messages Oct 27, 2016 – Oct 30, 2016 PA-RISC architecture

lore

Share & tools

lore

Helge Deller

author maintainer

2016-10-27 18:39 UTC #0 lore raw

shared mappings (v2)
Reply-To:
In-Reply-To: <[email protected]>

* Helge Deller <[email protected]>:

Quoted 20 lines

[RFC,PATCH] mm,parisc: keep track of last mmap'ed address
Because of parisc's cache aliasing constraints we need to map shared pages at a multiple of 4MB while most other architectures can map files at any multiple of PAGE_SIZE. In the past this constraint was ensured by calculating a virtual offset into this 4MB region which is based on the physical address of the kernel mapping variable (right-shift value of filp->f_mapping by 8 bits). Since we only have a 32bit userspace (even when running on a 64bit kernel) this often leads to large gaps in the maps of the userspace processes and to out of memory situations even if physical memory was still free. Of course I did played with other variants of shifting the f_mapping value to find better offsets but this didn't helped either.
This patch chooses a different approach.
It adds the additional field i_mmap_lastmmap to the address_space struct to keep track of the last mapping of a shared file. With this bookkeeping it's possible for the parisc memory allocator to
a) choose a new mapping offset if the file hasn't been mapped yet, and
b) take the last-used mapping if it was already mapped by another process.

Quoted 5 lines

Overall this approach leads to a more condensed memory usage on parisc because the shared files will now be mapped much closer to each other. This is e.g. visible with shared libraries which are now not any longer cluttered around in the userspace process but close to each other at the top of the userspace memory.

This is version 2 of this patch.

Instead of adding one additional field to the address space struct, it now uses the private_data field of the file struct to store the last used mmap address.

Additionally I tweaked the flush_dcache_page() function to warn for INEQUIVALENT ALIASES only for shared mappings and ignore private mappings. Without this change, stracing applications sometimes showed those warnings. Not sure if this is the right thing to do.

My testing didn't showed any negative effects yet, although I'm not sure if the usage of the private_data field may conflict with other use cases, e.g. compressed file systems or such.

To benefit from the more effective/condensed memory usage one additional patch is needed.

Signed-off-by Helge Deller <[email protected]>

arch/parisc/kernel/cache.c +5 -5

index 629eb46..f72789b 100644
@@ -321,10 +321,10 @@ void flush_dcache_page(struct page *page)
 
 	pgoff = page->index;
 
-	/* We have carefully arranged in arch_get_unmapped_area() that
-	 * *any* mappings of a file are always congruently mapped (whether
-	 * declared as MAP_PRIVATE or MAP_SHARED), so we only need
-	 * to flush one address here for them all to become coherent */
+	/* We have carefully arranged in arch_get_unmapped_area() that mappings
+	 * of a file are always congruently mapped (if declared as MAP_SHARED, not
+	 * MAP_PRIVATE), so we only need to flush one address here for them all to
+	 * become coherent */
 
 	flush_dcache_mmap_lock(mapping);
 	vma_interval_tree_foreach(mpnt, &mapping->i_mmap, pgoff, pgoff) {

@@ -344,7 +344,7 @@ void flush_dcache_page(struct page *page)
 		if (old_addr == 0 || (old_addr & (SHM_COLOUR - 1))
 				      != (addr & (SHM_COLOUR - 1))) {
 			__flush_cache_page(mpnt, addr, page_to_phys(page));
-			if (old_addr)
+			if (old_addr && (mpnt->vm_flags & VM_SHARED))
 				printk(KERN_ERR "INEQUIVALENT ALIASES 0x%lx and 0x%lx in file %pD\n", old_addr, addr, mpnt->vm_file);
 			old_addr = addr;
 		}

arch/parisc/kernel/sys_parisc.c +3 -4

index 0a393a0..c04d946 100644
@@ -36,12 +36,11 @@
 #include <linux/personality.h>
 #include <linux/random.h>
 
-/* we construct an artificial offset for the mapping based on the physical
- * address of the kernel mapping variable */
+/* keep track of the last mapping address offset in the private_data field */
 #define GET_LAST_MMAP(filp)		\
-	(filp ? ((unsigned long) filp->f_mapping) >> 8 : 0UL)
+	(filp && (file_count(filp) > 0) ? (unsigned long) filp->private_data : 0UL)
 #define SET_LAST_MMAP(filp, val)	\
-	 { /* nothing */ }
+	{ if (filp) filp->private_data = (void *) (val); }
 
 static int get_offset(unsigned int last_mmap)
 {

John David Anglin

2016-10-30 22:16 UTC #1 lore raw

Quoted 7 lines · Helge Deller

arch/parisc/kernel/sys_parisc.c +0 -0

index 0a393a0..c04d946 100644
@@ -36,12 +36,11 @@
 #include <linux/personality.h>
 #include <linux/random.h>

arch/parisc/kernel/sys_parisc.c line 43

Quoted 10 lines

arch/parisc/kernel/sys_parisc.c +3 -4

 
-/* we construct an artificial offset for the mapping based on the physical
- * address of the kernel mapping variable */
+/* keep track of the last mapping address offset in the private_data field */
 #define GET_LAST_MMAP(filp)		\
-	(filp ? ((unsigned long) filp->f_mapping) >> 8 : 0UL)
+	(filp && (file_count(filp) > 0) ? (unsigned long) filp->private_data : 0UL)
 #define SET_LAST_MMAP(filp, val)	\
-	 { /* nothing */ }
+	{ if (filp) filp->private_data = (void *) (val); }

In my testing, this causes hpmc's doing serial io.

Dave
--
John David Anglin [email protected]