Messages in this thread | | | Subject | Re: [PATCH v3 07/12] mm/debug_vm_pgtable: Use struct pgtable_debug_args in PTE modifying tests | From | Gavin Shan <> | Date | Fri, 23 Jul 2021 14:23:24 +1000 |
| |
Hi Anshuman,
On 7/23/21 12:39 PM, Anshuman Khandual wrote: > On 7/22/21 12:07 PM, Gavin Shan wrote: >> >>>> pgdp = pgd_offset(mm, vaddr); >>>> p4dp = p4d_alloc(mm, pgdp, vaddr); >>>> @@ -1272,11 +1275,11 @@ static int __init debug_vm_pgtable(void) >>>> * Page table modifying tests. They need to hold >>>> * proper page table lock. >>>> */ >>>> - >>>> - ptep = pte_offset_map_lock(mm, pmdp, vaddr, &ptl); >>>> - pte_clear_tests(mm, ptep, pte_aligned, vaddr, prot); >>>> - pte_advanced_tests(mm, vma, ptep, pte_aligned, vaddr, prot); >>>> - pte_unmap_unlock(ptep, ptl); >>>> + ptl = pte_lockptr(args.mm, args.pmdp); >>>> + spin_lock(ptl); >>>> + pte_clear_tests(&args); >>>> + pte_advanced_tests(&args); >>>> + spin_unlock(ptl); >>> >>> Why pte_offset_map_lock()/pte_unmap_unlock() has been dropped and >>> spin_lock()/spin_unlock() sequence has been added ? Please dont >>> change the tests in these patches. >>> >> >> The semantics of pte_offset_map_lock() is to grab and take the lock >> and return the PTE entry, which is mapped if needed. We already had >> the PTE entry tracked by args->ptep in init_args(). So some of the >> operations covered by pte_offset_map_lock() isn't needed any more > > To keep the patch on purpose, please avoid this change here. But if > required, you could send a follow up patch later. >
In order to use pte_offset_map_lock() and pte_unmap_unlock(), we need a temporary variable @ptep to store the return value from pte_offset_map_lock(). The temporary variable @ptep is passed to pte_unmap_unlock(). It means we just need the temporary variable to keep the original implementation in this regard. I will keep pte_offset_map_lock() and pte_unmap_unlock() in v4.
If we really want to remove the temporary variable (@ptep), we can do it after this series gets merged.
Thanks, Gavin
| |