Messages in this thread | | | Subject | Re: [PATCH 5/7] mm: free user PTE page table pages | From | Mika Penttilä <> | Date | Mon, 19 Jul 2021 16:55:10 +0300 |
| |
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().
> 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.
| |