>> Open questions: >> - Willy pointed out that the calls to folio_set_error() and >> folio_clear_uptodate() are not needed anymore in the read path when an >> error happens[2]. I still don't understand 100% why they aren't needed >> anymore as I see those functions are still called in iomap. It will be >> good to put that rationale as a part of the commit message. > > page_endio() was generic. It needed to handle a lot of cases. When it's > being inlined into various completion routines, we know which cases we > need to handle and can omit all the cases which we don't. > > We know the uptodate flag is clear. If the uptodate flag is set, > we don't call the filesystem's read path. Since we know it's clear, > we don't need to clear it. > Got it. > We don't need to set the error flag. Only some filesystems still use > the error flag, and orangefs isn't one of them. I'd like to get rid > of the error flag altogether, and I've sent patches in the past which > get us a lot closer to that desired outcome. Not sure we're there yet. > Regardless, generic code doesn't check the error flag. Thanks for the explanation. I think found the series you are referring here. https://lore.kernel.org/linux-mm/20220527155036.524743-1-willy@xxxxxxxxxxxxx/#t I see orangefs is still setting the error flag in orangefs_read_folio(), so it should be removed at some point? I also changed mpage to **not set** the error flag in the read path. It does beg the question whether block_read_full_folio() and iomap_finish_folio_read() should also follow the suit. -- Pankaj
