On Fri, Oct 21, 2016 at 03:06:45PM +0200, Arnd Bergmann wrote:

Regarding the review process, I'd really hope we've improved enough
that we can rely on the review on linux-arch/linux-api to catch
all serious issues like system call that cannot be defined the same
way on all architectures. If we fail at this, there is a more
serious issue with the review process.

Well, forget linux-arch, that's hopeless because that became a very
x86-centric linux-kernel-v2, and as such I refuse to subscribe to it -
it would be a total waste of my network bandwidth because I wouldn't
have time to read it.

I somehow suspect that linux-api isn't that much better either.  What
I want from any "arch" specific thing is a heads-up to alert me to
something so that I can then choose whether to look deeper at the
subject or just ignore it completely.  I don't want to be buried under
lots of x86 specific drivel about a new feature.

So, the reality is, that I don't see any of the new syscall discussions
anymore.  The first that I'm aware of them is when they hit in some way
that becomes visible to me - which is normally sometime during the merge
window.

Since all syscalls now go through SYSCALL_DEFINEx(), we have
covered the hardest part (sign/zero extending short arguments),
and a lot more people are aware of problems with struct alignment
since it differs between i386 and x86_64 and also affects all
ioctl interfaces. I think the last time a syscall made it in that
didn't just work on all architectures was sync_file_range, and
that was nine years ago.

It's not really about struct alignment, although that is a concern.
For ARM, it's more about argument alignment, and whether a 64-bit
argument gets passed in (eg) r1/r2 or r2/r3, and whether we run out
of registers to pass the arguments.

If we hit this case, why not just use the wrapper on both EABI
and OABI for simplicity? It's not like we care a lot about
micro-optimizing OABI any more.

I'd still like to retain the ability to only add to EABI in the future.

quoted

You'll find the latest version in the next linux-next, or my current
for-next branch.

Ok. After rebasing my randconfig tree on today's linux-next, I needed
this hunk to avoid a warning:

<stdin>:1143:2: error: #warning syscall sync_file_range not implemented [-Werror=cpp]

I don't get that on my builds, for EABI or OABI - for EABI:

  CHK     include/generated/bounds.h
  CC      arch/arm/kernel/asm-offsets.s
  CHK     include/generated/asm-offsets.h
  CALL    /home/rmk/git/linux-rmk/scripts/checksyscalls.sh
make[1]: Leaving directory '/home/rmk/git/build/hdrtst'

and for OABI:

  CHK     include/generated/bounds.h
  CC      arch/arm/kernel/asm-offsets.s
  CHK     include/generated/asm-offsets.h
  CALL    /home/rmk/git/linux-rmk/scripts/checksyscalls.sh
make[1]: Leaving directory '/home/rmk/git/build/hdrtst-oabi'

So, I'd like to know how you're seeing that warning.  We have never
provided sync_file_range on ARM, and we must never define it, because
the user API for it is broken.

Well, I think you have a mis-merge somewhere, beacuse uapi/asm/unistd.h
does have:

#define __NR_sync_file_range2           __NR_arm_sync_file_range

in it, which triggers this in scripts/checksyscalls.sh:

/* sync_file_range had a stupid ABI. Allow sync_file_range2 instead */
#ifdef __NR_sync_file_range2
#define __IGNORE_sync_file_range
#endif

Hence why I don't see the warning you're seeing.

That brings up an interesting issue: it would be nice to use the
same input file for arch/arm/ and the compat mode of arch/arm64,
like x86 does. If we handle both oabi and arm64-compat in the same
file, we end up with a superset of what x86 does, and we could
use a single script again, and generate all four tables for
ARM (native OABI, OABI-on-EABI, native EABI, EABI-on-arm64).

OABI-compat != ARM64-EABI-compat though.  They're two completely
different things.

Moreover, the syscall numbers ARM64 uses natively are completely
different from the syscall numbers 32-bit ARM uses, either for EABI
or OABI.  So I really don't see this working.

I've no idea how ARM64 deals with 32-bit binaries, so I can't comment
on how it deals with those syscalls, sorry.

Another related case in asm-generic, which defines three tables:
native 32-bit, native 64-bit and compat 32-bit. This one not only
needs to have three different function pointers (e.g. sys_fcntl64,
sys_fcntl and compat_sys_fcntl64) but also different macros (e.g.
__NR_fcntl64 and __NR_fcntl).

Anything wrong with this approach:?

/* ARM */
221  oabi  fcntl64                 sys_fcntl64             sys_oabi_fcntl64
221  eabi  fcntl64                 sys_fcntl64             compat_sys_fcntl64

/* asm-generic */
25   32    fcntl64                 sys_fcntl64             compat_sys_fcntl64
25   64    fcntl                   sys_fcntl

Don't know, sorry.  I know virtually nothing about the differences
between the 64-bit and 32-bit ABIs, so I can't comment on anything
to do with the compat_* interfaces.

quoted

The syscallnr.sh script kind-of looks like a candidate, but it has
ARM arch specifics to it (knowing that the number of system calls
needs to fit within the 8-bit value plus 4-bit shift constant
representation of ARM ALU instructions.)  Maybe a generic version
without that knowledge would work, provided architectures can
override it.

syscallnr.sh isn't used on x86, and probably won't be needed on
most (or all) others, right? 

Why not - the point of syscallnr.sh is to remove the need to manually
update the __NR_syscalls definition, which is a generic kernel thing.
Unless other architectures just define a fixed-size table with plenty
of extra space, they'd need to adjust __NR_syscalls when adding new
calls.


-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.