On Sat, Sep 09, 2023 at 09:36:10AM +0300, Amir Goldstein wrote: > On Sat, Sep 9, 2023 at 7:39 AM Reuben Hawkins <reubenhwk@xxxxxxxxx> wrote: > > > > Readahead was factored to call generic_fadvise. That refactor broke > > readahead on block devices. > > More accurately: That refactor added a S_ISREG restriction to the syscall > that broke readahead on block devices. > > > > > The fix is to check F_ISFIFO rather than F_ISREG. It would also work to > > not check and let generic_fadvise to do the checking, but then the > > generic_fadvise return value would have to be checked and changed from > > -ESPIPE to -EINVAL to comply with the readahead(2) man-pages. > > > > We do not treat the man-pages as a holy script :) > > It is quite likely that the code needs to change and the man-page will > also be changed to reflect the fact that ESPIPE is a possible return value. > In fact, see what the man page says about posix_fadvise(2): > > ESPIPE The specified file descriptor refers to a pipe or FIFO. > (ESPIPE is the error specified by POSIX, but before kernel version > 2.6.16, Linux returned EINVAL in this case.) > > My opinion is that we should drop the ISREG/ISFIFO altogether, > let the error code change to ESPIPE, and send a patch to man-pages > to reflect that change (after it was merged and released), > but I would like to hear what other people think. Probably fine with the understanding that if we get regression reports it needs to be reverted quickly and the two of you are around to take care of that... ;)
