On 7/1/20 1:47 AM, Greg Thelen wrote: > Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx> wrote: >> From: Keith Busch <kbusch@xxxxxxxxxx> >> Defer allocating the page until we are actually ready to make use of >> it, after locking the original page. This simplifies error handling, >> but should not have any functional change in behavior. This is just >> refactoring page migration so the main part can more easily be reused >> by other code. > > Is there any concern that the src page is now held PG_locked over the > dst page allocation, which might wander into > reclaim/cond_resched/oom_kill? I don't have a deadlock in mind. I'm > just wondering about the additional latency imposed on unrelated threads > who want access src page. It's not great. *But*, the alternative is to toss the page contents out and let users encounter a fault and an allocation. They would be subject to all the latency associated with an allocation, just at a slightly later time. If it's a problem it seems like it would be pretty easy to fix, at least for non-cgroup reclaim. We know which node we're reclaiming from and we know if it has a demotion path, so we could proactively allocate a single migration target page before doing the source lock_page(). That creates some other problems, but I think it would be straightforward. >> #Signed-off-by: Keith Busch <keith.busch@xxxxxxxxx> > > Is commented Signed-off-by intentional? Same applies to later patches. Yes, Keith is no longer at Intel, so that @intel.com mail would bounce. I left the @intel.com SoB so it would be clear that the code originated from Keith while at Intel, but commented it out to avoid it being picked up by anyone's tooling.