Skip to content

Commit c33c794

Browse files
ryanhrobakpm00
authored andcommitted
mm: ptep_get() conversion
Convert all instances of direct pte_t* dereferencing to instead use ptep_get() helper. This means that by default, the accesses change from a C dereference to a READ_ONCE(). This is technically the correct thing to do since where pgtables are modified by HW (for access/dirty) they are volatile and therefore we should always ensure READ_ONCE() semantics. But more importantly, by always using the helper, it can be overridden by the architecture to fully encapsulate the contents of the pte. Arch code is deliberately not converted, as the arch code knows best. It is intended that arch code (arm64) will override the default with its own implementation that can (e.g.) hide certain bits from the core code, or determine young/dirty status by mixing in state from another source. Conversion was done using Coccinelle: ---- // $ make coccicheck \ // COCCI=ptepget.cocci \ // SPFLAGS="--include-headers" \ // MODE=patch virtual patch @ depends on patch @ pte_t *v; @@ - *v + ptep_get(v) ---- Then reviewed and hand-edited to avoid multiple unnecessary calls to ptep_get(), instead opting to store the result of a single call in a variable, where it is correct to do so. This aims to negate any cost of READ_ONCE() and will benefit arch-overrides that may be more complex. Included is a fix for an issue in an earlier version of this patch that was pointed out by kernel test robot. The issue arose because config MMU=n elides definition of the ptep helper functions, including ptep_get(). HUGETLB_PAGE=n configs still define a simple huge_ptep_clear_flush() for linking purposes, which dereferences the ptep. So when both configs are disabled, this caused a build error because ptep_get() is not defined. Fix by continuing to do a direct dereference when MMU=n. This is safe because for this config the arch code cannot be trying to virtualize the ptes because none of the ptep helpers are defined. Link: https://lkml.kernel.org/r/20230612151545.3317766-4-ryan.roberts@arm.com Reported-by: kernel test robot <lkp@intel.com> Link: https://lore.kernel.org/oe-kbuild-all/202305120142.yXsNEo6H-lkp@intel.com/ Signed-off-by: Ryan Roberts <ryan.roberts@arm.com> Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Alexander Potapenko <glider@google.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Alex Williamson <alex.williamson@redhat.com> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Andrey Konovalov <andreyknvl@gmail.com> Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com> Cc: Christian Brauner <brauner@kernel.org> Cc: Christoph Hellwig <hch@infradead.org> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Dave Airlie <airlied@gmail.com> Cc: Dimitri Sivanich <dimitri.sivanich@hpe.com> Cc: Dmitry Vyukov <dvyukov@google.com> Cc: Ian Rogers <irogers@google.com> Cc: Jason Gunthorpe <jgg@ziepe.ca> Cc: Jérôme Glisse <jglisse@redhat.com> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Johannes Weiner <hannes@cmpxchg.org> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> Cc: Lorenzo Stoakes <lstoakes@gmail.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Matthew Wilcox <willy@infradead.org> Cc: Miaohe Lin <linmiaohe@huawei.com> Cc: Michal Hocko <mhocko@kernel.org> Cc: Mike Kravetz <mike.kravetz@oracle.com> Cc: Mike Rapoport (IBM) <rppt@kernel.org> Cc: Muchun Song <muchun.song@linux.dev> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Naoya Horiguchi <naoya.horiguchi@nec.com> Cc: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> Cc: Pavel Tatashin <pasha.tatashin@soleen.com> Cc: Roman Gushchin <roman.gushchin@linux.dev> Cc: SeongJae Park <sj@kernel.org> Cc: Shakeel Butt <shakeelb@google.com> Cc: Uladzislau Rezki (Sony) <urezki@gmail.com> Cc: Vincenzo Frascino <vincenzo.frascino@arm.com> Cc: Yu Zhao <yuzhao@google.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
1 parent 6c1d2a0 commit c33c794

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+301
-228
lines changed

drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1681,7 +1681,9 @@ static int igt_mmap_gpu(void *arg)
16811681

16821682
static int check_present_pte(pte_t *pte, unsigned long addr, void *data)
16831683
{
1684-
if (!pte_present(*pte) || pte_none(*pte)) {
1684+
pte_t ptent = ptep_get(pte);
1685+
1686+
if (!pte_present(ptent) || pte_none(ptent)) {
16851687
pr_err("missing PTE:%lx\n",
16861688
(addr - (unsigned long)data) >> PAGE_SHIFT);
16871689
return -EINVAL;
@@ -1692,7 +1694,9 @@ static int check_present_pte(pte_t *pte, unsigned long addr, void *data)
16921694

16931695
static int check_absent_pte(pte_t *pte, unsigned long addr, void *data)
16941696
{
1695-
if (pte_present(*pte) && !pte_none(*pte)) {
1697+
pte_t ptent = ptep_get(pte);
1698+
1699+
if (pte_present(ptent) && !pte_none(ptent)) {
16961700
pr_err("present PTE:%lx; expected to be revoked\n",
16971701
(addr - (unsigned long)data) >> PAGE_SHIFT);
16981702
return -EINVAL;

drivers/misc/sgi-gru/grufault.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ static int atomic_pte_lookup(struct vm_area_struct *vma, unsigned long vaddr,
228228
goto err;
229229
#ifdef CONFIG_X86_64
230230
if (unlikely(pmd_large(*pmdp)))
231-
pte = *(pte_t *) pmdp;
231+
pte = ptep_get((pte_t *)pmdp);
232232
else
233233
#endif
234234
pte = *pte_offset_kernel(pmdp, vaddr);

drivers/vfio/vfio_iommu_type1.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,7 @@ static int follow_fault_pfn(struct vm_area_struct *vma, struct mm_struct *mm,
514514
bool write_fault)
515515
{
516516
pte_t *ptep;
517+
pte_t pte;
517518
spinlock_t *ptl;
518519
int ret;
519520

@@ -536,10 +537,12 @@ static int follow_fault_pfn(struct vm_area_struct *vma, struct mm_struct *mm,
536537
return ret;
537538
}
538539

539-
if (write_fault && !pte_write(*ptep))
540+
pte = ptep_get(ptep);
541+
542+
if (write_fault && !pte_write(pte))
540543
ret = -EFAULT;
541544
else
542-
*pfn = pte_pfn(*ptep);
545+
*pfn = pte_pfn(pte);
543546

544547
pte_unmap_unlock(ptep, ptl);
545548
return ret;

drivers/xen/privcmd.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -949,7 +949,7 @@ static int privcmd_mmap(struct file *file, struct vm_area_struct *vma)
949949
*/
950950
static int is_mapped_fn(pte_t *pte, unsigned long addr, void *data)
951951
{
952-
return pte_none(*pte) ? 0 : -EBUSY;
952+
return pte_none(ptep_get(pte)) ? 0 : -EBUSY;
953953
}
954954

955955
static int privcmd_vma_range_is_mapped(

fs/proc/task_mmu.c

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -538,13 +538,14 @@ static void smaps_pte_entry(pte_t *pte, unsigned long addr,
538538
bool locked = !!(vma->vm_flags & VM_LOCKED);
539539
struct page *page = NULL;
540540
bool migration = false, young = false, dirty = false;
541+
pte_t ptent = ptep_get(pte);
541542

542-
if (pte_present(*pte)) {
543-
page = vm_normal_page(vma, addr, *pte);
544-
young = pte_young(*pte);
545-
dirty = pte_dirty(*pte);
546-
} else if (is_swap_pte(*pte)) {
547-
swp_entry_t swpent = pte_to_swp_entry(*pte);
543+
if (pte_present(ptent)) {
544+
page = vm_normal_page(vma, addr, ptent);
545+
young = pte_young(ptent);
546+
dirty = pte_dirty(ptent);
547+
} else if (is_swap_pte(ptent)) {
548+
swp_entry_t swpent = pte_to_swp_entry(ptent);
548549

549550
if (!non_swap_entry(swpent)) {
550551
int mapcount;
@@ -732,11 +733,12 @@ static int smaps_hugetlb_range(pte_t *pte, unsigned long hmask,
732733
struct mem_size_stats *mss = walk->private;
733734
struct vm_area_struct *vma = walk->vma;
734735
struct page *page = NULL;
736+
pte_t ptent = ptep_get(pte);
735737

736-
if (pte_present(*pte)) {
737-
page = vm_normal_page(vma, addr, *pte);
738-
} else if (is_swap_pte(*pte)) {
739-
swp_entry_t swpent = pte_to_swp_entry(*pte);
738+
if (pte_present(ptent)) {
739+
page = vm_normal_page(vma, addr, ptent);
740+
} else if (is_swap_pte(ptent)) {
741+
swp_entry_t swpent = pte_to_swp_entry(ptent);
740742

741743
if (is_pfn_swap_entry(swpent))
742744
page = pfn_swap_entry_to_page(swpent);
@@ -1105,7 +1107,7 @@ static inline void clear_soft_dirty(struct vm_area_struct *vma,
11051107
* Documentation/admin-guide/mm/soft-dirty.rst for full description
11061108
* of how soft-dirty works.
11071109
*/
1108-
pte_t ptent = *pte;
1110+
pte_t ptent = ptep_get(pte);
11091111

11101112
if (pte_present(ptent)) {
11111113
pte_t old_pte;
@@ -1194,7 +1196,7 @@ static int clear_refs_pte_range(pmd_t *pmd, unsigned long addr,
11941196
return 0;
11951197
}
11961198
for (; addr != end; pte++, addr += PAGE_SIZE) {
1197-
ptent = *pte;
1199+
ptent = ptep_get(pte);
11981200

11991201
if (cp->type == CLEAR_REFS_SOFT_DIRTY) {
12001202
clear_soft_dirty(vma, addr, pte);
@@ -1550,7 +1552,7 @@ static int pagemap_pmd_range(pmd_t *pmdp, unsigned long addr, unsigned long end,
15501552
for (; addr < end; pte++, addr += PAGE_SIZE) {
15511553
pagemap_entry_t pme;
15521554

1553-
pme = pte_to_pagemap_entry(pm, vma, addr, *pte);
1555+
pme = pte_to_pagemap_entry(pm, vma, addr, ptep_get(pte));
15541556
err = add_to_pagemap(addr, &pme, pm);
15551557
if (err)
15561558
break;
@@ -1893,10 +1895,11 @@ static int gather_pte_stats(pmd_t *pmd, unsigned long addr,
18931895
return 0;
18941896
}
18951897
do {
1896-
struct page *page = can_gather_numa_stats(*pte, vma, addr);
1898+
pte_t ptent = ptep_get(pte);
1899+
struct page *page = can_gather_numa_stats(ptent, vma, addr);
18971900
if (!page)
18981901
continue;
1899-
gather_stats(page, md, pte_dirty(*pte), 1);
1902+
gather_stats(page, md, pte_dirty(ptent), 1);
19001903

19011904
} while (pte++, addr += PAGE_SIZE, addr != end);
19021905
pte_unmap_unlock(orig_pte, ptl);

fs/userfaultfd.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,7 @@ static inline bool userfaultfd_must_wait(struct userfaultfd_ctx *ctx,
335335
pud_t *pud;
336336
pmd_t *pmd, _pmd;
337337
pte_t *pte;
338+
pte_t ptent;
338339
bool ret = true;
339340

340341
mmap_assert_locked(mm);
@@ -374,9 +375,10 @@ static inline bool userfaultfd_must_wait(struct userfaultfd_ctx *ctx,
374375
* changes under us. PTE markers should be handled the same as none
375376
* ptes here.
376377
*/
377-
if (pte_none_mostly(*pte))
378+
ptent = ptep_get(pte);
379+
if (pte_none_mostly(ptent))
378380
ret = true;
379-
if (!pte_write(*pte) && (reason & VM_UFFD_WP))
381+
if (!pte_write(ptent) && (reason & VM_UFFD_WP))
380382
ret = true;
381383
pte_unmap(pte);
382384

include/linux/hugetlb.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1185,7 +1185,11 @@ static inline void hugetlb_count_sub(long l, struct mm_struct *mm)
11851185
static inline pte_t huge_ptep_clear_flush(struct vm_area_struct *vma,
11861186
unsigned long addr, pte_t *ptep)
11871187
{
1188+
#ifdef CONFIG_MMU
1189+
return ptep_get(ptep);
1190+
#else
11881191
return *ptep;
1192+
#endif
11891193
}
11901194

11911195
static inline void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,

include/linux/mm_inline.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,7 @@ pte_install_uffd_wp_if_needed(struct vm_area_struct *vma, unsigned long addr,
555555
bool arm_uffd_pte = false;
556556

557557
/* The current status of the pte should be "cleared" before calling */
558-
WARN_ON_ONCE(!pte_none(*pte));
558+
WARN_ON_ONCE(!pte_none(ptep_get(pte)));
559559

560560
/*
561561
* NOTE: userfaultfd_wp_unpopulated() doesn't need this whole

include/linux/pgtable.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ static inline int ptep_test_and_clear_young(struct vm_area_struct *vma,
231231
unsigned long address,
232232
pte_t *ptep)
233233
{
234-
pte_t pte = *ptep;
234+
pte_t pte = ptep_get(ptep);
235235
int r = 1;
236236
if (!pte_young(pte))
237237
r = 0;
@@ -318,7 +318,7 @@ static inline pte_t ptep_get_and_clear(struct mm_struct *mm,
318318
unsigned long address,
319319
pte_t *ptep)
320320
{
321-
pte_t pte = *ptep;
321+
pte_t pte = ptep_get(ptep);
322322
pte_clear(mm, address, ptep);
323323
page_table_check_pte_clear(mm, address, pte);
324324
return pte;
@@ -519,7 +519,7 @@ extern pud_t pudp_huge_clear_flush(struct vm_area_struct *vma,
519519
struct mm_struct;
520520
static inline void ptep_set_wrprotect(struct mm_struct *mm, unsigned long address, pte_t *ptep)
521521
{
522-
pte_t old_pte = *ptep;
522+
pte_t old_pte = ptep_get(ptep);
523523
set_pte_at(mm, address, ptep, pte_wrprotect(old_pte));
524524
}
525525
#endif

kernel/events/uprobes.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ static int __replace_page(struct vm_area_struct *vma, unsigned long addr,
192192
inc_mm_counter(mm, MM_ANONPAGES);
193193
}
194194

195-
flush_cache_page(vma, addr, pte_pfn(*pvmw.pte));
195+
flush_cache_page(vma, addr, pte_pfn(ptep_get(pvmw.pte)));
196196
ptep_clear_flush_notify(vma, addr, pvmw.pte);
197197
if (new_page)
198198
set_pte_at_notify(mm, addr, pvmw.pte,

0 commit comments

Comments
 (0)