From mboxrd@z Thu Jan 1 00:00:00 1970 From: Al Viro Subject: Re: [RFC PATCH 4/6] mm: provide generic compat_sys_readahead() implementation Date: Sun, 18 Mar 2018 18:18:48 +0000 Message-ID: <20180318181848.GU30522@ZenIV.linux.org.uk> References: <20180318161056.5377-1-linux@dominikbrodowski.net> <20180318161056.5377-5-linux@dominikbrodowski.net> <20180318174014.GR30522@ZenIV.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Linus Torvalds Cc: Dominik Brodowski , Linux Kernel Mailing List , Arnd Bergmann , linux-arch , Ralf Baechle , James Hogan , linux-mips , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , ppc-dev , Martin Schwidefsky , Heiko Carstens , linux-s390 , "David S . Miller" , sparclinux@vger.kernel.org, Ingo Molnar , Jiri Slaby , the arch/x86 maintainers List-Id: linux-arch.vger.kernel.org On Sun, Mar 18, 2018 at 11:06:42AM -0700, Linus Torvalds wrote: > and then we can do > > COMPAT_SYSCALL_DEFINE5(readahead, int, fd, > COMPAT_ARG_64BIT_ODD(off), compat_size_t, count) > { > return do_readahead(fd, off_lo + ((u64)off_hi << 64), count); > } > > which at least looks reasonably legible, and has *zero* ifdef's anywhere. It's a bit more complicated, but... > I do *not* want to see those disgusting __ARCH_WANT_LE_COMPAT_SYS > things and crazy #ifdef's in code. Absolutely. Those piles of ifdefs are unreadable garbage. > So either let the architectures do their own trivial wrappers > entirely, or do something clean like the above. Do *not* do > #ifdef'fery at the system call declaration time. > > Also note that the "ODD" arguments may not be the ones that need > padding. I could easily see a system call argument numbering scheme > like > > r0 - system call number > r1 - first argument > r2 - second argument > ... > > and then it's the *EVEN* 64-bit arguments that would need the padding > (because they are actually odd in the register numbers). The above > COMPAT_ARG_64BIT[_ODD]() model allows for that too. > > Of course, if some architecture then has some other arbitrary rules (I > could see register pairing rules that aren't the usual "even register" > ones), then such an architecture would really have to have its own > wrapper, but the above at least would handle the simple cases, and > doesn't look disgusting to use. I'd done some digging in that area, will find the notes and post. Basically, we can even avoid the odd/even annotations and have COMPAT_SYSCALL_DEFINE... sort it out. It's a bit more hairy than I would like at this stage in the cycle, though. I'll see if it can be done without too much PITA. However, there still are genuinely speci^Wfucked in head cases - see e.g. this sad story: commit ab8a261ba5e2dd9206da640de5870cc31d568a7c Author: Helge Deller Date: Thu Jul 10 18:07:17 2014 +0200 parisc: fix fanotify_mark() syscall on 32bit compat kernel Those certainly ought to stay in arch/* From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from zeniv.linux.org.uk ([195.92.253.2]:41370 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750766AbeCRSSy (ORCPT ); Sun, 18 Mar 2018 14:18:54 -0400 Date: Sun, 18 Mar 2018 18:18:48 +0000 From: Al Viro Subject: Re: [RFC PATCH 4/6] mm: provide generic compat_sys_readahead() implementation Message-ID: <20180318181848.GU30522@ZenIV.linux.org.uk> References: <20180318161056.5377-1-linux@dominikbrodowski.net> <20180318161056.5377-5-linux@dominikbrodowski.net> <20180318174014.GR30522@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-arch-owner@vger.kernel.org List-ID: To: Linus Torvalds Cc: Dominik Brodowski , Linux Kernel Mailing List , Arnd Bergmann , linux-arch , Ralf Baechle , James Hogan , linux-mips , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman , ppc-dev , Martin Schwidefsky , Heiko Carstens , linux-s390 , "David S . Miller" , sparclinux@vger.kernel.org, Ingo Molnar , Jiri Slaby , the arch/x86 maintainers Message-ID: <20180318181848.KcnadLIyQbsQkuEtzLXDdl1nAjUYGiOPyX7o4MMbSk4@z> On Sun, Mar 18, 2018 at 11:06:42AM -0700, Linus Torvalds wrote: > and then we can do > > COMPAT_SYSCALL_DEFINE5(readahead, int, fd, > COMPAT_ARG_64BIT_ODD(off), compat_size_t, count) > { > return do_readahead(fd, off_lo + ((u64)off_hi << 64), count); > } > > which at least looks reasonably legible, and has *zero* ifdef's anywhere. It's a bit more complicated, but... > I do *not* want to see those disgusting __ARCH_WANT_LE_COMPAT_SYS > things and crazy #ifdef's in code. Absolutely. Those piles of ifdefs are unreadable garbage. > So either let the architectures do their own trivial wrappers > entirely, or do something clean like the above. Do *not* do > #ifdef'fery at the system call declaration time. > > Also note that the "ODD" arguments may not be the ones that need > padding. I could easily see a system call argument numbering scheme > like > > r0 - system call number > r1 - first argument > r2 - second argument > ... > > and then it's the *EVEN* 64-bit arguments that would need the padding > (because they are actually odd in the register numbers). The above > COMPAT_ARG_64BIT[_ODD]() model allows for that too. > > Of course, if some architecture then has some other arbitrary rules (I > could see register pairing rules that aren't the usual "even register" > ones), then such an architecture would really have to have its own > wrapper, but the above at least would handle the simple cases, and > doesn't look disgusting to use. I'd done some digging in that area, will find the notes and post. Basically, we can even avoid the odd/even annotations and have COMPAT_SYSCALL_DEFINE... sort it out. It's a bit more hairy than I would like at this stage in the cycle, though. I'll see if it can be done without too much PITA. However, there still are genuinely speci^Wfucked in head cases - see e.g. this sad story: commit ab8a261ba5e2dd9206da640de5870cc31d568a7c Author: Helge Deller Date: Thu Jul 10 18:07:17 2014 +0200 parisc: fix fanotify_mark() syscall on 32bit compat kernel Those certainly ought to stay in arch/*