review request for 7142641: -Xshared:on fails on ARM

[David Holmes]

On 29/02/2012 9:43 AM, Dean Long wrote:
> yes, I was concerned about that as well. But MAP_SHARED isn't actually
> needed to share read-only pages. If you trace an app's startup using
> "strace" you can see that the dynamic linker maps shared libraries such
> as libc with MAP_PRIVATE. Multiple processes still share those pages
> through the page cache. Furthermore, the linux mmap implementation turns
> off the shared flag if the file was opened read-only (but unfortunately
> it turns off the flag only after enforcing the cache coloring constraints.)

Man page indicates MAP_SHARED and MAP_PRIVATE only affect the behaviour 
of writable pages:

---
The flags argument determines whether updates to the mapping are visible 
to other processes mapping the same region, and whether updates are 
carried through to the underlying file. This behavior is determined by 
including exactly one of the following values in flags:

MAP_SHARED
     Share this mapping. Updates to the mapping are visible to other 
processes that map this file, and are carried through to the underlying 
file. The file may not actually be updated until msync(2) or munmap() is 
called.
MAP_PRIVATE
     Create a private copy-on-write mapping. Updates to the mapping are 
not visible to other processes mapping the same file, and are not 
carried through to the underlying file. It is unspecified whether 
changes made to the file after the mmap() call are visible in the mapped 
region.
---

So having a read-only page marked MAP_SHARED is pointless. So looks good 
to me.

The same mistake is made in other code in the closed repo, plus in the 
same code in os_bsd.cpp & os_solaris.cpp, plus perfMemory_<os>.cpp

David
------

>
> dl
>
> On 2/28/2012 2:40 PM, Daniel D. Daugherty wrote:
>> Dean,
>>
>> I'm confused... I thought mapping the read-only CDS archive pages as
>> "shared" permits those pages to be shared by other java processes that
>> are also using the CDS archive...
>>
>> Dan
>>
>> On 2/28/12 3:05 PM, Dean Long wrote:
>>> http://cr.openjdk.java.net/~dlong/7142641/webrev.0/
>>> Summary of changes: 3 lines changed: 0 ins; 2 del; 1 mod; 5538 unchg
>>>
>>> The Class Data Sharing file is mapped "shared" on linux, which places
>>> extra constraints on which virtual address a file offset can use.
>>> This is to ensure consistency on certain platforms with aliasing
>>> caches. However as the CDS file is opened read-only, mapping it
>>> "shared" has no benefit and can cause the mmap to fail. The simplest
>>> fix is to change the mapping to "private".
>>>
>>> Tested on linux arm platform with aliasing cache. Installed CDS with
>>> -Xshare:dump then ran through some vm tests as a sanity check.
>>>
>>> This bug is on the 7u4 watch list.
>>>
>>> dl