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.
>> diff --git a/mm/swap.c b/mm/swap.c
>> index 835bdf324b76..f2457acae383 100644
>> --- a/mm/swap.c
>> +++ b/mm/swap.c
>> @@ -472,6 +472,8 @@ static void folio_inc_refs(struct folio *folio)
>> */
>> void folio_mark_accessed(struct folio *folio)
>> {
>> + if (folio_test_uncached(folio))
>> + return;
>
> if (folio_test_uncached(folio)) {
> if (folio_mapped(folio))
> folio_clear_uncached(folio);
> else
> return;
> }
Noted, thanks!
--
Jens Axboe
