Hello,
kernel test robot noticed "ltp.readahead01.fail" on:
commit: cb12fd8e0dabb9a1c8aef55a6a41e2c255fcdf4b ("pidfd: add pidfs")
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
[test failed on linus/master 65d287c7eb1d14e0f4d56f19cec30d97fc7e8f66]
[test failed on linux-next/master a1184cae56bcb96b86df3ee0377cec507a3f56e0]
in testcase: ltp
version: ltp-x86_64-14c1f76-1_20240309
with following parameters:
disk: 1HDD
fs: f2fs
test: syscalls-00/readahead01
compiler: gcc-12
test machine: 4 threads 1 sockets Intel(R) Core(TM) i3-3220 CPU @ 3.30GHz (Ivy Bridge) with 8G memory
(please refer to attached dmesg/kmsg for entire log/backtrace)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]
Running tests.......
<<<test_start>>>
tag=readahead01 stime=1710262698
cmdline="readahead01"
contacts=""
analysis=exit
<<<test_output>>>
tst_test.c:1741: TINFO: LTP version: 20240129-91-gcbc2d0568
tst_test.c:1625: TINFO: Timeout per run is 0h 02m 30s
readahead01.c:36: TPASS: readahead() with fd = -1 : EBADF (9)
readahead01.c:43: TPASS: readahead() with invalid fd : EBADF (9)
readahead01.c:63: TPASS: readahead() on O_PATH file : EBADF (9)
readahead01.c:63: TPASS: readahead() on directory : EINVAL (22)
readahead01.c:63: TPASS: readahead() on /dev/zero : EINVAL (22)
readahead01.c:63: TPASS: readahead() on pipe read end : EINVAL (22)
readahead01.c:63: TPASS: readahead() on pipe write end : EBADF (9)
readahead01.c:63: TPASS: readahead() on unix socket : EINVAL (22)
readahead01.c:63: TPASS: readahead() on inet socket : EINVAL (22)
readahead01.c:63: TPASS: readahead() on epoll : EINVAL (22)
readahead01.c:63: TPASS: readahead() on eventfd : EINVAL (22)
readahead01.c:63: TPASS: readahead() on signalfd : EINVAL (22)
readahead01.c:63: TPASS: readahead() on timerfd : EINVAL (22)
readahead01.c:63: TFAIL: readahead() on pidfd succeeded
readahead01.c:63: TPASS: readahead() on fanotify : EINVAL (22)
readahead01.c:63: TPASS: readahead() on inotify : EINVAL (22)
readahead01.c:63: TPASS: readahead() on userfaultfd : EINVAL (22)
readahead01.c:63: TPASS: readahead() on perf event : EINVAL (22)
readahead01.c:63: TPASS: readahead() on io uring : EINVAL (22)
readahead01.c:63: TPASS: readahead() on bpf map : EINVAL (22)
readahead01.c:63: TPASS: readahead() on fsopen : EINVAL (22)
readahead01.c:63: TPASS: readahead() on fspick : EINVAL (22)
readahead01.c:63: TPASS: readahead() on open_tree : EBADF (9)
Summary:
passed 22
failed 1
broken 0
skipped 0
warnings 0
incrementing stop
<<<execution_status>>>
initiation_status="ok"
duration=0 termination_type=exited termination_id=1 corefile=no
cutime=0 cstime=3
<<<test_end>>>
INFO: ltp-pan reported some tests FAIL
LTP Version: 20240129-91-gcbc2d0568
###############################################################
Done executing testcases.
LTP Version: 20240129-91-gcbc2d0568
###############################################################
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20240315/[email protected]
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Fri, Mar 15, 2024 at 04:16:33PM +0800, kernel test robot wrote:
>
>
> Hello,
>
> kernel test robot noticed "ltp.readahead01.fail" on:
>
> commit: cb12fd8e0dabb9a1c8aef55a6a41e2c255fcdf4b ("pidfd: add pidfs")
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
>
> [test failed on linus/master 65d287c7eb1d14e0f4d56f19cec30d97fc7e8f66]
> [test failed on linux-next/master a1184cae56bcb96b86df3ee0377cec507a3f56e0]
>
> in testcase: ltp
> version: ltp-x86_64-14c1f76-1_20240309
> with following parameters:
>
> disk: 1HDD
> fs: f2fs
> test: syscalls-00/readahead01
>
>
>
> compiler: gcc-12
> test machine: 4 threads 1 sockets Intel(R) Core(TM) i3-3220 CPU @ 3.30GHz (Ivy Bridge) with 8G memory
Yes, this is an expected failure.
Before moving pidfds to pidfs they were based on anonymous inodes.
Anonymous inodes have a strange property: yhey have no file type. IOW,
(stat.st_mode & S_IFMT) == 0.
The readhead code looks at the filetype and if it isn't a regular file
then you'll get EINVAL. This is the case for anonymous inode based
pidfds:
/*
* 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.
*/
ret = -EINVAL;
if (!f.file->f_mapping || !f.file->f_mapping->a_ops ||
(!S_ISREG(file_inode(f.file)->i_mode) &&
!S_ISBLK(file_inode(f.file)->i_mode)))
goto out;
However, pidfs makes them regular files so they're not caught by that
check anymore.
However, pidfs doesn't implement any readahead support. Specifically,
it'll have sb->s_bdi == noop_backing_dev_info. Which will mean the
readahead request is just ignored:
if (IS_DAX(inode) || (bdi == &noop_backing_dev_info)) {
switch (advice) {
case POSIX_FADV_NORMAL:
case POSIX_FADV_RANDOM:
case POSIX_FADV_SEQUENTIAL:
case POSIX_FADV_WILLNEED:
case POSIX_FADV_NOREUSE:
case POSIX_FADV_DONTNEED:
/* no bad return value, but ignore advice */
break;
default:
return -EINVAL;
}
return 0;
}
So I'd just remove that test. It's meaningless for pseudo fses.
Hi!
> So I'd just remove that test. It's meaningless for pseudo fses.
Wouldn't it make more sense to actually return EINVAL instead of
ignoring the request if readahead() is not implemented?
--
Cyril Hrubis
[email protected]
On Fri, Mar 15, 2024 at 03:49:03PM +0100, Cyril Hrubis wrote:
> Hi!
> > So I'd just remove that test. It's meaningless for pseudo fses.
>
> Wouldn't it make more sense to actually return EINVAL instead of
> ignoring the request if readahead() is not implemented?
It would change the return value for a whole bunch of stuff. I'm not
sure that wouldn't cause regressions but is in any case a question for
the readahead maintainers. For now I'd just remove that test for pidfds
imho.
Hi,
> On Fri, Mar 15, 2024 at 03:49:03PM +0100, Cyril Hrubis wrote:
> > Hi!
> > > So I'd just remove that test. It's meaningless for pseudo fses.
> > Wouldn't it make more sense to actually return EINVAL instead of
> > ignoring the request if readahead() is not implemented?
> It would change the return value for a whole bunch of stuff. I'm not
> sure that wouldn't cause regressions but is in any case a question for
> the readahead maintainers. For now I'd just remove that test for pidfds
> imho.
@Matthew, any input on Cyril's question please?
Kind regards,
Petr