Messages in this thread | | | Subject | Re: [PATCH 5/7] mm: free user PTE page table pages | From | Qi Zheng <> | Date | Mon, 19 Jul 2021 22:12:04 +0800 |
| |
On 7/19/21 9:55 PM, Mika Penttilä wrote: > > > On 19.7.2021 15.56, Qi Zheng wrote: >> On 7/18/21 2:19 PM, Mika Penttilä wrote: >> >>>> + >>>> +/* >>>> + * returns true if the pmd has been populated with PTE page table, >>>> + * or false for all other cases. >>>> + */ >>>> +bool pte_install_try_get(struct mm_struct *mm, pmd_t *pmd, >>>> pgtable_t *pte) >>>> +{ >>>> + spinlock_t *ptl; >>>> + bool retval = true; >>>> + >>>> +retry: >>>> + ptl = pmd_lock(mm, pmd); >>>> + if (likely(pmd_none(*pmd))) { >>>> + __pte_install(mm, pmd, pte); >>>> + } else if (pmd_leaf(*pmd) || !pmd_present(*pmd)) { >>>> + retval = false; >>>> + } else if (!pte_get_unless_zero(pmd)) { >>>> + spin_unlock(ptl); >>>> + goto retry; >>>> + } >>>> + spin_unlock(ptl); >>>> + return retval; >>>> +} >>>> + >>> >>> Can pte_get_unless_zero() return true above? Can the pmd have been by >>> populated by others? In that case the ref count is wrongly incremented. >>> >> >> Here we only have mmap_read_lock(mm), so the pmd can be populated with >> other PTE page table page after a page fault in a different thread B >> of this mm. In this case, thread B already hold a pte_refcount of the >> PTE page table page populated in the pmd, so pte_get_unless_zero() can >> return true above. >> > > Yes but if thread B populates the page table page and pte, then we also > increase the refcount with pte_get_unless_zero() , but dont decrease it > when notice !pte_none(). > And in the pte_none() case, the refcount is increased again, so double > accounting. see finish_fault().
The semantics of this function is to hold the pte_refcount count when it returns 1, and its caller is responsible for decrease the pte_refcount by calling pte_put(), like the following pattern:
do_anonymous_page()
--> pte_alloc_try_get()
do something about pte
pte_put()
Similarly in finish_fault().
Thanks,
Qi
> >> Similarly, if THP is enabled, the pmd also can be populated with a THP >> page, we can see more detail in comment in handle_pte_fault(). The >> pmd_leaf() above is to detect this situation. >
| |