Re: [PATCH] vfs: fix readahead(2) on block devices

[Amir Goldstein <amir73il@gmail.com>]

On Thu, Sep 21, 2023 at 4:01 PM Reuben Hawkins <reubenhwk@gmail.com> wrote:
>
>
> On Tue, Sep 19, 2023 at 3:43 AM Amir Goldstein <amir73il@gmail.com> wrote:
>>
>> On Tue, Sep 19, 2023 at 5:47 AM kernel test robot <oliver.sang@intel.com> wrote:
>> >
>> >
>> >
>> > Hello,
>> >
>> > kernel test robot noticed "ltp.readahead01.fail" on:
>> >
>> > commit: f49a20c992d7fed16e04c4cfa40e9f28f18f81f7 ("[PATCH] vfs: fix readahead(2) on block devices")
>> > url: https://github.com/intel-lab-lkp/linux/commits/Reuben-Hawkins/vfs-fix-readahead-2-on-block-devices/20230909-124349
>> > base: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git 32bf43e4efdb87e0f7e90ba3883e07b8522322ad
>> > patch link: https://lore.kernel.org/all/20230909043806.3539-1-reubenhwk@gmail.com/
>> > patch subject: [PATCH] vfs: fix readahead(2) on block devices
>> >
>> > in testcase: ltp
>> > version: ltp-x86_64-14c1f76-1_20230715
>> > with following parameters:
>> >
>> >         disk: 1HDD
>> >         fs: ext4
>> >         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 <oliver.sang@intel.com>
>> > | Closes: https://lore.kernel.org/oe-lkp/202309191018.68ec87d7-oliver.sang@intel.com
>> >
>> >
>> >
>> > COMMAND:    /lkp/benchmarks/ltp/bin/ltp-pan   -e -S   -a 3917     -n 3917 -p -f /fs/sdb2/tmpdir/ltp-R8Bqhtsv5t/alltests -l /lkp/benchmarks/ltp/results/LTP_RUN_ON-2023_09_13-20h_17m_53s.log  -C /lkp/benchmarks/ltp/output/LTP_RUN_ON-2023_09_13-20h_17m_53s.failed -T /lkp/benchmarks/ltp/output/LTP_RUN_ON-2023_09_13-20h_17m_53s.tconf
>> > LOG File: /lkp/benchmarks/ltp/results/LTP_RUN_ON-2023_09_13-20h_17m_53s.log
>> > FAILED COMMAND File: /lkp/benchmarks/ltp/output/LTP_RUN_ON-2023_09_13-20h_17m_53s.failed
>> > TCONF COMMAND File: /lkp/benchmarks/ltp/output/LTP_RUN_ON-2023_09_13-20h_17m_53s.tconf
>> > Running tests.......
>> > <<<test_start>>>
>> > tag=readahead01 stime=1694636274
>> > cmdline="readahead01"
>> > contacts=""
>> > analysis=exit
>> > <<<test_output>>>
>> > tst_test.c:1558: TINFO: Timeout per run is 0h 02m 30s
>> > readahead01.c:36: TINFO: test_bad_fd -1
>> > readahead01.c:37: TPASS: readahead(-1, 0, getpagesize()) : EBADF (9)
>> > readahead01.c:39: TINFO: test_bad_fd O_WRONLY
>> > readahead01.c:45: TPASS: readahead(fd, 0, getpagesize()) : EBADF (9)
>> > readahead01.c:54: TINFO: test_invalid_fd pipe
>> > readahead01.c:56: TPASS: readahead(fd[0], 0, getpagesize()) : EINVAL (22)
>> > readahead01.c:60: TINFO: test_invalid_fd socket
>> > readahead01.c:62: TFAIL: readahead(fd[0], 0, getpagesize()) succeeded
>> >
>>
>> Reuben,
>>
>> This report is on an old version of your patch.
>> However:
>> 1. LTP test readahead01 will need to be fixed to accept also ESPIPE
>> 2. I am surprised that with the old patch readahead on socket did not
>>     fail. Does socket have aops?
>>
>> Please try to run LTP test readahead01 on the patch that Christian queued
>> and see how it behaves and if anything needs to be fixed wrt sockets.
>>
>> Thanks,
>> Amir.
>
>
> ack.  Will try to test.  My Ubuntu 22.04 system wasn't able to find packages called
> for by the test case, so it'll take me a little while to figure out how to get the
> test case working...

Heh! you can write a small C program instead, you don't even need to
build the LTP test.

It is clear what the failed test is doing:

static void test_invalid_fd(void)
{
        int fd[2];

        tst_res(TINFO, "%s pipe", __func__);
        SAFE_PIPE(fd);
        TST_EXP_FAIL(readahead(fd[0], 0, getpagesize()), EINVAL);
        SAFE_CLOSE(fd[0]);
        SAFE_CLOSE(fd[1]);

        tst_res(TINFO, "%s socket", __func__);
        fd[0] = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0);
        TST_EXP_FAIL(readahead(fd[0], 0, getpagesize()), EINVAL);
        SAFE_CLOSE(fd[0]);
}

The report claims that readahead on socket succeeds
and this is surprising.

Thanks,
Amir.