| Message ID | 20240913084433.1016256-4-anshuman.khandual@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: Use pxdp_get() for accessing page table entries | expand |
On 13/09/2024 09:44, Anshuman Khandual wrote: > Convert PTE accesses via ptep_get() helper that defaults as READ_ONCE() but > also provides the platform an opportunity to override when required. > > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: David Hildenbrand <david@redhat.com> > Cc: Ryan Roberts <ryan.roberts@arm.com> > Cc: "Mike Rapoport (IBM)" <rppt@kernel.org> > Cc: linux-mm@kvack.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> > --- > include/linux/pgtable.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h > index 2a6a3cccfc36..05e6995c1b93 100644 > --- a/include/linux/pgtable.h > +++ b/include/linux/pgtable.h > @@ -1060,7 +1060,7 @@ static inline int pgd_same(pgd_t pgd_a, pgd_t pgd_b) > */ > #define set_pte_safe(ptep, pte) \ > ({ \ > - WARN_ON_ONCE(pte_present(*ptep) && !pte_same(*ptep, pte)); \ > + WARN_ON_ONCE(pte_present(ptep_get(ptep)) && !pte_same(ptep_get(ptep), pte)); \ Suggest reading once into a temporary so that the pte can't change between the 2 gets. In practice, it's not likely to be a huge problem for this instance since its under the PTL so can only be racing with HW update of access and dirty. But good practice IMHO: pte_t __old = ptep_get(ptep); \ WARN_ON_ONCE(pte_present(__old) && !pte_same(__old, pte)); \ Thanks, Ryan > set_pte(ptep, pte); \ > }) >
On 9/13/24 15:57, Ryan Roberts wrote: > On 13/09/2024 09:44, Anshuman Khandual wrote: >> Convert PTE accesses via ptep_get() helper that defaults as READ_ONCE() but >> also provides the platform an opportunity to override when required. >> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: David Hildenbrand <david@redhat.com> >> Cc: Ryan Roberts <ryan.roberts@arm.com> >> Cc: "Mike Rapoport (IBM)" <rppt@kernel.org> >> Cc: linux-mm@kvack.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> >> --- >> include/linux/pgtable.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h >> index 2a6a3cccfc36..05e6995c1b93 100644 >> --- a/include/linux/pgtable.h >> +++ b/include/linux/pgtable.h >> @@ -1060,7 +1060,7 @@ static inline int pgd_same(pgd_t pgd_a, pgd_t pgd_b) >> */ >> #define set_pte_safe(ptep, pte) \ >> ({ \ >> - WARN_ON_ONCE(pte_present(*ptep) && !pte_same(*ptep, pte)); \ >> + WARN_ON_ONCE(pte_present(ptep_get(ptep)) && !pte_same(ptep_get(ptep), pte)); \ > > Suggest reading once into a temporary so that the pte can't change between the 2 > gets. In practice, it's not likely to be a huge problem for this instance since > its under the PTL so can only be racing with HW update of access and dirty. But > good practice IMHO: > > pte_t __old = ptep_get(ptep); \ > WARN_ON_ONCE(pte_present(__old) && !pte_same(__old, pte)); \ Sure, will change as suggested. > > Thanks, > Ryan > >> set_pte(ptep, pte); \ >> }) >> >
diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h index 2a6a3cccfc36..05e6995c1b93 100644 --- a/include/linux/pgtable.h +++ b/include/linux/pgtable.h @@ -1060,7 +1060,7 @@ static inline int pgd_same(pgd_t pgd_a, pgd_t pgd_b) */ #define set_pte_safe(ptep, pte) \ ({ \ - WARN_ON_ONCE(pte_present(*ptep) && !pte_same(*ptep, pte)); \ + WARN_ON_ONCE(pte_present(ptep_get(ptep)) && !pte_same(ptep_get(ptep), pte)); \ set_pte(ptep, pte); \ })
Convert PTE accesses via ptep_get() helper that defaults as READ_ONCE() but also provides the platform an opportunity to override when required. Cc: Andrew Morton <akpm@linux-foundation.org> Cc: David Hildenbrand <david@redhat.com> Cc: Ryan Roberts <ryan.roberts@arm.com> Cc: "Mike Rapoport (IBM)" <rppt@kernel.org> Cc: linux-mm@kvack.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> --- include/linux/pgtable.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)