| Message ID | 1542104370-22831-2-git-send-email-firoz.khan@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | xtensa: system call table generation support | expand |
Hi Firoz, I have one more question: On Tue, Nov 13, 2018 at 2:20 AM Firoz Khan <firoz.khan@linaro.org> wrote: > The 2nd option will be the recommended one. For that, I > added the __NR_syscalls macro in uapi/asm/unistd.h along > with __NR_syscall_count asm/unistd.h. The macro __NR_sys- > calls also added for making the name convention same across > all architecture. While __NR_syscalls isn't strictly part > of the uapi, having it as part of the generated header to > simplifies the implementation. We also need to enclose > this macro with #ifdef __KERNEL__ to avoid side effects. Looking at the include/uapi/asm-generic/unistd.h I see that __NR_syscalls is not guarded by the #ifdef __KERNEL__, why should it be guarded for xtensa? If we remove __NR_syscall_count from the uapi header I'd suggest dropping it completely and switching its two current users to __NR_syscalls. -- Thanks. -- Max
Hi Max, On Wed, 14 Nov 2018 at 05:19, Max Filippov <jcmvbkbc@gmail.com> wrote: > > Hi Firoz, > > I have one more question: > > On Tue, Nov 13, 2018 at 2:20 AM Firoz Khan <firoz.khan@linaro.org> wrote: > > The 2nd option will be the recommended one. For that, I > > added the __NR_syscalls macro in uapi/asm/unistd.h along > > with __NR_syscall_count asm/unistd.h. The macro __NR_sys- > > calls also added for making the name convention same across > > all architecture. While __NR_syscalls isn't strictly part > > of the uapi, having it as part of the generated header to > > simplifies the implementation. We also need to enclose > > this macro with #ifdef __KERNEL__ to avoid side effects. > > Looking at the include/uapi/asm-generic/unistd.h I see that > __NR_syscalls is not guarded by the #ifdef __KERNEL__, > why should it be guarded for xtensa? __NR_syscalls for kernel. So this macro is present in asm/unistd.h rather than uapi/asm/unistd.h. As I mentioned in the patch, it would be better to keep in uapi/asm/unistd.h to count the number of syscalls. But this will create some side effect. So I was guarded with __KERNEL__. In order to come up with common implementation, I kept this for all architecture. > > If we remove __NR_syscall_count from the uapi header I'd > suggest dropping it completely and switching its two current > users to __NR_syscalls. I'm not removing the __NR_syscall_count macro; just place it in asm/unistd.h file for the above reason. FYI, I made sure that the kernel will build with my patch. I would appreciate if you can perform the boot test on the actual platform. Thanks Firoz > > -- > Thanks. > -- Max
On Thu, Nov 15, 2018 at 2:05 AM Firoz Khan <firoz.khan@linaro.org> wrote: > On Wed, 14 Nov 2018 at 05:19, Max Filippov <jcmvbkbc@gmail.com> wrote: > > If we remove __NR_syscall_count from the uapi header I'd > > suggest dropping it completely and switching its two current > > users to __NR_syscalls. > > I'm not removing the __NR_syscall_count macro; just place it > in asm/unistd.h file for the above reason. Ok, I'll apply this series to the xtensa tree and make my cleanups on top of it. > FYI, I made sure that the kernel will build with my patch. I would > appreciate if you can perform the boot test on the actual platform. I did a simple boot testing on a range of configurations, it works. I'll also run LTP a bit later. -- Thanks. -- Max
Hi Max, On Fri, 16 Nov 2018 at 01:15, Max Filippov <jcmvbkbc@gmail.com> wrote: > > On Thu, Nov 15, 2018 at 2:05 AM Firoz Khan <firoz.khan@linaro.org> wrote: > > On Wed, 14 Nov 2018 at 05:19, Max Filippov <jcmvbkbc@gmail.com> wrote: > > > If we remove __NR_syscall_count from the uapi header I'd > > > suggest dropping it completely and switching its two current > > > users to __NR_syscalls. > > > > I'm not removing the __NR_syscall_count macro; just place it > > in asm/unistd.h file for the above reason. > > Ok, I'll apply this series to the xtensa tree and make my cleanups > on top of it. > > > FYI, I made sure that the kernel will build with my patch. I would > > appreciate if you can perform the boot test on the actual platform. > > I did a simple boot testing on a range of configurations, it works. > I'll also run LTP a bit later. Great, thanks! Firoz > > -- > Thanks. > -- Max
diff --git a/arch/xtensa/include/asm/unistd.h b/arch/xtensa/include/asm/unistd.h index 574e552..787987a 100644 --- a/arch/xtensa/include/asm/unistd.h +++ b/arch/xtensa/include/asm/unistd.h @@ -5,6 +5,8 @@ #define __ARCH_WANT_SYS_CLONE #include <uapi/asm/unistd.h> +#define __NR_syscall_count __NR_syscalls + #define __ARCH_WANT_NEW_STAT #define __ARCH_WANT_STAT64 #define __ARCH_WANT_SYS_UTIME diff --git a/arch/xtensa/include/uapi/asm/unistd.h b/arch/xtensa/include/uapi/asm/unistd.h index bc3f62d..332d67a 100644 --- a/arch/xtensa/include/uapi/asm/unistd.h +++ b/arch/xtensa/include/uapi/asm/unistd.h @@ -778,7 +778,9 @@ #define __NR_statx 351 __SYSCALL(351, sys_statx, 5) -#define __NR_syscall_count 352 +#ifdef __KERNEL__ +#define __NR_syscalls 352 +#endif /* * sysxtensa syscall handler
__NR_syscall_count macro holds the number of system call exist in xtensa architecture. We have to change the value of __NR_syscall_count, if we add or delete a system call. One of the patch in this patch series has a script which will generate a uapi header based on syscall.tbl file. The syscall.tbl file contains the total number of system calls information. So we have two option to update __NR- _syscall_count value. 1. Update __NR_syscall_count in asm/unistd.h manually by counting the no.of system calls. No need to update __NR- _syscall_count until we either add a new system call or delete existing system call. 2. We can keep this feature it above mentioned script, that will count the number of syscalls and keep it in a generated file. In this case we don't need to expli- citly update __NR_syscall_count in asm/unistd.h file. The 2nd option will be the recommended one. For that, I added the __NR_syscalls macro in uapi/asm/unistd.h along with __NR_syscall_count asm/unistd.h. The macro __NR_sys- calls also added for making the name convention same across all architecture. While __NR_syscalls isn't strictly part of the uapi, having it as part of the generated header to simplifies the implementation. We also need to enclose this macro with #ifdef __KERNEL__ to avoid side effects. Signed-off-by: Firoz Khan <firoz.khan@linaro.org> --- arch/xtensa/include/asm/unistd.h | 2 ++ arch/xtensa/include/uapi/asm/unistd.h | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) -- 1.9.1