[catching up on mails]
indicates that file truncation seems to end up messing with a PFNMAP mapping
that has PAT set. That is ... weird. I would have thought that PFNMAP would
never really happen with file truncation.
Does this only happen with an OOT driver, that seems to do weird truncate
stuff on files that have a PFNMAP mapping?
[1]
https://lore.kernel.org/all/3879ee72-84de-4d2a-93a8-c0b3dc3f0a4c@xxxxxxxxxx/
Ohhh.. I guess this will also stop working in VFIO, but I think it's fine
for now because as Yan pointed out VFIO PCI doesn't register those regions
now so VM_PAT is not yet set..
Interesting, I was assuming that VFIO might be relying on that.
And one thing I said wrong in the previous reply to Yan is, obviously
memtype_check_insert() can work with >1 owners as long as the memtype
matches.. and that's how fork() works where VM_PAT needs to be duplicated.
But this whole thing is a bit confusing to me.. As I think it also means
when fork the track_pfn_copy() will call memtype_kernel_map_sync one more
time even if we're 100% sure the pgprot will be the same for the kernel
mappings..
I consider the VM_PAT code quite ugly and I wish we could just get rid
of it (especially, the automatic "entire VMA covered" handling thingy).
I wonder whether there's some way that untrack pfn framework doesn't need
to rely on the pgtable to fetch the pfn, because VFIO MMIO region
protection will also do that in the near future, AFAICT. The pgprot part
should be easy there to fetch: get_pat_info() should fallback to vma's
pgprot if no mapping found; the only outlier should be CoW pages in
reality. The pfn is the real issue so far, so that either track_pfn_copy()
or untrack_pfn() may need to know the pfn to untrack, even if it only has
the vma information.
I had a prototype to store that information per VMA to avoid the page
table lookup. VMA splitting was a bit "added complication", but I got it
to work. (maybe I can still find it if there is demand)
The downside was having to consume more memory for all VMAs in the
system simply (even if only 8 byte) because a handful of VMAs in the
system could be VM_PAT. I decided that's not what we want. I managed to
not consume memory in some configurations, but not in all, so I
discarded that approach.
I did not explore storing that information in some auxiliary datastructure.
IMHO the whole VM_PAT model is weird:
1) mmap()
2) remap_pfn_range(): if it covers the whole VMA apply some magic
reservation.
3) munmap(): we unmap *all* PFNs and, therefore, clean up VM_PAT
(VMA splitting make the whole model weirder, but it works, because we
never merge these VMAs)
This model cannot properly work if we get partial page table zapping via
truncation/MADV_DONTNEED or similar things after 2). And likely we also
shouldn't be doing it that way. We should forbid any partial unmappings
in that model, just like we already disallow MADV_DONTNEED as you note.
As you mention in your other comment, maybe relevant/all? caller should
just manage the PAT side independently. So maybe we can move to a
different model.
--
Cheers,
David / dhildenb
