On 1/29/19 2:12 AM, Jan Kara wrote: > On Mon 28-01-19 22:41:41, John Hubbard wrote: [...] >> Here is the case I'm wondering about: >> >> thread A thread B >> -------- -------- >> gup_fast >> page_mkclean >> is page gup-pinned?(no) >> page_cache_get_speculative >> (gup-pins the page here) >> check pte_val unchanged (yes) >> set_pte_at() >> >> ...and now thread A has created a read-only PTE, after gup_fast walked >> the page tables and found a writeable entry. And so far, thread A has >> not seen that the page is pinned. >> >> What am I missing here? The above seems like a problem even before we >> change anything. > > Your implementation of page_mkclean() is wrong :) It needs to first call > set_pte_at() and only after that ask "is page gup pinned?". In fact, > page_mkclean() probably has no bussiness in checking for page pins > whatsoever. It is clear_page_dirty_for_io() that cares, so that should > check for page pins after page_mkclean() has returned. > Perfect, that was the missing piece for me: page_mkclean() internally doesn't need the consistent view, just the caller does. The whole situation with two distinct lock-free algorithms going on here actually seems clear at last. :) Thanks (also to Jerome) for explaining this! thanks, -- John Hubbard NVIDIA
