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

[Reuben Hawkins]

On Sat, Sep 23, 2023 at 7:20 AM Reuben Hawkins <reubenhwk@gmail.com> wrote:

>
>
> On Sat, Sep 23, 2023 at 12:56 AM Amir Goldstein <amir73il@gmail.com>
> wrote:
>
>> On Fri, Sep 22, 2023 at 11:29 PM Reuben Hawkins <reubenhwk@gmail.com>
>> wrote:
>> >
>> >
>> >
>> > On Fri, Sep 22, 2023 at 4:09 AM Cyril Hrubis <chrubis@suse.cz> wrote:
>> >>
>> >> Hi!
>> >> > 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...
>> >>
>> >> Huh? The test is a simple C binary you shouldn't need anything more
>> >> than:
>> >>
>> >> $ git clone https://github.com/linux-test-project/ltp.git
>> >> $ cd ltp
>> >> $ make autotools
>> >> $ ./configure
>> >>
>> >> $ cd testcases/kernel/syscalls/readahead
>> >> $ make
>> >> $ ./readahead01
>> >>
>> >> And this is well described in the readme at:
>> >>
>> >> https://github.com/linux-test-project/ltp/
>> >>
>> >> And the packages required for the compilation are make, C compiler and
>> >> autotools nothing extraordinary.
>> >>
>> > Awesome.  That was simpler than whatever it was I was trying.  I've
>> > reproduced the failed test and will try a few variations on the patch.
>>
>> Cool.
>>
>> For people that were not following the patch review,
>> the goal is not to pass the existing test.
>>
>> We decided to deliberately try the change of behavior
>> from EINVAL to ESPIPE, to align with fadvise behavior,
>> so eventually the LTP test should be changed to allow both.
>>
>> It was the test failure on the socket that alarmed me.
>> However, if we will have to special case socket in
>> readahead() after all, we may as well also special case
>> pipe with it and retain the EINVAL behavior - let's see
>> what your findings are and decide.
>>
>> Thanks,
>> Amir.
>>
>
> I don't want to change the behavior other than to fix readahead on block
> devices.  If we change the test now we're likely to find out that we broke
> somebody's application who hardcoded the return value handling of readahead
> to look specifically for rc != EINVAL.
>
> I think my v1 patch, in this regard, is better than the v2 patch.  It
> doesn't
> break the test.  It doesn't make the readahead man page wrong.   And it
> fixes
> readahead on block devices.   ...however I think I had a typo in the v1
> patch,
> so I think what I'll do is resubmit the v1 patch as v3 with the typoes
> fixed.
> I think I had F_ISREG vs S_ISREG in a couple places in the commit message.
>
> -Reuben
>

I may have to retract my last message because it appears to me the v2 patch
just passed the test in the latest kernel I built last night.  Will have to
double check exactly what happened after coffee.

-Reuben