| Message ID | 20240327141034.3712697-5-wangkefeng.wang@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: remove isolate_lru_page() and isolate_movable_page() | expand |
On Wed, Mar 27, 2024 at 10:10:32PM +0800, Kefeng Wang wrote: > This moves folio_get_nontail_page() before non-lru movable pages check, > and directly call isolate_movable_folio() to save compound_head() calls, > since the reference count of the non-lru movable page is increased, a > folio_put() is need() whether the folio is isolated or not. > > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> > --- > mm/compaction.c | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/mm/compaction.c b/mm/compaction.c > index 807b58e6eb68..74ac65daaed1 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -1097,41 +1097,41 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, > } > } > > + /* > + * 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; > + If you wanted to move this, I think this should be part of your first patch (or prior to it). It would make your first patch be more sensible as is. You could then also consider making isolate_movable_folio() more similar to folio_isolate_lru() if you really wanted to. > /* > * 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 > */ > - if (!PageLRU(page)) { > + if (!folio_test_lru(folio)) { > /* > * __PageMovable 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_folio(folio, 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 > >
On 2024/3/28 2:49, Vishal Moola wrote: > On Wed, Mar 27, 2024 at 10:10:32PM +0800, Kefeng Wang wrote: >> This moves folio_get_nontail_page() before non-lru movable pages check, >> and directly call isolate_movable_folio() to save compound_head() calls, >> since the reference count of the non-lru movable page is increased, a >> folio_put() is need() whether the folio is isolated or not. >> >> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> >> --- >> mm/compaction.c | 30 +++++++++++++++--------------- >> 1 file changed, 15 insertions(+), 15 deletions(-) >> >> diff --git a/mm/compaction.c b/mm/compaction.c >> index 807b58e6eb68..74ac65daaed1 100644 >> --- a/mm/compaction.c >> +++ b/mm/compaction.c >> @@ -1097,41 +1097,41 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, >> } >> } >> >> + /* >> + * 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; >> + > > If you wanted to move this, I think this should be part of your first > patch (or prior to it). It would make your first patch be more sensible as ok, will re-order the patches. > is. You could then also consider making isolate_movable_folio() more similar > to folio_isolate_lru() if you really wanted to. Maybe just rename it folio_isolate_movable and no more changes now. Thanks. > >> /* >> * 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 >> */ >> - if (!PageLRU(page)) { >> + if (!folio_test_lru(folio)) { >> /* >> * __PageMovable 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_folio(folio, 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 >> >>
diff --git a/mm/compaction.c b/mm/compaction.c index 807b58e6eb68..74ac65daaed1 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -1097,41 +1097,41 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, } } + /* + * 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; + /* * 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 */ - if (!PageLRU(page)) { + if (!folio_test_lru(folio)) { /* * __PageMovable 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_folio(folio, 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
This moves folio_get_nontail_page() before non-lru movable pages check, and directly call isolate_movable_folio() to save compound_head() calls, since the reference count of the non-lru movable page is increased, a folio_put() is need() whether the folio is isolated or not. Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- mm/compaction.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)