From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brian Gerst Date: Wed, 23 Sep 2020 17:08:49 +0000 Subject: Re: [PATCH 5/9] fs: remove various compat readv/writev helpers Message-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit List-Id: References: <20200923060547.16903-1-hch@lst.de> <20200923060547.16903-6-hch@lst.de> <20200923142549.GK3421308@ZenIV.linux.org.uk> <20200923143251.GA14062@lst.de> <20200923145901.GN3421308@ZenIV.linux.org.uk> <20200923163831.GO3421308@ZenIV.linux.org.uk> In-Reply-To: <20200923163831.GO3421308@ZenIV.linux.org.uk> To: Al Viro Cc: Christoph Hellwig , Andrew Morton , Jens Axboe , Arnd Bergmann , David Howells , David Laight , Linux ARM , Linux Kernel Mailing List , "open list:BROADCOM NVRAM DRIVER" , Parisc List , linuxppc-dev , linux-s390 , sparclinux , linux-block@vger.kernel.org, linux-scsi@vger.kernel.org, Linux FS-devel Mailing List , linux-aio@kvack.org, io-uring@vger.kernel.org, linux-arch , Linux-MM , netdev@vger.kernel.org, keyrings@vger.kernel.org, linux-security-module@vger.kernel.org On Wed, Sep 23, 2020 at 12:39 PM Al Viro wrote: > > On Wed, Sep 23, 2020 at 03:59:01PM +0100, Al Viro wrote: > > > > That's a very good question. But it does not just compile but actually > > > works. Probably because all the syscall wrappers mean that we don't > > > actually generate the normal names. I just tried this: > > > > > > --- a/include/linux/syscalls.h > > > +++ b/include/linux/syscalls.h > > > @@ -468,7 +468,7 @@ asmlinkage long sys_lseek(unsigned int fd, off_t offset, > > > asmlinkage long sys_read(unsigned int fd, char __user *buf, size_t count); > > > asmlinkage long sys_write(unsigned int fd, const char __user *buf, > > > size_t count); > > > -asmlinkage long sys_readv(unsigned long fd, > > > +asmlinkage long sys_readv(void *fd, > > > > > > for fun, and the compiler doesn't care either.. > > > > Try to build it for sparc or ppc... > > FWIW, declarations in syscalls.h used to serve 4 purposes: > 1) syscall table initializers needed symbols declared > 2) direct calls needed the same > 3) catching mismatches between the declarations and definitions > 4) centralized list of all syscalls > > (2) has been (thankfully) reduced for some time; in any case, ksys_... is > used for the remaining ones. > > (1) and (3) are served by syscalls.h in architectures other than x86, arm64 > and s390. On those 3 (1) is done otherwise (near the syscall table initializer) > and (3) is not done at all. > > I wonder if we should do something like > > SYSCALL_DECLARE3(readv, unsigned long, fd, const struct iovec __user *, vec, > unsigned long, vlen); > in syscalls.h instead, and not under that ifdef. > > Let it expand to declaration of sys_...() in generic case and, on x86, into > __do_sys_...() and __ia32_sys_...()/__x64_sys_...(), with types matching > what SYSCALL_DEFINE ends up using. > > Similar macro would cover compat_sys_...() declarations. That would > restore mismatch checking for x86 and friends. AFAICS, the cost wouldn't > be terribly high - cpp would have more to chew through in syscalls.h, > but it shouldn't be all that costly. Famous last words, of course... > > Does anybody see fundamental problems with that? I think this would be a good idea. I have been working on a patchset to clean up the conditional syscall handling (sys_ni.c), and conflicts with the prototypes in syscalls.h have been getting in the way. Having the prototypes use SYSCALL_DECLAREx(...) would solve that issue. -- Brian Gerst