When hit pmd_trans_unstable() under mmap read lock, it means we raced with something else. Per the comment above the helper, we can definitely treat it as some pmd (none?) but the 100% correct way is always retry, and I don't think it should race again in most cases. Not taking care of that retry can mean different things on different paths. For example, for smaps it means inaccurate accountings when we skip those raced regions, but it's fine anyway because the accounting is not for 100% accurate. I think it's broken for pagemap OTOH, because we have the pagemap buffer linear to the VA we're scanning, it means if we skip some region the follow up scans can fill in the wrong slots, I think. It means the pagemap results returned to userapp will be wrong when very unlucky. This reminded me that I should have a look at all call sites of pmd_trans_unstable(), some of them are alright but I do see many of them may still be better to give another shot when hit. This series tries to resolve all call sites for it on that retry attempt. I really don't know whether I missed something, even if not, whether it matters a lot to anyone. Still, _if_ I'm correct may worth consider fixing. Happy to be prove wrong. Then Muhammad should know how to code his. The patchset is only smoke tested, nothing wrong I see. Please have a look, thanks. Peter Xu (4): mm/mprotect: Retry on pmd_trans_unstable() mm/migrate: Unify and retry an unstable pmd when hit mm: Warn for unstable pmd in move_page_tables() mm: Make most walk page paths with pmd_trans_unstable() to retry fs/proc/task_mmu.c | 17 +++++++++++++---- mm/madvise.c | 8 ++++++-- mm/memcontrol.c | 8 ++++++-- mm/memory-failure.c | 4 +++- mm/mempolicy.c | 4 +++- mm/migrate_device.c | 9 ++++----- mm/mprotect.c | 20 +++++++++++--------- mm/mremap.c | 4 ++-- 8 files changed, 48 insertions(+), 26 deletions(-) -- 2.40.1
