Messages in this thread Patch in this message | | | Date | Tue, 11 Jul 2017 10:28:14 -0700 (PDT) | Subject | Re: [patches] Re: [PATCH 16/17] RISC-V: User-facing API | From | Palmer Dabbelt <> |
| |
On Tue, 11 Jul 2017 07:01:32 PDT (-0700), james.hogan@imgtec.com wrote: > 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.
This looks like what arm64 does, though I'm OK either way. Here's my attempt at consolidating the code, even though there isn't a lot to help with:
diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c index e18fc0ebdd91..4351be7d0533 100644 --- a/arch/riscv/kernel/sys_riscv.c +++ b/arch/riscv/kernel/sys_riscv.c @@ -17,14 +17,23 @@ #include <asm/cmpxchg.h> #include <asm/unistd.h>
+static long riscv_sys_mmap(unsigned long addr, unsigned long len, + unsigned long prot, unsigned long flags, + unsigned long fd, off_t offset, + unsigned long page_shift_offset) +{ + if (unlikely(offset & (~PAGE_MASK >> page_shift_offset))) + return -EINVAL; + return sys_mmap_pgoff(addr, len, prot, flags, fd, + offset >> (PAGE_SHIFT - page_shift_offset)); +} + #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); + return riscv_sys_mmap(addr, len, prot, flags, fd, offset, 0); } #else SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len, @@ -35,9 +44,6 @@ SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len, * 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)); + return riscv_sys_mmap(addr, len, prot, flags, fd, offset, 12); } #endif /* !CONFIG_64BIT */ I'll submit this as part of our v6, which will hopefully be coming out soon.
Thanks!
| |