[LTP] [PATCH V2 15/17] syscalls/semtimedop: Add support for semtimedop and its time64 version

[Viresh Kumar]

On 08-05-20, 09:18, Arnd Bergmann wrote:
> On Fri, May 8, 2020 at 6:24 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> 
> > +
> > +static inline int sys_semtimedop(int semid, struct sembuf *sops, size_t nsops,
> > +               void *timeout)
> > +{
> > +       return tst_syscall(__NR_semtimedop, semid, sops, nsops, timeout);
> > +}
> > +
> > +static inline int sys_semtimedop_time64(int semid, struct sembuf *sops,
> > +                                       size_t nsops, void *timeout)
> > +{
> > +       return tst_syscall(__NR_semtimedop_time64, semid, sops, nsops, timeout);
> > +}
> > +
> > +struct test_variants {
> > +       int (*semop)(int semid, struct sembuf *sops, size_t nsops);
> > +       int (*semtimedop)(int semid, struct sembuf *sops, size_t nsops, void *timeout);
> > +       enum tst_ts_type type;
> > +       char *desc;
> > +} variants[] = {
> > +       { .semop = semop, .type = TST_LIBC_TIMESPEC, .desc = "semop: vDSO or syscall"},
> > +#if defined(TST_ABI32)
> > +       { .semtimedop = sys_semtimedop, .type = TST_LIBC_TIMESPEC, .desc = "semtimedop: syscall with libc spec"},
> > +       { .semtimedop = sys_semtimedop, .type = TST_KERN_OLD_TIMESPEC, .desc = "semtimedop: syscall with kernel spec32"},
> > +#endif
> > +
> > +#if defined(TST_ABI64)
> > +       { .semtimedop = sys_semtimedop, .type = TST_KERN_TIMESPEC, .desc = "semtimedop: syscall with kernel spec64"},
> > +#endif
> 
> 
> It feels like this is more complicated than it need to be. The line
> 
> semtimedop = sys_semtimedop, .type = TST_KERN_OLD_TIMESPEC, .desc =
> "semtimedop: syscall with kernel spec32"},
> 
> should apply to any kernel that has "__NR_semtimedop !=
> __LTP__NR_INVALID_SYSCALL",
> regardless of any other macros set, and then you don't need the separate line
> 
> { .semtimedop = sys_semtimedop, .type = TST_KERN_TIMESPEC, .desc =
> "semtimedop: syscall with kernel spec64"},
 
> which is not what the ABI is meant to be anyway (sys_semtimedop takes
> a __kernel_old_timespec,
> not a __kernel_timespec).

There is some misunderstanding here, surely from my side. The sys_
helpers here are the direct syscalls called from userspace with help
of tst_syscall().

AFAIU, on 32 bit systems we need to call __NR_semtimedop with the 32
bit and 64 bit timespec (both), and on 64 bit systems which don't
implement __NR_semtimedop_time64, we need to call __NR_semtimedop with
the 64 bit timespec only.

What you are telling now is very different from that and so I don't
get it.

> Similarly, the line
> 
>  { .semop = semop, .type = TST_LIBC_TIMESPEC, .desc = "semop: vDSO or syscall"},
> 
> should apply to both 32 and 64 bit machines

Yes and so it is called without ifdef hackery. Isn't that correct ? 

-- 
viresh