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

[Matthew Wilcox <willy@infradead.org>]

On Sun, Sep 24, 2023 at 11:35:48PM -0500, Reuben Hawkins wrote:
> The v2 patch does NOT return ESPIPE on a socket.  It succeeds.
> 
> readahead01.c:54: TINFO: test_invalid_fd pipe
> readahead01.c:56: TFAIL: readahead(fd[0], 0, getpagesize()) expected
> EINVAL: ESPIPE (29)
> readahead01.c:60: TINFO: test_invalid_fd socket
> readahead01.c:62: TFAIL: readahead(fd[0], 0, getpagesize()) succeeded
> <-------here

Thanks!  I am of the view that this is wrong (although probably
harmless).  I suspect what happens is that we take the
'bdi == &noop_backing_dev_info' condition in generic_fadvise()
(since I don't see anywhere in net/ setting f_op->fadvise) and so
return 0 without doing any work.

The correct solution is probably your v2, combined with:

        inode = file_inode(file);
-       if (S_ISFIFO(inode->i_mode))
+       if (S_ISFIFO(inode->i_mode) || S_ISSOCK(inode->i_mode))
                return -ESPIPE;

in generic_fadvise(), but that then changes the return value from
posix_fadvise(), as I outlined in my previous email.  And I'm OK with
that, because I think it's what POSIX intended.  Amir may well disagree
;-)