On Thu, Aug 29, 2024 at 10:54:52PM +0800, Kefeng Wang wrote:
> Non-LRU movable folio isolation will fail if it can't grab a reference
> in isolate_movable_page(), so folio_get_nontail_page() could be called
> ahead to unify the handling of non-LRU movable/LRU folio isolation a bit,
> this is also prepare to convert isolate_movable_page() to take a folio.
> Since the reference count of the non-LRU movable folio is increased,
> a folio_put() is needed whether the folio is isolated or not.
There's a reason I stopped where I did when converting this function
to use folios. Usually I would explain, but I think it would do you
good to think about why for a bit.
Andrew, please drop these patches.
> Signed-off-by: Kefeng Wang <wangkefeng.wang@xxxxxxxxxx>
> ---
> mm/compaction.c | 38 +++++++++++++++++++-------------------
> 1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index a2b16b08cbbf..8998574da065 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -1074,41 +1074,41 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
> }
> }
>
> + /*
> + * Be careful not to clear lru flag until after we're
> + * sure the folio is not being freed elsewhere -- the
> + * folio release code relies on it.
> + */
> + folio = folio_get_nontail_page(page);
> + if (unlikely(!folio))
> + goto isolate_fail;
> +
> /*
> * Check may be lockless but that's ok as we recheck later.
> - * It's possible to migrate LRU and non-lru movable pages.
> - * Skip any other type of page
> + * It's possible to migrate LRU and non-lru movable folioss.
> + * Skip any other type of folios.
> */
> - if (!PageLRU(page)) {
> + if (!folio_test_lru(folio)) {
> /*
> - * __PageMovable can return false positive so we need
> - * to verify it under page_lock.
> + * __folio_test_movable can return false positive so
> + * we need to verify it under page_lock.
> */
> - if (unlikely(__PageMovable(page)) &&
> - !PageIsolated(page)) {
> + if (unlikely(__folio_test_movable(folio)) &&
> + !folio_test_isolated(folio)) {
> if (locked) {
> unlock_page_lruvec_irqrestore(locked, flags);
> locked = NULL;
> }
>
> - if (isolate_movable_page(page, mode)) {
> - folio = page_folio(page);
> + if (isolate_movable_page(&folio->page, mode)) {
> + folio_put(folio);
> goto isolate_success;
> }
> }
>
> - goto isolate_fail;
> + goto isolate_fail_put;
> }
>
> - /*
> - * Be careful not to clear PageLRU until after we're
> - * sure the page is not being freed elsewhere -- the
> - * page release code relies on it.
> - */
> - folio = folio_get_nontail_page(page);
> - if (unlikely(!folio))
> - goto isolate_fail;
> -
> /*
> * Migration will fail if an anonymous page is pinned in memory,
> * so avoid taking lru_lock and isolating it unnecessarily in an
> --
> 2.27.0
>
