| Message ID | 1430839821-9259-1-git-send-email-ldufour@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 7978f76c4495c194183e03f65406f78cdda37882 |
| Delegated to: | Michael Ellerman |
| Headers | show |
On Tue, 2015-05-05 at 15:30:21 UTC, Laurent Dufour wrote: > The commit 8170a83f15ee ("powerpc: Wireup the kcmp syscall to sys_ni") has > disabled the kcmp syscall for powerpc. This has been done due to the use > of unsigned long parameters which may require a dedicated wrapper to handle > 32bit process on top of 64bit kernel. However in the kcmp() case, the 2 > unsigned long parameters are currently only used to carry file descriptors > from user space to the kernel. Since such a parameter is passed through > register, and file descriptor doesn't need to get extended, there is, > today, no need for a wrapper. > > In the case there will be a need to pass address in or out of this system > call, then a wrapper could be required, it will then be to care of it. > > As today this is not the case, it is safe to enable kcmp() on powerpc. That's mostly convincing. Though I see that s390 does have a compat wrapper, i386 doesn't. So who knows what that means. But, there's a selftest in tools/testing/selftests/kcmp. So can you run that as 32-bit and confirm it works? Then I'd be 100% convinced :) cheers
On 11/05/2015 12:31, Michael Ellerman wrote: > On Tue, 2015-05-05 at 15:30:21 UTC, Laurent Dufour wrote: >> The commit 8170a83f15ee ("powerpc: Wireup the kcmp syscall to sys_ni") has >> disabled the kcmp syscall for powerpc. This has been done due to the use >> of unsigned long parameters which may require a dedicated wrapper to handle >> 32bit process on top of 64bit kernel. However in the kcmp() case, the 2 >> unsigned long parameters are currently only used to carry file descriptors >> from user space to the kernel. Since such a parameter is passed through >> register, and file descriptor doesn't need to get extended, there is, >> today, no need for a wrapper. >> >> In the case there will be a need to pass address in or out of this system >> call, then a wrapper could be required, it will then be to care of it. >> >> As today this is not the case, it is safe to enable kcmp() on powerpc. > > That's mostly convincing. Though I see that s390 does have a compat wrapper, > i386 doesn't. So who knows what that means. > > But, there's a selftest in tools/testing/selftests/kcmp. So can you run that as > 32-bit and confirm it works? Then I'd be 100% convinced :) Hi Michael, I was myself a bit confused about this system call, so to be sure before sending the patch I ran the selftest/kcmp series on 64bit, 32bit, and also build it in 32bit and run it on top of a 64bit kernel without any trouble... Cheers, Laurent.
diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h index 91062eef582f..b257f02ab6bf 100644 --- a/arch/powerpc/include/asm/systbl.h +++ b/arch/powerpc/include/asm/systbl.h @@ -358,7 +358,7 @@ SYSCALL_SPU(setns) COMPAT_SYS(process_vm_readv) COMPAT_SYS(process_vm_writev) SYSCALL(finit_module) -SYSCALL(ni_syscall) /* sys_kcmp */ +SYSCALL(kcmp) /* sys_kcmp */ SYSCALL_SPU(sched_setattr) SYSCALL_SPU(sched_getattr) SYSCALL_SPU(renameat2)
The commit 8170a83f15ee ("powerpc: Wireup the kcmp syscall to sys_ni") has disabled the kcmp syscall for powerpc. This has been done due to the use of unsigned long parameters which may require a dedicated wrapper to handle 32bit process on top of 64bit kernel. However in the kcmp() case, the 2 unsigned long parameters are currently only used to carry file descriptors from user space to the kernel. Since such a parameter is passed through register, and file descriptor doesn't need to get extended, there is, today, no need for a wrapper. In the case there will be a need to pass address in or out of this system call, then a wrapper could be required, it will then be to care of it. As today this is not the case, it is safe to enable kcmp() on powerpc. Cc: Tony Breeds <tony@bakeyournoodle.com> Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com> --- arch/powerpc/include/asm/systbl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)