[question] pread2, pwrite2 unistd symbols for compat

Commit Message

Yury Norov May 2, 2016, 1:47 p.m. UTC

Hi Andre, 

In patch 987aedb5d (eneric syscalls: wire up preadv2 and pwritev2
syscalls) you add those syscalls with __SYSCALL macro. However,
compat architectures that does not use generic unistd (mips, s390),
declare compat version in their syscall tables. Maybe we'd replace
__SYSCALL macro with __SC_COMP?

Yury.

Comments

Arnd Bergmann May 2, 2016, 3:20 p.m. UTC | #1

On Monday 02 May 2016 16:47:06 Yury Norov wrote:
> Hi Andre, 
> 
> In patch 987aedb5d (eneric syscalls: wire up preadv2 and pwritev2
> syscalls) you add those syscalls with __SYSCALL macro. However,
> compat architectures that does not use generic unistd (mips, s390),
> declare compat version in their syscall tables. Maybe we'd replace
> __SYSCALL macro with __SC_COMP?

I think you are right and your change looks good.

After I got the original patch from Andre and the same one from Christoph,
I did not consider the possibility that they were both wrong and so I
applied Andre's version.

Can you resend the patch in proper form (with a Signed-off-by line,
rebased to mainline and with a changelog I can use)? I'll apply it as
soon as I get an Ack from Christoph then.

Thanks!

	Arnd

> diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> index 723479c..6ed4613 100644
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -721,9 +721,9 @@ __SC_WRAP(__NR_mlock2, sys_mlock2)
>  #define __NR_copy_file_range 285
>  __SC_WRAP(__NR_copy_file_range, sys_copy_file_range)
>  #define __NR_preadv2 286
> -__SYSCALL(__NR_preadv2, sys_preadv2)
> +__SC_COMP(__NR_preadv2, sys_preadv2, compat_sys_preadv2)
>  #define __NR_pwritev2 287
> -__SYSCALL(__NR_pwritev2, sys_pwritev2)
> +__SC_COMP(__NR_pwritev2, sys_pwritev2, compat_sys_pwritev2)
>  
>  #undef __NR_syscalls
>  #define __NR_syscalls 288
> 
> _______________________________________________
> linux-arm-kernel mailing list
> [e-mail]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Christoph Hellwig May 2, 2016, 3:22 p.m. UTC | #2

On Mon, May 02, 2016 at 05:20:22PM +0200, Arnd Bergmann wrote:
> After I got the original patch from Andre and the same one from Christoph,
> I did not consider the possibility that they were both wrong and so I
> applied Andre's version.

It's because adding syscalls is simple a giant pain, full of pitfalls
that are completely non-obvious.  When asked to add syscalls for
architectures I can't personally test I'll have to rely on the test
bot giving me some feedback - and it seems the wrong thing here will
just silently pass without any notice.

The change looks fine to me, but then again I just demonstrated a complete
lack of clue on asm-generic syscalls..

Arnd Bergmann May 2, 2016, 3:40 p.m. UTC | #3

On Monday 02 May 2016 17:22:48 Christoph Hellwig wrote:
> On Mon, May 02, 2016 at 05:20:22PM +0200, Arnd Bergmann wrote:
> > After I got the original patch from Andre and the same one from Christoph,
> > I did not consider the possibility that they were both wrong and so I
> > applied Andre's version.
> 
> It's because adding syscalls is simple a giant pain, full of pitfalls
> that are completely non-obvious.  When asked to add syscalls for
> architectures I can't personally test I'll have to rely on the test
> bot giving me some feedback - and it seems the wrong thing here will
> just silently pass without any notice.
> 
> The change looks fine to me, but then again I just demonstrated a complete
> lack of clue on asm-generic syscalls..

Ok, so I didn't miss anything obvious here and will forward the patch
as soon as Yury sends it.

I guess this is one more reason for me to finally do the job of
generalizing the system call tables to the point where we only
add them in one place of the kernel. There are probably 22 new syscalls
we need for 64-bit time_t, so doing it right then can also some
copy-paste errors.

	Arnd

Patch

diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 723479c..6ed4613 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -721,9 +721,9 @@  __SC_WRAP(__NR_mlock2, sys_mlock2)
 #define __NR_copy_file_range 285
 __SC_WRAP(__NR_copy_file_range, sys_copy_file_range)
 #define __NR_preadv2 286
-__SYSCALL(__NR_preadv2, sys_preadv2)
+__SC_COMP(__NR_preadv2, sys_preadv2, compat_sys_preadv2)
 #define __NR_pwritev2 287
-__SYSCALL(__NR_pwritev2, sys_pwritev2)
+__SC_COMP(__NR_pwritev2, sys_pwritev2, compat_sys_pwritev2)
 
 #undef __NR_syscalls
 #define __NR_syscalls 288