On Mon, Nov 11, 2024 at 07:12:35AM -0700, Jens Axboe wrote: > On 11/11/24 2:15 AM, Kirill A. Shutemov wrote: > >> @@ -2706,8 +2712,16 @@ ssize_t filemap_read(struct kiocb *iocb, struct iov_iter *iter, > >> } > >> } > >> put_folios: > >> - for (i = 0; i < folio_batch_count(&fbatch); i++) > >> - folio_put(fbatch.folios[i]); > >> + for (i = 0; i < folio_batch_count(&fbatch); i++) { > >> + struct folio *folio = fbatch.folios[i]; > >> + > >> + if (folio_test_uncached(folio)) { > >> + folio_lock(folio); > >> + invalidate_complete_folio2(mapping, folio, 0); > >> + folio_unlock(folio); > > > > I am not sure it is safe. What happens if it races with page fault? > > > > The only current caller of invalidate_complete_folio2() unmaps the folio > > explicitly before calling it. And folio lock prevents re-faulting. > > > > I think we need to give up PG_uncached if we see folio_mapped(). And maybe > > also mark the page accessed. > > Ok thanks, let me take a look at that and create a test case that > exercises that explicitly. It might be worth generalizing it to clearing PG_uncached for any page cache lookups that don't come from RWF_UNCACHED. -- Kiryl Shutsemau / Kirill A. Shutemov