Re: [PATCH v9 2/3] cachestat: implement cachestat syscall

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Sun, Feb 5, 2023 at 4:56 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
On Fri, Feb 3, 2023, at 20:04, Nhat Pham wrote:

> +SYSCALL_DEFINE5(cachestat, unsigned int, fd, loff_t, off, size_t, len,
> +             struct cachestat __user *, cstat, unsigned int, flags)
> +{
> +     return ksys_cachestat(fd, off, len, cstat, flags);
> +}
> +
> +#ifdef CONFIG_COMPAT
> +COMPAT_SYSCALL_DEFINE6(cachestat, unsigned int, fd,
> compat_arg_u64_dual(off),
> +             size_t, len, struct cachestat __user *, cstat, unsigned int, flags)
> +{
> +     return ksys_cachestat(fd, compat_arg_u64_glue(off), len, cstat,
> flags);
> +}

This still looks wrong to me, as this compat definition does not match
the native variant on architectures that require 64-bit arguments to
be passed in aligned register pairs, such as arm, mips or ppc, but
not x86, s390 or riscv.

Oh I see - thanks for pointing that out! And the last bit means this
is a non-issue for x86, s390 or riscv right? 

And iirc from the last thread, this is fixable via a simple reordering
of the args in order to properly align the 64-bit arguments, for e.g:

SYSCALL_DEFINE5(cachestat, loff_t, off, unsigned int, fd, size_t, len,
              struct cachestat __user *, cstat, unsigned int, flags)

...

COMPAT_SYSCALL_DEFINE6(cachestat, compat_arg_u64_dual(off), 
             unsigned int, fd, size_t, len, struct cachestat __user *, cstat, 
             unsigned int, flags)


It looks a bit odd to me that fd is not the first argument, but perhaps this
is an acceptable sacrifice to avoid unused arg and keep the flags...

Let me know what you think about this!



     Arnd

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux