On Wed, Jun 13, 2012 at 03:29:36PM -0400, Rik van Riel wrote:COLOUR_ALIGN_DOWN can use the pgoff % shm_align_mask either positively
or negatively, depending on the address initially found by
get_unmapped_area
static inline unsigned long COLOUR_ALIGN_DOWN(unsigned long addr,
unsigned long pgoff)
{
unsigned long base = addr& ~shm_align_mask;
unsigned long off = (pgoff<< PAGE_SHIFT)& shm_align_mask;
if (base + off<= addr)
return base + off;
return base - off;
}
Yes, that is bollocks code, introduced by this commit:
commit 7dbaa466780a754154531b44c2086f6618cee3a8
Author: Rob Herring<rob.herring@xxxxxxxxxxx>
Date: Tue Nov 22 04:01:07 2011 +0100
The fix would be to return an address that is a whole shm_align_mask
lower: (((base - shm_align_mask)& ~shm_align_mask) + off
Yes, agreed.
This brings up the question: should a MAP_PRIVATE mapping see updates
to the backing file made via a shared mapping and/or writing the file
directly? After all, a r/w MAP_PRIVATE mapping which has been CoW'd
won't see the updates.
So I'd argue that a file mapped MAP_SHARED must be mapped according to
the colour rules, but a MAP_PRIVATE is free not to be so.
Secondly, MAP_FIXED never checks for page colouring alignment.
I assume the cache aliasing on AMD Bulldozer is merely a performance
issue, and we can simply ignore page colouring for MAP_FIXED?
That will be easy to get right in an architecture-independent
implementation.
There's a whole bunch of issues with MAP_FIXED, specifically address
space overflow has been discussed previously, and resulted in this patch:
[PATCH 0/6] get rid of extra check for TASK_SIZE in get_unmapped_area