On Tue, May 02, 2023 at 09:33:45PM +0200, David Hildenbrand wrote: > I'll just stress again that I think there are cases where we unmap and free > a page without synchronizing against GUP-fast using an IPI or RCU. AFAIK this is true on ARM64 and other arches that do not use IPIs for TLB shootdown. Eg the broadcast TLBI described here: https://developer.arm.com/documentation/101811/0102/Translation-Lookaside-Buffer-maintenance TLB invalidation of remote CPUs Is done at the interconnect level and does not trigger any interrupt. So, arches that don't use IPI have to RCU free the page table entires to work with GUP fast. They will set MMU_GATHER_RCU_TABLE_FREE. The whole interrupt disable thing in GUP turns into nothing more than a hacky RCU on those arches. The ugly bit is the comment: * We do not adopt an rcu_read_lock() here as we also want to block IPIs * that come from THPs splitting. Which, I think, today can be summarized in today's code base as serializing with split_huge_page_to_list(). I don't know this code well, but the comment there says "Only caller must hold pin on the @page" which is only possible if all the PTEs have been replaced with migration entries or otherwise before we get here. So the IPI the comment talks about, I suppose, is from the installation of the migration entry? However gup_huge_pmd() does the usual read, ref, check pattern, and split_huge_page_to_list() uses page_ref_freeze() which is cmpxchg So the three races seem to resolve themselves without IPI magic - GUP sees the THP folio before the migration entry and +1's the ref split_huge_page_to_list() bails beacuse the ref is elevated - GUP fails to +1 the ref because it is zero and bails, split_huge_page_to_list() made it zero, so it runs - GUP sees the THP folio, split_huge_page_to_list() ran to completion, and then GUP +1's a 4k page. The recheck of pmd_val will see the migration entry, or the new PTE table pointer, but never the original THP folio. So GUP will bail. The speculative ref on the 4k page is harmless. I can't figure out what this 2014 comment means in today's code. Or where ARM64 hid the "IPI broadcast on THP split" :) See commit 2667f50e8b81457fcb4a3dbe6aff3e81ea009e13 > That's one of the reasons why we recheck if the PTE changed to back off, so > I've been told. Yes, with no synchronizing point the refcount in GUP fast could be taken on the page after it has been free'd and reallocated, but this is only possible on RCU > I'm happy if someone proves me wrong and a page we just (temporarily) pinned > cannot have been freed+reused in the meantime. Sadly I think no.. We'd need to RCU free the page itself as well to make this true. Jason
