513716 - VMPI_getPrivateResidentPageCount() for Linux assumes pagesize

Status: VERIFIED FIXED

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

Description

[Steven Johnson]

The Linux implementation assumes that the system pagesize matches MMgc pagesize (which is always 4k). This isn't a valid assumption since system pagesize might vary.

Comment 1

[Lars T Hansen]

Closely related to bug #520080.

Comment 2

[Lars T Hansen]

Attachment: General cleanup around getVMPageSize and getPrivateResidentPageCount

Comment 3

[Steven Johnson]

Comment on attachment 404214 [details] [diff] [review]
General cleanup around getVMPageSize and getPrivateResidentPageCount

approved with nits:

-- "static long pagesize" -> "static size_t pagesize"

-- since we have VMPI_snprintf we should be using it for all new code and deprecating VMPI_sprintf

Comment 4

[Lars T Hansen]

Good catches both.  Will fix before pushing.

Comment 5

[Lars T Hansen]

redux changeset:   2670:a004bf5fcae0

Comment 6

[Edwin Smith]

I pushed this fix, to fix the build:
http://hg.mozilla.org/tamarin-redux/rev/a4ca57beee55

as a result, GCHeap::kNativePageSize is now unused in TR.  If it's unused everywhere then we should remove it.  reopening the bug for post-mortem review of the patch.

Comment 7

[Edwin Smith]

Attachment: use VMPI_getVMPageSize() instad of GCHeap::kNativePageSize

already pushed, posting here to trigger review.

Comment 8

[Lars T Hansen]

Attachment: Final cleanup

Clean a little more: 

(1) All client code that I have access to reads kNativePageSize through a pointer to a GCHeap instance, so it does not need to be static.  That is good, because if it is static then there's a question of order of initialization of kNativePageSize and the cached value in VMPI_getVMPageSize.  So make it a non-static propertly.

(2) Nobody uses the abstraction vmPageSize.  So remove it.

Comment 9

[Lars T Hansen]

(In reply to comment #8)
> Created an attachment (id=404633) [details]
> Final cleanup

redux changeset:   2687:7f7ea1a67bf6

Comment 10

[Chris Peyer]

Resolved fixed engineering / work item that has been pushed.  Setting status to verified.