Peter Xu peterx@redhat.com writes:
On Fri, Nov 11, 2022 at 10:42:13AM +1100, Alistair Popple wrote:
Hi Peter, for the patch feel free to add:
Reviewed-by: Alistair Popple apopple@nvidia.com
Will do, thanks.
I did wonder if this should be backported further for migrate_vma as well given that a migration failure there might lead a shmem read-only PTE to become read-write. I couldn't think of an obvious reason why that would cause an actual problem though.
I think folio_mkclean() will wrprotect the pte for writeback to swap, but it holds the page lock which prevents migrate_vma installing migration entries in the first place.
I suppose there is a small window there because migrate_vma will unlock the page before removing the migration entries. So to be safe we could consider going back to 8763cb45ab96 ("mm/migrate: new memory migration helper for use with device memory") but I doubt in practice it's a real problem.
IIRC migrate_vma API only supports anonymous memory, then it's not affected by this issue?
It does only support anonymous memory, but it doesn't actually check that until after the page mapping has already been replaced with migration entries.
So I was worried that could remap a previously read-only page as read-write and what interaction that would have with a page that was swapped out to disk say. But I haven't tought of a case where that would be a problem, and I'm pretty convinced it isn't. It's just I haven't had the time to entirely prove that for myself.
One thing reminded me is I thought mprotect could be affected but I think it's actually not, because mprotect is vma-based, and that should always be fine with current mk_pte(). I'll remove the paragraph on mprotect in the commit message; that could be slightly misleading.