On 02.05.23 21:25, Lorenzo Stoakes wrote:
On Tue, May 02, 2023 at 04:07:50PM -0300, Jason Gunthorpe wrote:
On Tue, May 02, 2023 at 07:17:14PM +0100, Lorenzo Stoakes wrote:
On a specific point - if mapping turns out to be NULL after we confirm
stable PTE, I'd be inclined to reject and let the slow path take care of
it, would you agree that that's the correct approach?
I think in general if GUP fast detects any kind of race it should bail
to the slow path.
The races it tries to resolve itself should have really safe and
obvious solutions.
I think this comment is misleading:
+ /*
+ * GUP-fast disables IRQs - this prevents IPIs from causing page tables
+ * to disappear from under us, as well as preventing RCU grace periods
+ * from making progress (i.e. implying rcu_read_lock()).
True, but that is not important here since we are not reading page
tables
+ * This means we can rely on the folio remaining stable for all
+ * architectures, both those that set CONFIG_MMU_GATHER_RCU_TABLE_FREE
+ * and those that do not.
Not really clear. We have a valid folio refcount here, that is all.
Some of this is a product of mixed signals from different commenters and
my being perhaps a little _too_ willing to just go with the flow.
With interrupts disabled and IPI blocked, plus the assurances that
interrupts being disabled implied the RCU version of page table
manipulation is also blocked, my understanding was that remapping in this
process to another page could not occur.
Of course the folio is 'stable' in the sense we have a refcount on it, but
it is unlocked so things can change.
I'm guessing the RCU guarantees in the TLB logic are not as solid as IPI,
because in the IPI case it seems to me you couldn't even clear the PTE
entry before getting to the page table case.
Otherwise, I'm a bit uncertain actually as to how we can get to the point
where the folio->mapping is being manipulated. Is this why?
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.
That's one of the reasons why we recheck if the PTE changed to back off,
so I've been told.
I'm happy if someone proves me wrong and a page we just (temporarily)
pinned cannot have been freed+reused in the meantime.
--
Thanks,
David / dhildenb
