lkml.org 
[lkml]   [2023]   [May]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 11/13] tools/nolibc: sys_select: riscv: use __NR_pselect6_time64 for rv32
    Date
    > On 2023-05-25 15:10:21+0800, Zhangjin Wu wrote:
    > > Hi, Thomas
    > >
    > > > On 2023-05-25 01:59:55+0800, Zhangjin Wu wrote:
    > > > > rv32 uses the generic include/uapi/asm-generic/unistd.h and it has no
    > > > > __NR_pselect6 after kernel commit d4c08b9776b3 ("riscv: Use latest
    > > > > system call ABI"), use __NR_pselect6_time64 instead.
    > > > >
    > > > > Signed-off-by: Zhangjin Wu <falcon@tinylab.org>
    > > > > ---
    > > > > tools/include/nolibc/sys.h | 7 ++++++-
    > > > > 1 file changed, 6 insertions(+), 1 deletion(-)
    > > > >
    > > > > diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
    > > > > index c0335a84f880..00c7197dcd50 100644
    > > > > --- a/tools/include/nolibc/sys.h
    > > > > +++ b/tools/include/nolibc/sys.h
    > > > > @@ -1041,8 +1041,13 @@ int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeva
    > > > > struct timeval *t;
    > > > > } arg = { .n = nfds, .r = rfds, .w = wfds, .e = efds, .t = timeout };
    > > > > return my_syscall1(__NR_select, &arg);
    > > > > -#elif defined(__ARCH_WANT_SYS_PSELECT6) && defined(__NR_pselect6)
    > > > > +#elif defined(__ARCH_WANT_SYS_PSELECT6) && (defined(__NR_pselect6) || defined(__NR_pselect6_time64))
    > > > > +#ifdef __NR_pselect6
    > > > > struct timespec t;
    > > > > +#else
    > > > > + struct timespec64 t;
    > > > > +#define __NR_pselect6 __NR_pselect6_time64
    > > >
    > > > Wouldn't this #define leak to the users of nolibc and lead to calls to
    > > > pselect6_time64 with the ABI of the __NR_pselect6 if userspace is doing
    > > > its own raw syscalls?
    > > >
    > >
    > > Yeah, it would break the user-side raw __NR_pselect6 syscall for nolibc is a
    > > header-only libc, so, it is not safe to use such method like glibc.
    > >
    > > Something like this will let the syscall call to pselect6_time64 instead of the
    > > user-required __NR_pselect6 and pass the wrong type of argument.
    > >
    > > #include "nolibc.h" // If no __NR_pselect6 defined, __NR_pselect6 = __NR_pselect6_time64
    > >
    > > #ifdef __NR_pselect6
    > > struct timespec t; // come here for __NR_pselect6_time64, but t is not timespec64, broken
    > > syscall(__NR_pselect6, nfds, rfds, wfds, efds, timeout ? &t : NULL, NULL);
    > > #else
    > > struct timespec64 t;
    > > syscall(__NR_pselect6, nfds, rfds, wfds, efds, timeout ? &t : NULL, NULL);
    > > #endif
    > >
    > > I have used something like __NR_pselect6_time3264 locally, before
    > > sending the patchset, I found a cleaner method already used in sys.h:
    > >
    > > #ifndef __NR__newselect
    > > #define __NR__newselect __NR_select
    > > #endif
    > >
    > > But I forgot the arguments mixing issue, __NR__newselect and __NR_select
    > > share the same type of arguments, but __NR_pselect6 and
    > > __NR_pselect6_time64 not, so, I will use back the old method but still
    > > need to find a better string, just like __NR__newselect, __NR__pselect6
    > > may be used in kernel space in the future, and __NR_pselect6_time3264 is
    > > too long, what about this?
    > >
    > > #ifdef __NR_pselect6
    > > struct timespec t;
    > > #define __NR_pselect6__ __NR_pselect6
    > > #else
    > > struct timespec64 t;
    > > #define __NR_pselect6__ __NR_pselect6_time64
    > > #endif
    > >
    > > Or even ___NR_pselect6?
    >
    > What about:
    >
    > #ifdef __NR_pselect6
    > struct timespec t;
    > const long nr_pselect = __NR_pselect6;
    > #else
    > struct timespec64 t;
    > const long nr_pselect = __NR_pselect6_time64;
    > #endif
    >

    It looks better and cleaner, will apply this method, thanks!

    > >
    > > The same issue is in this patch:
    > >
    > > [PATCH 13/13] tools/nolibc: sys_gettimeofday: riscv: use __NR_clock_gettime64
    > >
    > > will solve it with the same method.
    >

    And also this one:

    [PATCH 09/13] tools/nolibc: sys_poll: riscv: use __NR_ppoll_time64

    Have tested all of them, will send a v2 later.

    Best regards,
    Zhangjin

    > Thanks!

    \
     
     \ /
      Last update: 2023-05-26 03:51    [W:4.067 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site