Re: [PATCH 16/17] RISC-V: User-facing API

[James Hogan <james.hogan@imgtec.com>]

[-- Attachment #1: Type: text/plain, Size: 1812 bytes --]

Hi Christoph,

On Tue, Jul 11, 2017 at 06:39:48AM -0700, Christoph Hellwig wrote:
> > +#ifdef CONFIG_64BIT
> > +SYSCALL_DEFINE6(mmap, unsigned long, addr, unsigned long, len,
> > +	unsigned long, prot, unsigned long, flags,
> > +	unsigned long, fd, off_t, offset)
> > +{
> > +	if (unlikely(offset & (~PAGE_MASK)))
> > +		return -EINVAL;
> > +	return sys_mmap_pgoff(addr, len, prot, flags, fd, offset >> PAGE_SHIFT);
> > +}
> > +#else
> > +SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len,
> > +	unsigned long, prot, unsigned long, flags,
> > +	unsigned long, fd, off_t, offset)
> > +{
> > +	/*
> > +	 * Note that the shift for mmap2 is constant (12),
> > +	 * regardless of PAGE_SIZE
> > +	 */
> > +	if (unlikely(offset & (~PAGE_MASK >> 12)))
> > +		return -EINVAL;
> > +	return sys_mmap_pgoff(addr, len, prot, flags, fd,
> > +		offset >> (PAGE_SHIFT - 12));
> > +}
> > +#endif /* !CONFIG_64BIT */
> 
> Most modern ports seem to expose sys_mmap_pgoff as the
> syscall directly.  Any reason you're doing this differently?

I think Palmer's patch is probably correct here. Exposing sys_mmap_pgoff
is only really correct on 32-bit arches where the only page size is 4k.
If other page sizes are supported then this is the correct way to handle
it as the page offset from 32-bit userland is supposed to be in 4k
units.

64-bit doesn't need to worry about squeezing big file offsets into the
off_t offset so don't need to do the shift at all.

See the mmap2 man page. It says "the final argument specifies the offset
into the file in 4096-byte units", and it points out ia64 as an
exception where it depends on the page size of the system.

> 
> But even the code for the older ones should probably be consolidated..

Quite probably, yes.

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]