Re: [PATCH v2] mmap.2: MAP_FIXED updated documentation

[Michal Hocko]

On Mon 04-12-17 18:14:18, John Hubbard wrote:
> On 12/04/2017 02:55 AM, Cyril Hrubis wrote:
> > Hi!
> > I know that we are not touching the rest of the existing description for
> > MAP_FIXED however the second sentence in the manual page says that "addr
> > must be a multiple of the page size." Which however is misleading as
> > this is not enough on some architectures. Code in the wild seems to
> > (mis)use SHMLBA for aligment purposes but I'm not sure that we should
> > advise something like that in the manpages.
> > 
> > So what about something as:
> > 
> > "addr must be suitably aligned, for most architectures multiple of page
> > size is sufficient, however some may impose additional restrictions for
> > page mapping addresses."
> > 
> 
> Hi Cyril,
> 
> Right, so I've been looking into this today, and I think we can go a bit
> further than that, even. The kernel, as far back as the *original* git
> commit in 2005, implements mmap on ARM by requiring that the address is
> aligned to SHMLBA:
> 
> arch/arm/mm/mmap.c:50:
> 
> 	if (flags & MAP_FIXED) {
> 		if (aliasing && flags & MAP_SHARED &&
> 		    (addr - (pgoff << PAGE_SHIFT)) & (SHMLBA - 1))
> 			return -EINVAL;
> 		return addr;
> 	}
> 
> So, given that this has been the implementation for the last 12+ years (and
> probably the whole time, in fact), I think we can be bold enough to use this
> wording for the second sentence of MAP_FIXED:
> 
> "addr must be a multiple of SHMLBA (<sys/shm.h>), which in turn is either
> the system page size (on many architectures) or a multiple of the system
> page size (on some architectures)."
> 
> What do you think?

I am not sure this is a good idea. This is pulling way too many
implementation details into the man page IMHO. Note that your wording is
even incorrect because this applies only to shared mappings and on some
architectures it even requires special memory regions. We do not want
all that in the man page...

-- 
Michal Hocko
SUSE Labs