Currently when we try to do page migration and we're in "synchronous" mode (and not doing direct compaction) then we'll wait an infinite amount of time for a page lock. This does not appear to be a great idea. One issue can be seen when I put a device under extreme memory pressure. I took a sc7180-trogdor Chromebook (4GB RAM, 8GB zram swap). I ran the browser along with Android (which runs from a loopback mounted 128K block-size squashfs "disk"). I then manually ran the mmm_donut memory pressure tool [1]. The system is completely unusable both with and without this patch since there are 8 processes completely thrashing memory, but it was still interesting to look at how migration was behaving. I put some timing code in and I could see that we sometimes waited over 25 seconds (in the context of kcompactd0) for a page lock to become available. Although the 25 seconds was the high mark, it was easy to see tens, hundreds, or thousands of milliseconds spent waiting on the lock. Instead of waiting, if I bailed out right away (as this patch does), I could see kcompactd0 move forward to successfully to migrate other pages instead. This seems like a better use of kcompactd's time. Thus, even though this didn't make the system any more usable in my absurd test case, it still seemed to make migration behave better and that feels like a win. It also makes the code simpler since we have one fewer special case. The second issue that this patch attempts to fix is one that I haven't managed to reproduce yet. We have crash reports from the field that report that kcompactd0 was blocked for more than ~120 seconds on this exact lock. These crash reports are on devices running older kernels (5.10 mostly). In most of these crash reports the device is under memory pressure and many tasks were blocked in squashfs code, ext4 code, or memory allocation code. While I don't know if unblocking kcompactd would actually have helped relieve the memory pressure, at least there was a chance that it could have helped a little bit. [1] https://chromium.googlesource.com/chromiumos/platform/microbenchmarks/+/refs/heads/main/mmm_donut.py Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx> --- mm/migrate.c | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/mm/migrate.c b/mm/migrate.c index db3f154446af..dfb0a6944181 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -1143,26 +1143,15 @@ static int migrate_folio_unmap(new_page_t get_new_page, free_page_t put_new_page dst->private = NULL; if (!folio_trylock(src)) { - if (mode == MIGRATE_ASYNC) - goto out; - /* - * It's not safe for direct compaction to call lock_page. - * For example, during page readahead pages are added locked - * to the LRU. Later, when the IO completes the pages are - * marked uptodate and unlocked. However, the queueing - * could be merging multiple pages for one bio (e.g. - * mpage_readahead). If an allocation happens for the - * second or third page, the process can end up locking - * the same page twice and deadlocking. Rather than - * trying to be clever about what pages can be locked, - * avoid the use of lock_page for direct compaction - * altogether. + * While there are some modes we could be running in where we + * could block here waiting for the lock (specifically + * modes other than MIGRATE_ASYNC and when we're not in + * direct compaction), it's not worth the wait. Instead of + * waiting, we'll bail. This will let the caller try to + * migrate some other pages that aren't contended. */ - if (current->flags & PF_MEMALLOC) - goto out; - - folio_lock(src); + goto out; } locked = true; -- 2.40.0.634.g4ca3ef3211-goog
