On Fri, 11 Jun 2021, Jason Gunthorpe wrote: > On Thu, Jun 10, 2021 at 11:37:14PM -0700, Hugh Dickins wrote: > > On Thu, 10 Jun 2021, Jason Gunthorpe wrote: > > > On Thu, Jun 10, 2021 at 12:06:17PM +0300, Kirill A. Shutemov wrote: > > > > On Wed, Jun 09, 2021 at 11:38:11PM -0700, Hugh Dickins wrote: > > > > > page_vma_mapped_walk() cleanup: use pmd_read_atomic() with barrier() > > > > > instead of READ_ONCE() for pmde: some architectures (e.g. i386 with PAE) > > > > > have a multi-word pmd entry, for which READ_ONCE() is not good enough. > > > > > > > > > > Signed-off-by: Hugh Dickins <hughd@xxxxxxxxxx> > > > > > Cc: <stable@xxxxxxxxxxxxxxx> > > > > > mm/page_vma_mapped.c | 5 ++++- > > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c > > > > > index 7c0504641fb8..973c3c4e72cc 100644 > > > > > +++ b/mm/page_vma_mapped.c > > > > > @@ -182,13 +182,16 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw) > > > > > pud = pud_offset(p4d, pvmw->address); > > > > > if (!pud_present(*pud)) > > > > > return false; > > > > > + > > > > > pvmw->pmd = pmd_offset(pud, pvmw->address); > > > > > /* > > > > > * Make sure the pmd value isn't cached in a register by the > > > > > * compiler and used as a stale value after we've observed a > > > > > * subsequent update. > > > > > */ > > > > > - pmde = READ_ONCE(*pvmw->pmd); > > > > > + pmde = pmd_read_atomic(pvmw->pmd); > > > > > + barrier(); > > > > > + Andrew, please drop this patch from your queue: it's harmless, but pretending to do some good when it does not. The situation has changed since I originally wrote it, gcc min version has been raised, Will Deacon has corrected the applicability of READ_ONCE() to pmd_t in April 2020 commits, pmd_read_atomic() is not as magic as I thought (it has good uses but not here), and the commit comment is no longer right. However... if you're nearing the point of a fresh mmotm, probably best to leave it in for now and we sort out the mess later. Because although it's functionally independent from the other patches in the series, there is of course the "indentation" patch, and this falls in the middle of what's indented a level there. I don't imagine that will tax your abilities to their limit, but after that there's the main bugfix patch, which expects a blank line I added in this one, that's no longer there when reverted. Enough to make me sigh, and just write to say "drop, when you have time". Let me know if you'd prefer a resend of the series (today? not sure). (Discussion of pmd_read_atomic() and barrier() and mm_find_pmd() and suchlike may continue in this thread, but this 03/11 should just be dropped. Whether it's in or out should not affect Alistair's series.) Thanks, HUgh
