Re: [PATCH v3 6/6] mm: swap: entirely map large folios found in swapcache

[David Hildenbrand <david@redhat.com>]

>> Let's assume a single subpage of a large folio is no longer mapped.
>> Then, we'd have:
>>
>> nr_pages == folio_nr_pages(folio) - 1.
>>
>> You could simply map+reuse most of the folio without COWing.
> 
> yes. This is good but the pte which is no longer mapped could be
> anyone within the nr_pages PTEs. so it could be quite tricky for
> set_ptes.

The swap batching logic should take care of that, otherwise it would be 
buggy.

> 
>>
>> Once we support COW reuse of PTE-mapped THP we'd do the same. Here, it's
>> just easy to detect that the folio is exclusive (folio_ref_count(folio)
>> == 1 before mapping anything).
>>
>> If you really want to mimic what do_wp_page() currently does, you should
>> have:
>>
>> exclusive || (folio_ref_count(folio) == 1 && !folio_test_large(folio))
> 
> I actually dislike the part that do_wp_page() handles the reuse of a large
> folio which is entirely mapped. For example, A forks B, B exit, we write
> A's large folio, we get nr_pages CoW of small folios. Ideally, we can
> reuse the whole folios for writing.

Yes, see the link I shared to what I am working on. There isn't really a 
question if what we do right now needs to be improved and all these 
scenarios are pretty obvious clear.

> 
>>
>> Personally, I think we should keep it simple here and use:
>>
>> exclusive || folio_ref_count(folio) == 1
> 
> I feel this is still better than
> exclusive || (folio_ref_count(folio) == 1 && !folio_test_large(folio))
> as we reuse the whole large folio. the do_wp_page() behaviour
> doesn't have this.

Yes, but there is the comment "Same logic as in do_wp_page();". We 
already ran into issues having different COW reuse logic all over the 
place. For this case here, I don't care if we leave it as

"exclusive || folio_ref_count(folio) == 1"

But let's not try inventing new stuff here.

-- 
Cheers,

David / dhildenb