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

[Matthew Wilcox <willy@infradead.org>]

On Sat, Sep 23, 2023 at 08:56:28AM +0300, Amir Goldstein wrote:
> 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.

If I read it correctly, LTP is reporting that readhaead() on a socket
returned success instead of an error.  Sockets do have a_ops, right?
It's set to empty_aops in inode_init_always, I think.

It would be nice if we documented somewhere which pointers should be
checked for NULL for which cases ... it doesn't really make sense for
a socket inode to have an i_mapping since it doesn't have pagecache.
But maybe we rely on i_mapping always being set.

Irritatingly, POSIX specifies ESPIPE for pipes, but does not specify
what to do with sockets.  It's kind of a meaningless syscall for
any kind of non-seekable fd.  lseek() returns ESPIPE for sockets
as well as pipes, so I'd see this as an oversight.
https://pubs.opengroup.org/onlinepubs/9699919799/functions/posix_fadvise.html
https://pubs.opengroup.org/onlinepubs/9699919799/functions/lseek.html

Of course readahead() is a Linux-specific syscall, so we can do whatever
we want here, but I'm really tempted to just allow readahead() for
regular files and block devices.  Hmm.  Can we check FMODE_LSEEK
instead of (S_ISFILE || S_ISBLK)?

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp