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

[David Hildenbrand]

> > 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