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

[Reuben Hawkins]

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