Re: [PATCH 00/17] VFS: Filesystem information and notifications [ver #17]

Ian Kent
2020-03-04T02:01:46Z

On Tue, 2020-03-03 at 14:03 +0100, Greg Kroah-Hartman wrote:
> On Tue, Mar 03, 2020 at 12:38:14PM +0100, Karel Zak wrote:
> > On Tue, Mar 03, 2020 at 10:26:21AM +0100, Miklos Szeredi wrote:
> > > No, I don't think this is going to be a performance issue at all,
> > > but
> > > if anything we could introduce a syscall
> > >
> > > ssize_t readfile(int dfd, const char *path, char *buf, size_t
> > > bufsize, int flags);
> >
> > off-topic, but I'll buy you many many beers if you implement it ;-
> > ),
> > because open + read + close is pretty common for /sys and /proc in
> > many userspace tools; for example ps, top, lsblk, lsmem, lsns,
> > udevd
> > etc. is all about it.
>
> Unlimited beers for a 21-line kernel patch? Sign me up!
>
> Totally untested, barely compiled patch below.
>
> Actually, I like this idea (the syscall, not just the unlimited
> beers).
> Maybe this could make a lot of sense, I'll write some actual tests
> for
> it now that syscalls are getting "heavy" again due to CPU vendors
> finally paying the price for their madness...

The problem isn't with open->read->close but with the mount info.
changing between reads (ie. seq file read takes and drops the
needed lock between reads at least once).

The problem is you don't know the buffer size needed to get this
in one hit, how is this different to read(2)?

>
> thanks,
>
> greg k-h
> -------------------
>
>
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl
> b/arch/x86/entry/syscalls/syscall_64.tbl
> index 44d510bc9b78..178cd45340e2 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -359,6 +359,7 @@
> 435 common clone3 __x64_sys_clone3/ptregs
> 437 common openat2 __x64_sys_openat2
> 438 common pidfd_getfd __x64_sys_pidfd_getfd
> +439 common readfile __x86_sys_readfile
>
> #
> # x32-specific system call numbers start at 512 to avoid cache
> impact
> diff --git a/fs/open.c b/fs/open.c
> index 0788b3715731..1a830fada750 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -1340,3 +1340,23 @@ int stream_open(struct inode *inode, struct
> file *filp)
> }
>
> EXPORT_SYMBOL(stream_open);
> +
> +SYSCALL_DEFINE5(readfile, int, dfd, const char __user *, filename,
> + char __user *, buffer, size_t, bufsize, int, flags)
> +{
> + int retval;
> + int fd;
> +
> + if (force_o_largefile())
> + flags |= O_LARGEFILE;
> +
> + fd = do_sys_open(dfd, filename, flags, O_RDONLY);
> + if (fd <= 0)
> + return fd;
> +
> + retval = ksys_read(fd, buffer, bufsize);
> +
> + __close_fd(current->files, fd);
> +
> + return retval;
> +}