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
;-)
