On 06.02.23 17:43, Matthew Wilcox wrote:
On Mon, Feb 06, 2023 at 05:35:55PM +0100, David Hildenbrand wrote:
On 06.02.23 17:33, Matthew Wilcox wrote:
On Mon, Feb 06, 2023 at 04:13:44PM +0100, David Hildenbrand wrote:
The handling of cow pages is still very clunky.
folio_add_new_anon_rmap() handles anonymous large folios just fine. I
think David was looking at current code, not the code in mm-next.
OK. Let's wait for further comment from David.
As I raised, page_add_new_anon_rmap() -> folio_add_new_anon_rmap() can be
used to add a fresh (a) PMD-mapped THP or (b) order-0 folio.
folio_add_new_anon_rmap() is not suitable for PTE-mapping a large folio.
Which is what we are intending to do here unless I am completely off.
I think you are. While the infrastructure here handles large folios
which are not PMDs, there's nobody who will allocate such a thing, so
there is no problem. Right>
And that's precisely what I want to have fenced off here. I want that
function to complain instead of silently doing the wrong thing.
If this were a widely called function, I'd agree. But there are two
callers of do_set_pte; one in filemap.c and one in memory.c. It's
overcautious and has created huge churn in this patchset. If you're
really that concerned, stick a VM_BUG_ON() in folio_add_new_anon_rmap()
instead of making weird stuff happen in set_pte_range().
We have
+ if (!cow) {
+ folio_add_file_rmap_range(folio, start, nr, vma, false);
+ add_mm_counter(vma->vm_mm, mm_counter_file(page), nr);
+ } else {
+ /*
+ * rmap code is not ready to handle COW with anonymous
+ * large folio yet. Capture and warn if large folio
+ * is given.
+ */
+ VM_WARN_ON_FOLIO(folio_test_large(folio), folio);
+ }
now.
What are we supposed to add instead on the else branch instead that
would be correct in the future? Or not look weird?
Go on, scream louder at me, I don't care.
--
Thanks,
David / dhildenb
