Re: parisc: fix mmap(MAP_FIXED|MAP_SHARED) to already mmapped address

[Helge Deller <deller@gmx.de>]

On 22.02.2015 18:58, James Bottomley wrote:
> On Sun, 2015-02-22 at 12:54 -0500, John David Anglin wrote:
>> On 2015-02-22, at 12:17 PM, James Bottomley wrote:
>>
>>> On Sun, 2015-02-22 at 11:45 -0500, John David Anglin wrote:
>>>> On 2015-02-21, at 6:57 PM, James Bottomley wrote:
>>>>
>>>>> On Sun, 2015-02-22 at 00:26 +0100, Helge Deller wrote:
>>>>>> On 22.02.2015 00:09, James Bottomley wrote:
>>>>>>> On Sat, 2015-02-21 at 15:40 -0500, John David Anglin wrote:
>>>>>>>> On 2015-02-21, at 3:31 PM, John David Anglin wrote:
>>>>>>>>
>>>>>>>>> On 2015-02-20, at 4:36 PM, Carlos O'Donell wrote:
>>>>>>>>>
>>>>>>>>>> On Thu, Apr 3, 2014 at 4:26 PM, Helge Deller <deller@gmx.de> wrote:
>>>>>>>>>>> In current eglibc it's set to 0x00400000
>>>>>>>>>>> That's what my eglibc-patch changes...
>>>>>>>>>>> I'm currently building a eglibc on hpviz with SHMLBA set to 4096 (__getpagesize()).
>>>>>>>>>>
>>>>>>>>>> Anyone object to me fixing this upstream by making SHMLBA match the kernel?
>>>>>>>>>>
>>>>>>>>>> I plan to use a fixed value of 4096, since I never expect hppa
>>>>>>>>>> userspace to have to care (even if the kernel uses superpages).
>>>>>>>>>
>>>>>>>>> We currently use (__getpagesize ()) in Debian and this seems to be a common definition.
>>>>>>>>> Is there a performance advantage in using 4096?
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Please correct me if I'm wrong.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> At one time, we thought this value needed to be 4 MB.  Helge was
>>>>>>>> working on improving the mmap
>>>>>>>>> allocation scheme but this work stalled after some improvement.  I
>>>>>>>> can't remember the issues and how
>>>>>>>>> they relate to SHMLBA.
>>>>>>>>
>>>>>>>>
>>>>>>>> Actually, the number was 4 Mb (bit).
>>>>>>>
>>>>>>> No, it was 4MB.  That's the cache equivalency stride on PA processors
>>>>>>> because we have a VIPT cache.  The architectural requirement according
>>>>>>> to the dreaded appendix F is 16MB but we were assured by the PA
>>>>>>> architects that it was 4 because they never planned producing processors
>>>>>>> that would require 16.  The actual meaning is it's the number of bits of
>>>>>>> the virtual address that are significant in the virtual index.
>>>>>>>
>>>>>>
>>>>>> Your following statement:
>>>>>>
>>>>>>> The point of SHMLBA is that if the same physical page is mapped into two
>>>>>>> different virtual addresses but the two addresses are equal, modulo
>>>>>>> SHMLBA, then the L1 cache sees the equivalency and you can't get
>>>>>>> inequivalent cache aliases for the page (two writes to the two different
>>>>>>> addresses producing two separately dirty cache lines which can never
>>>>>>> resolve).  This means that the virtual addresses of all shared mappings
>>>>>>> have to be equal modulo SHMLBA for the caches not to alias.
>>>>>>
>>>>>> With this you define SHMLBA to be the representative number which defines
>>>>>> what the current cache equivalency stride of the kernel is, *and* which then can
>>>>>> be used by userspace. I think this is a misinterpretation of SHMLBA (or at
>>>>>> least a parisc-specific interpretation of SHMLBA), which is not like how it
>>>>>> is used on other architectures with similar limitations.
>>>>>> Userspace should not know the kernel/architecture specifics. Instead they
>>>>>> should try to mmap() memory somewhere (e.g. 4KB aligned) and if they need shared mappings then
>>>>>> kernel/glibc will return a corrected mapping address (modulo 4MB).
>>>>>> I think this is important, since most userspace programs usually try to mmap at
>>>>>> a multiple of SHMLBA with which we then run very soon out of userspace (with SHMLBA=4MB).
>>>>>> This has been the issue with localedef in glibc (a strange coding which tries
>>>>>> to be platform-specific with mmap-calculation). Because of that in the end
>>>>>> it turned out to be best for parisc to have SHMLBA defined to 4kb (and not 4MB).
>>>>>>
>>>>>> So, your statement above is correct, I would just not use "SHMLBA" in this term,
>>>>>> but maybe "KERNEL_SHMLBA" instead.
>>>>>
>>>>> Um, no, SHMLBA comes from the SYS-V IPC primitives.  They were stupid
>>>>> enough to allow the user pick the address of the region of shared
>>>>> memory, so the user had to know these architectural details and SHMLBA
>>>>> encodes them (man shmat will give you the gory details).
>>>>>
>>>>> For mmap, we can mostly do the right thing in the kernel, except for
>>>>> MAP_FIXED, where the user has to know what they're doing again.
>>>>>
>>>>> For the cases the user thinks they know best, we can't avoid giving out
>>>>> the knowledge somehow, because inequivalent aliases in writeable
>>>>> mappings will HPMC a system.  We could be more relaxed about
>>>>> inequivalent aliases in read only mappings (say shared libraries), but
>>>>> the consequence of that is an explosion in the use of cache space, so we
>>>>> would want some libraries (like glibc) with many shared copies to obey
>>>>> SHMLBA.
>>>>
>>>>
>>>> I agree with Helge.  We run out of memory too quickly with 4 MB.  This resulted in various
>>>> userspace applications failing as mentioned by Helge.
>>>>
>>>> MAP_FIXED can fail it the address is a problem.  I believe we check for that.
>>>>
>>>> If you look at the pch implementation in gcc, you will see that the MAP_FIXED problem can be
>>>> worked around, and this problem is not specific to parisc.  The pch data has to mapped at the same
>>>> address as it was originally created as there is no way to relocate the data.
>>>>
>>>> SHMLBA is 4096 /* (1 << PGSHIFT) */ on hpux.
>>>>
>>>> The following is in <asm-generic/shmparam.h>:
>>>> #define SHMLBA PAGE_SIZE	 /* attach addr a multiple of this */
>>>>
>>>> Shared mappings are handled with
>>>> asm/shmparam.h:#define SHM_COLOUR 0x00400000	/* shared mappings colouring */
>>>
>>> So how is the sys-v ipc problem fixed?  There the user is told to select
>>> an address which is a multiple of SHMLBA.  Programs that do this today
>>> will start to break on writeable mappings if we set SHMLBA to PAGE_SIZE
>>> because the colour will be wrong.
>>
>>
>> The code returns -EINVAL.  See arch_get_unmapped_area.
>
> But that's not a solution.  Let me try to illustrate: I have an existing
> application, it uses sys-v ipc and selects a shmat address based on the
> multiple of SHMLBA for a writeable mapping.  Today it works.

It will work as well with SHMBLA=4096, if you just use SHM_RND too
(and most applications do have SHM_RND).
man shmat says:
  * If shmaddr isn't NULL and SHM_RND is specified in shmflg, the attach occurs at the address equal to shmaddr rounded down to the nearest multiple of SHMLBA.
* Otherwise, shmaddr must be a page-aligned address at which the attach occurs.

So, even here shmaddr is mentioned to be page-aligned (4k), not SHMLBA-aligned (4M in your case).

>Tomorrow
> when you make this change, it fails with -EINVAL.  That's breaking an
> existing application because chances are the app will just report the
> failure and exit.

Tomorrow?  This change is *already* implemented in eglibc since a year or
so and I don't see any applications which break because of SHMBLA=4096...

Helge