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