On Thu, Nov 05, 2015 at 07:03:24PM +0300, Vladimir Davydov wrote: > On Tue, Nov 03, 2015 at 05:26:15PM +0200, Kirill A. Shutemov wrote: > ... > > @@ -812,60 +812,104 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma, > > spinlock_t *ptl; > > int referenced = 0; > > struct page_referenced_arg *pra = arg; > > + pgd_t *pgd; > > + pud_t *pud; > > + pmd_t *pmd; > > + pte_t *pte; > > > > - if (unlikely(PageTransHuge(page))) { > > - pmd_t *pmd; > > - > > - /* > > - * rmap might return false positives; we must filter > > - * these out using page_check_address_pmd(). > > - */ > > - pmd = page_check_address_pmd(page, mm, address, &ptl); > > - if (!pmd) > > + if (unlikely(PageHuge(page))) { > > + /* when pud is not present, pte will be NULL */ > > + pte = huge_pte_offset(mm, address); > > + if (!pte) > > return SWAP_AGAIN; > > > > - if (vma->vm_flags & VM_LOCKED) { > > + ptl = huge_pte_lockptr(page_hstate(page), mm, pte); > > + goto check_pte; > > + } > > + > > + pgd = pgd_offset(mm, address); > > + if (!pgd_present(*pgd)) > > + return SWAP_AGAIN; > > + pud = pud_offset(pgd, address); > > + if (!pud_present(*pud)) > > + return SWAP_AGAIN; > > + pmd = pmd_offset(pud, address); > > + > > + if (pmd_trans_huge(*pmd)) { > > + int ret = SWAP_AGAIN; > > + > > + ptl = pmd_lock(mm, pmd); > > + if (!pmd_present(*pmd)) > > + goto unlock_pmd; > > + if (unlikely(!pmd_trans_huge(*pmd))) { > > spin_unlock(ptl); > > + goto map_pte; > > + } > > + > > + if (pmd_page(*pmd) != page) > > + goto unlock_pmd; > > + > > + if (vma->vm_flags & VM_LOCKED) { > > pra->vm_flags |= VM_LOCKED; > > - return SWAP_FAIL; /* To break the loop */ > > + ret = SWAP_FAIL; /* To break the loop */ > > + goto unlock_pmd; > > } > > > > if (pmdp_clear_flush_young_notify(vma, address, pmd)) > > referenced++; > > - > > spin_unlock(ptl); > > + goto found; > > +unlock_pmd: > > + spin_unlock(ptl); > > + return ret; > > } else { > > - pte_t *pte; > > - > > - /* > > - * rmap might return false positives; we must filter > > - * these out using page_check_address(). > > - */ > > - pte = page_check_address(page, mm, address, &ptl, 0); > > - if (!pte) > > + pmd_t pmde = *pmd; > > + barrier(); > > This is supposed to be > > pmd_t pmde = READ_ONCE(*pmd); > > Right? See e37c69827063. If I read this correctly, barrier() is less overhead for some archs. > > I don't understand why we need a barrier here. Why can't we just do > > } else if (!pmd_present(*pmd)) > reutnr SWAP_AGAIN; > > ? See f72e7dcdd252 too. > > + if (!pmd_present(pmde) || pmd_trans_huge(pmde)) > > return SWAP_AGAIN; > > + } -- Kirill A. Shutemov -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>