520080 - mmap/munmap alignment when pagesize!=4k

Status: RESOLVED WONTFIX

(Tamarin Graveyard :: Garbage Collection (mmGC), defect)

Description

[Chris Dearman]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.14) Gecko/2009090217 Ubuntu/9.04 (jaunty) Firefox/3.0.14
Build Identifier: 

The mmap/munmap system call with MAP_FIXED requires that the address be page aligned.
On a system with 16K pages I've observed that MMgcPortUnix.cpp:VMPI_decommitMemory attempts to munmap/mmap a 4k aligned page given to it by GCHeap::Decommit. The call fails which leads to an assertion failure in GCHeap::Decommit


Reproducible: Always

Steps to Reproduce:
I have only tested this on a MIPS system with 16K pages. Running any of the 
sunspider performance tests (eg /performance/sunspider/date-format-tofte.abc) cause VMPI_decommitMemory to be called with a 4k aligned page.

Comment 1

[Chris Dearman]

Attachment: Workaround for munmap alignment

This is a workaround for the problem I see. If the munmap fails, the address will still be valid so there is no need mmap it again. This patch also clears the area to zero to mimic the effect of munmap/mmap, but I don't know if anything else relies on this.

A better fix would be to persuade the higher level code to decommit memory in pagesize chunks.

Comment 2

[Lars T Hansen]

Closely related to bug #513716.

Comment 3

[Lars T Hansen]

(In reply to comment #1)

> A better fix would be to persuade the higher level code to decommit memory in
> pagesize chunks.

Agree.  However, the higher level conflates management of committed memory with management of kBlockSize (4K) blocks, and fixing that may take some work.

Comment 4

[Lars T Hansen]

(In reply to comment #3)
> Agree.  However, the higher level conflates management of committed memory with
> management of kBlockSize (4K) blocks, and fixing that may take some work.

Specifically, it looks like we can easily fix this problem if we don't care about internal fragmentation in the memory returned from GCHeap (just make sure to alloc in appropriate multiples of 4K pages, and also split blocks in correct multiples of 4K pages), but on average for a 16K page size system we'll be wasting 8KB per allocation in the worst case - probably far too much in practice, and I suspect the common case is not much better.  The fix for that appears to be decoupling the management of 4K blocks from the underlying VM blocks.

Comment 5

[Lars T Hansen]

See also bug #513906.

Comment 6

[Chris Dearman]

Attachment: Add assertions to check address/size arguments for mmap/munmap

These seem like useful assertions to highlight potential problems

Comment 7

[Chris Dearman]

Attachment: Update: Workaround for munmap alignment

Beefed up the workaround to deal with recent changes
This allows Tamarin to run on systems with 16K pages

Comment 8

[Lars T Hansen]

(In reply to comment #7)
> Created an attachment (id=405742) [details]
> Update: Workaround for munmap alignment
> 
> Beefed up the workaround to deal with recent changes
> This allows Tamarin to run on systems with 16K pages

I don't see how this can be right, unless you make assumptions about the order of requests from the GCHeap layer.  It seems pretty arbitrary whether some memory will be decommitted at all (ie decommit is not reliable), and it seems like there's a hazard that you'll be touching uncommitted memory in the Commit operation.

For the time being the best option for MIPS may be to simply disable virtual memory.  Since that option is available I will demote this bug to a P3 and remove the block it holds on the MIPS code landing.

Comment 9

[Dan Smith]

Marking as Resolved/WONTFIX. If this becomes necessary, let's discuss and reopen.