On Thu, Jun 04, 2026 at 11:04:35AM -0600, Nico Pache wrote: > On Mon, Jun 1, 2026 at 9:00 AM Nico Pache <[email protected]> wrote: > > > > On Mon, Jun 1, 2026 at 5:14 AM David Hildenbrand (Arm) <[email protected]> > > wrote: > > > > > > On 6/1/26 12:47, Lance Yang wrote: > > > > > > > > > > > > On 2026/6/1 18:23, David Hildenbrand (Arm) wrote: > > > >> On 6/1/26 11:08, Lance Yang wrote: > > > >>> > > > >>> > > > >>> > > > >>> One small thing, I think we should probably keep the smp_wmb(), and > > > >>> just > > > >>> move it before the earlier pmd_populate(). > > > >>> > > > >>> IIUC, the ordering we want is still: > > > >>> > > > >>> clear old PTEs > > > >>> smp_wmb() > > > >>> pmd_populate() > > > >>> > > > >>> so another CPU cannot walk through the re-installed PMD and still > > > >>> observe > > > >>> the old PTEs, right? > > > >> > > > >> There is a smp_wmb() in __folio_mark_uptodate(), that should be > > > >> sufficient? > > > > > > > > Ah, cool! __folio_mark_uptodate() already does the job :P > > > > > > > > So yeah, no extra smp_wmb() needed here! > > > > > > Yeah. BTW, I think we'd need a spin_lock_nested(), so @Nico, treat my > > > code as a > > > draft. > > > > Okay, I read the above and did some investigating. > > > > I will try to implement and verify the changes you suggested :) > > I've implemented something slightly different actually and I *think* its > better! > > } else { > /* this is map_anon_folio_pte_nopf with no mmu update */ > __map_anon_folio_pte_nopf(folio, pte, vma, start_addr, > /*uffd_wp=*/ false); > smp_wmb(); > pmd_populate(mm, pmd, pmd_pgtable(_pmd)); > /* > * Some architectures (e.g. MIPS) walk the live page table in > * their implementation. update_mmu_cache_range() must be called > * with a valid page table hierarchy and the PTE lock held. > * Acquire it nested inside pmd_ptl when they are distinct locks. > */ > if (pte_ptl != pmd_ptl) > spin_lock_nested(pte_ptl, SINGLE_DEPTH_NESTING); > update_mmu_cache_range(NULL, vma, start_addr, pte, nr_pages); > if (pte_ptl != pmd_ptl) > spin_unlock(pte_ptl); > } > spin_unlock(pmd_ptl); > > The logic here is that when the PMD becomes visible, PTEs are already > populated (no possibility of spurious faults on local CPU) > > the SMP_WMB makes sure of the above > > And the pmd is installed with the pte and pmd lock both held through > the mmu_cache update. > > This follows the conventions used in pmd_install() and clears the > potential for local CPU faults hitting cleared PTE entries. > > I think both approaches are correct but this prevents any possibility > of my first point. although mmap_write_lock prevents this too. > > Let me know what you think. I can revert to your implementation but > this is what I tested.
Yeah let's go with the original implementation please :) Thanks! > > Cheers, > -- Nico > > > > > Or an even crazier idea... what if we ensure MIPS checks for PMD_none > > before walking a PTE table? > > > > -- Nico > > > > > > > > -- > > > Cheers, > > > > > > David > > > > Cheers, Lorenzo
