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

[Matthew Wilcox <willy@infradead.org>]

On Sun, Sep 24, 2023 at 12:08:46AM -0500, Reuben Hawkins wrote:
> Readahead was factored to call generic_fadvise.  That refactor added an S_ISREG
> restriction which broke readahead on block devices.
> 
> This change swaps out the existing restrictions with an FMODE_LSEEK check to
> fix block device readahead.
> 
> The readahead01.c and readahead02.c tests pass in ltp/testcases/...

I realise we could add new test cases _basically_ forever, but I'd like
to see a little more coverage in test_invalid_fd().  It currently tests
both pipes and sockets, but we have so many more fd types.  Maybe there
are good abstractions inside LTP already for creating these?  I'd
like to see tests that the following also return -EINVAL:

 - an io_uring fd
 - /dev/zero
 - /proc/self/maps (or something else in /proc we can get unprivileged
   access to)
 - a directory (debatable!  maybe we should allow prefetching a
   directory!)

This ad-hoc approach to testing syscalls is probably not the best idea.
Have the LTP considered a more thorough approach where we have a central
iterator that returns a file descriptor of various types (the ones listed
above, plus block devices, and regular files), and individual syscall
testcases can express whether this syscall should pass/fail for each type
of fd?  That would give us one central place to add new fd types, and we
wouldn't be relying on syzbot to try fds at random until something fails.

Or something.  I'm not an expert on the LTP or testing in general; it
just feels like we could do better here.