On 7/2/20 7:47 PM, Adhemerval Zanella via Libc-alpha wrote: > > > On 30/06/2020 21:08, Vineet Gupta via Libc-alpha wrote: >> --- >> Changes since v7: >> - used long int (iso int) in sysdep.h/syscall_error for handling >> registers > > Patch looks ok, but I have question on the __NR_* undef/define. >
>> diff --git a/sysdeps/unix/sysv/linux/arc/arch-syscall.h >> b/sysdeps/unix/sysv/linux/arc/arch-syscall.h ... >> +#define __NR_write 64 >> +#define __NR_writev 66 > > Looks good. As side note I think an improvement would be to add > an annotation on update-syscall-lists.py to specify which kernel > version it used. Very good idea indeed. I've asked myself the very question atleast once. >> diff --git a/sysdeps/unix/sysv/linux/arc/clone.S >> b/sysdeps/unix/sysv/linux/arc/clone.S .. >> + >> + ; adjust libc args for syscall > > Use the C comment style for constency with rest of the file. Done. >> diff --git a/sysdeps/unix/sysv/linux/arc/fixup-asm-unistd.h >> b/sysdeps/unix/sysv/linux/arc/fixup-asm-unistd.h >> + >> +/* Adjustments to ARC asm-generic syscall ABI (3.9 kernel) for 64-bit time_t >> + support. */ >> + >> +/* fstat64 and fstatat64 need to be replaced with statx. */ >> + >> +#undef __NR_fstat64 >> +#undef __NR_fstatat64 This is certainly needed as they are present in ARC arch-syscall.h but we need to use statx. >> +/* Replace all other 32-bit time syscalls with 64-bit variants. */ >> + >> +# undef __NR_clock_adjtime >> +# undef __NR_clock_getres >> +# undef __NR_futex >> +# undef __NR_mq_timedreceive >> +# undef __NR_mq_timedsend >> +# undef __NR_ppoll >> +# undef __NR_pselect6 >> +# undef __NR_recvmmsg >> +# undef __NR_rt_sigtimedwait >> +# undef __NR_sched_rr_get_interval >> +# undef __NR_semtimedop >> +# undef __NR_timerfd_settime >> +# undef __NR_timerfd_gettime >> +# undef __NR_utimensat > > I am trying to understand why these are required since arc does not define > them in arch-syscall.h. arch-syscall.h doesn't define them precisely due to these being here. When update-syscalls is run, the 32-bit syscalls are generated for ARC (since kernel ABI provides these because that was v3.9 circa 2013). Adding them fixup-asm-unistd.h removes them (perhaps I need to add this in changelog to clarify - atleast for myself). > And the generic implementation should handle the time64 variant. If they > are not this is something we need to handle it. At the time we we doing this, arch-syscall.h generation was not yet in place, however I tried to undef in generic/sysdep.h for TIMESIZE==64. However I was asked me to add this to ARC specific fixup-asm-unistd.h https://sourceware.org/pipermail/libc-alpha/2020-March/112395.html https://sourceware.org/pipermail/libc-alpha/2020-April/112909.html >> diff --git a/sysdeps/unix/sysv/linux/arc/kernel_stat.h >> b/sysdeps/unix/sysv/linux/arc/kernel_stat.h ... >> +#define STATFS_IS_STATFS64 0 > > Ok. This specific one is actually dead code. I did post a patch to this effect and followed up with supporting data that enabling it on 64-bit arches doesn't lead to any changes in generated code. https://sourceware.org/pipermail/libc-alpha/2020-February/111259.html https://sourceware.org/pipermail/libc-alpha/2020-June/115217.html >> diff --git a/sysdeps/unix/sysv/linux/arc/sysdep.h >> b/sysdeps/unix/sysv/linux/arc/sysdep.h ... >> +/* 32-bit time syscalls are not available, but the redefines allow generic >> + wrappers to work. */ >> +#define __NR_clock_adjtime __NR_clock_adjtime64 >> +#define __NR_clock_getres __NR_clock_getres_time64 >> +#define __NR_futex __NR_futex_time64 >> +#define __NR_mq_timedreceive __NR_mq_timedreceive_time64 >> +#define __NR_mq_timedsend __NR_mq_timedsend_time64 >> +#define __NR_ppoll __NR_ppoll_time64 >> +#define __NR_pselect6 __NR_pselect6_time64 >> +#define __NR_recvmmsg __NR_recvmmsg_time64 >> +#define __NR_rt_sigtimedwait __NR_rt_sigtimedwait_time64 >> +#define __NR_sched_rr_get_interval __NR_sched_rr_get_interval_time64 >> +#define __NR_semtimedop __NR_semtimedop_time64 >> +#define __NR_timerfd_gettime __NR_timerfd_gettime64 >> +#define __NR_timerfd_settime __NR_timerfd_settime64 >> +#define __NR_utimensat __NR_utimensat_time64 > > As for the fixup-asm-unistd.h, the generic implementation should handle it > without the requirement of the ABI to add such tricks. fixup-asm-unistd.h is different, but this could be avoided. I know for sure that ll code literally expects __NR_futex (atleast used to). But I can remove this and see what comes out. > > However it seems that we are still missing support for pselect > (__NR_pselect6_time64), recvmmsg (__NR_recvmmsg_time64), sigtimedwait > (__NR_rt_sigtimedwait_time64), and semtimeop (__NR_semtimedop_time64). > > I think we can add the redefine hack only the aforementioned symbols for > now and removed them once we implement the y2038 support on such symbols > (since the expected ABI won't change for ARC, only for old ABIs with > 32 time_t support). Sorry /me horribly confused here. >> + >> +#undef SYS_ify >> +#define SYS_ify(syscall_name) __NR_##syscall_name > > The code mixes __NR_* and SYS_ify macro. This macro is really superflous > and I am preparing some patches to cleanup this up along with C asm > macros to generate syscall. So I would suggest to just use the __NR_* > way and drop this definition. I don't mind, but it seems that the wrapper was a simply way to avoid open-coding the macro concatenation. e.g. # define DO_CALL(syscall_name, args) \ - mov r8, SYS_ify (syscall_name) ASM_LINE_SEP \ + mov r8, __NR__##syscall_name ASM_LINE_SEP \ ARC_TRAP_INSN _______________________________________________ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
