| Message ID | 20211110084057.27676-3-zhengqi.arch@bytedance.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Free user PTE page table pages | expand |
On Wed, Nov 10, 2021 at 04:40:44PM +0800, Qi Zheng wrote: > Currently we have some times the following judgments repeated in the > code: > > is_swap_pmd(*pmd) || pmd_trans_huge(*pmd) || pmd_devmap(*pmd) > > which is to determine whether the *pmd is a huge pmd, so introduce > is_huge_pmd() helper to deduplicate them. Isn't this pmd_leaf() ? Jason
On 11/10/21 8:29 PM, Jason Gunthorpe wrote: > On Wed, Nov 10, 2021 at 04:40:44PM +0800, Qi Zheng wrote: >> Currently we have some times the following judgments repeated in the >> code: >> >> is_swap_pmd(*pmd) || pmd_trans_huge(*pmd) || pmd_devmap(*pmd) >> >> which is to determine whether the *pmd is a huge pmd, so introduce >> is_huge_pmd() helper to deduplicate them. > > Isn't this pmd_leaf() ? Currently, the implementation of pmd_leaf() does not include pmd_devmap() checks. But considering the semantics of pmd_leaf(), the "devmap" pmd should also belong to "leaf" pmd. Maybe we should modify pmd_leaf() to make it more semantically consistent? By the way, something went wrong when sending this patchset, and I have re-sent the complete patchset, please comment over there. Thanks, Qi > > Jason >
On Wed, Nov 10, 2021 at 08:58:35PM +0800, Qi Zheng wrote: > > On 11/10/21 8:29 PM, Jason Gunthorpe wrote: > > On Wed, Nov 10, 2021 at 04:40:44PM +0800, Qi Zheng wrote: > > > Currently we have some times the following judgments repeated in the > > > code: > > > > > > is_swap_pmd(*pmd) || pmd_trans_huge(*pmd) || pmd_devmap(*pmd) > > > > > > which is to determine whether the *pmd is a huge pmd, so introduce > > > is_huge_pmd() helper to deduplicate them. > > > > Isn't this pmd_leaf() ? > > Currently, the implementation of pmd_leaf() does not include > pmd_devmap() checks. Are you sure? I thought x86 did via the tricky bit checks? > But considering the semantics of pmd_leaf(), the "devmap" pmd should > also belong to "leaf" pmd. Maybe we should modify pmd_leaf() to make > it more semantically consistent? I would prefer that.. Jason
diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h index f280f33ff223..b37a89180846 100644 --- a/include/linux/huge_mm.h +++ b/include/linux/huge_mm.h @@ -199,8 +199,7 @@ void __split_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd, #define split_huge_pmd(__vma, __pmd, __address) \ do { \ pmd_t *____pmd = (__pmd); \ - if (is_swap_pmd(*____pmd) || pmd_trans_huge(*____pmd) \ - || pmd_devmap(*____pmd)) \ + if (is_huge_pmd(*____pmd)) \ __split_huge_pmd(__vma, __pmd, __address, \ false, NULL); \ } while (0) @@ -232,11 +231,16 @@ static inline int is_swap_pmd(pmd_t pmd) return !pmd_none(pmd) && !pmd_present(pmd); } +static inline int is_huge_pmd(pmd_t pmd) +{ + return is_swap_pmd(pmd) || pmd_trans_huge(pmd) || pmd_devmap(pmd); +} + /* mmap_lock must be held on entry */ static inline spinlock_t *pmd_trans_huge_lock(pmd_t *pmd, struct vm_area_struct *vma) { - if (is_swap_pmd(*pmd) || pmd_trans_huge(*pmd) || pmd_devmap(*pmd)) + if (is_huge_pmd(*pmd)) return __pmd_trans_huge_lock(pmd, vma); else return NULL; diff --git a/mm/huge_memory.c b/mm/huge_memory.c index e5483347291c..e76ee2e1e423 100644 --- a/mm/huge_memory.c +++ b/mm/huge_memory.c @@ -1832,8 +1832,7 @@ spinlock_t *__pmd_trans_huge_lock(pmd_t *pmd, struct vm_area_struct *vma) { spinlock_t *ptl; ptl = pmd_lock(vma->vm_mm, pmd); - if (likely(is_swap_pmd(*pmd) || pmd_trans_huge(*pmd) || - pmd_devmap(*pmd))) + if (likely(is_huge_pmd(*pmd))) return ptl; spin_unlock(ptl); return NULL; diff --git a/mm/memory.c b/mm/memory.c index 855486fff526..b00cd60fc368 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -1146,8 +1146,7 @@ copy_pmd_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma, src_pmd = pmd_offset(src_pud, addr); do { next = pmd_addr_end(addr, end); - if (is_swap_pmd(*src_pmd) || pmd_trans_huge(*src_pmd) - || pmd_devmap(*src_pmd)) { + if (is_huge_pmd(*src_pmd)) { int err; VM_BUG_ON_VMA(next-addr != HPAGE_PMD_SIZE, src_vma); err = copy_huge_pmd(dst_mm, src_mm, dst_pmd, src_pmd, @@ -1441,7 +1440,7 @@ static inline unsigned long zap_pmd_range(struct mmu_gather *tlb, pmd = pmd_offset(pud, addr); do { next = pmd_addr_end(addr, end); - if (is_swap_pmd(*pmd) || pmd_trans_huge(*pmd) || pmd_devmap(*pmd)) { + if (is_huge_pmd(*pmd)) { if (next - addr != HPAGE_PMD_SIZE) __split_huge_pmd(vma, pmd, addr, false, NULL); else if (zap_huge_pmd(tlb, vma, pmd, addr)) diff --git a/mm/mprotect.c b/mm/mprotect.c index e552f5e0ccbd..2d5064a4631c 100644 --- a/mm/mprotect.c +++ b/mm/mprotect.c @@ -257,7 +257,7 @@ static inline unsigned long change_pmd_range(struct vm_area_struct *vma, mmu_notifier_invalidate_range_start(&range); } - if (is_swap_pmd(*pmd) || pmd_trans_huge(*pmd) || pmd_devmap(*pmd)) { + if (is_huge_pmd(*pmd)) { if (next - addr != HPAGE_PMD_SIZE) { __split_huge_pmd(vma, pmd, addr, false, NULL); } else { diff --git a/mm/mremap.c b/mm/mremap.c index 002eec83e91e..c6e9da09dd0a 100644 --- a/mm/mremap.c +++ b/mm/mremap.c @@ -532,8 +532,7 @@ unsigned long move_page_tables(struct vm_area_struct *vma, new_pmd = alloc_new_pmd(vma->vm_mm, vma, new_addr); if (!new_pmd) break; - if (is_swap_pmd(*old_pmd) || pmd_trans_huge(*old_pmd) || - pmd_devmap(*old_pmd)) { + if (is_huge_pmd(*old_pmd)) { if (extent == HPAGE_PMD_SIZE && move_pgt_entry(HPAGE_PMD, vma, old_addr, new_addr, old_pmd, new_pmd, need_rmap_locks))
Currently we have some times the following judgments repeated in the code: is_swap_pmd(*pmd) || pmd_trans_huge(*pmd) || pmd_devmap(*pmd) which is to determine whether the *pmd is a huge pmd, so introduce is_huge_pmd() helper to deduplicate them. Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> --- include/linux/huge_mm.h | 10 +++++++--- mm/huge_memory.c | 3 +-- mm/memory.c | 5 ++--- mm/mprotect.c | 2 +- mm/mremap.c | 3 +-- 5 files changed, 12 insertions(+), 11 deletions(-)