On Tue, Aug 28, 2018 at 11:00:33AM +0300, Amir Goldstein wrote:
> The implementation of readahead(2) syscall is identical to that of
> fadvise64(POSIX_FADV_WILLNEED) with a few exceptions:
> 1. readahead(2) returns -EINVAL for !mapping->a_ops and fadvise64()
> ignores the request and returns 0.
> 2. fadvise64() checks for integer overflow corner case
> 3. fadvise64() calls the optional filesystem fadvice() file operation
fadvise
>
> Unite the two implementations by calling vfs_fadvice() from readahead(2)
vfs_fadvise
> syscall. Check the !mapping->a_ops in readahead(2) syscall to preserve
> legacy behavior.
It's not "legacy behaviour" - it's "documented syscall ABI behaviour"
>
> Suggested-by: Miklos Szeredi <mszeredi@xxxxxxxxxx>
> Fixes: d1d04ef8572b ("ovl: stack file ops")
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
>
> Miklos,
>
> Following v4 addresses comment from dchinner.
More comments below...
> @@ -600,16 +583,22 @@ ssize_t ksys_readahead(int fd, loff_t offset, size_t count)
>
> ret = -EBADF;
> f = fdget(fd);
> - if (f.file) {
> - if (f.file->f_mode & FMODE_READ) {
> - struct address_space *mapping = f.file->f_mapping;
> - pgoff_t start = offset >> PAGE_SHIFT;
> - pgoff_t end = (offset + count - 1) >> PAGE_SHIFT;
> - unsigned long len = end - start + 1;
> - ret = do_readahead(mapping, f.file, start, len);
> - }
> - fdput(f);
> - }
> + if (!f.file || !(f.file->f_mode & FMODE_READ))
> + goto out;
> +
> + /*
> + * fadvise() silently ignores an advice for a file with !a_ops and
> + * returns -EPIPE for a pipe. Keep this check here to comply with legacy
> + * -EINVAL behavior of readahead(2).
> + */
> + ret = -EINVAL;
> + if (!f.file->f_mapping || !f.file->f_mapping->a_ops ||
> + !S_ISREG(file_inode(f.file)->i_mode))
> + goto out;
This is not the place to document vfs_advise() behaviour - this is
where you document the readahead syscall error requirements.
"Legacy -EINVAL behaviour" is not a useful description outside of
this specific patch context.
/*
* The readahead() syscall is intended to run only on files
* that can execute readahead. If readahead is not possible
* on this file, then we must return -EINVAL.
*/
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
