在 2020/8/13 下午12:02, Alexander Duyck 写道:
> From: Alexander Duyck <alexander.h.duyck@xxxxxxxxxxxxxxx>
>
> The only user of test_and_set_skip was isolate_migratepages_block and it
> was using it after a call that was testing and clearing the LRU flag. As
> such it really didn't need to be behind the LRU lock anymore as it wasn't
> really fulfilling its purpose.
>
> With that being the case we can simply drop the bit and instead directly
> just call the set_pageblock_skip function if the page we are working on is
> the valid_page at the start of the pageblock. It shouldn't be possible for
> us to encounter the bit being set since we obtained the LRU flag for the
> first page in the pageblock which means we would have exclusive access to
> setting the skip bit. As such we don't need to worry about the abort case
> since no other thread will be able to call what used to be
> test_and_set_skip.
>
> Since we have dropped the late abort case we can drop the code that was
> clearing the LRU flag and calling page_put since the abort case will now
> not be holding a reference to a page.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@xxxxxxxxxxxxxxx>
After my false sharing remove on pageblock_flags, this patch looks fine with
a minor change
> ---
> mm/compaction.c | 50 +++++++-------------------------------------------
> 1 file changed, 7 insertions(+), 43 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 5021a18ef722..c1e9918f9dd4 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -399,29 +399,6 @@ void reset_isolation_suitable(pg_data_t *pgdat)
> }
> }
>
> -/*
> - * Sets the pageblock skip bit if it was clear. Note that this is a hint as
> - * locks are not required for read/writers. Returns true if it was already set.
> - */
> -static bool test_and_set_skip(struct compact_control *cc, struct page *page,
> - unsigned long pfn)
> -{
> - bool skip;
> -
> - /* Do no update if skip hint is being ignored */
> - if (cc->ignore_skip_hint)
> - return false;
> -
> - if (!IS_ALIGNED(pfn, pageblock_nr_pages))
> - return false;
> -
> - skip = get_pageblock_skip(page);
> - if (!skip && !cc->no_set_skip_hint)
> - skip = !set_pageblock_skip(page);
> -
> - return skip;
> -}
> -
> static void update_cached_migrate(struct compact_control *cc, unsigned long pfn)
> {
> struct zone *zone = cc->zone;
> @@ -480,12 +457,6 @@ static inline void update_pageblock_skip(struct compact_control *cc,
> static void update_cached_migrate(struct compact_control *cc, unsigned long pfn)
> {
> }
> -
> -static bool test_and_set_skip(struct compact_control *cc, struct page *page,
> - unsigned long pfn)
> -{
> - return false;
> -}
> #endif /* CONFIG_COMPACTION */
>
> /*
> @@ -895,7 +866,6 @@ static bool too_many_isolated(pg_data_t *pgdat)
> if (!valid_page && IS_ALIGNED(low_pfn, pageblock_nr_pages)) {
> if (!cc->ignore_skip_hint && get_pageblock_skip(page)) {
> low_pfn = end_pfn;
> - page = NULL;
> goto isolate_abort;
> }
> valid_page = page;
> @@ -991,6 +961,13 @@ static bool too_many_isolated(pg_data_t *pgdat)
> if (!TestClearPageLRU(page))
> goto isolate_fail_put;
>
> + /* Indicate that we want exclusive access to this pageblock */
> + if (page == valid_page) {
> + skip_updated = true;
> + if (!cc->ignore_skip_hint)
if (!cc->ignore_skip_hint && !cc->no_set_skip_hint)
no_set_skip_hint needs to add here.
Thanks
Alex
> + set_pageblock_skip(page);
> + }
> +
> /* If we already hold the lock, we can skip some rechecking */
> if (!lruvec || !lruvec_holds_page_lru_lock(page, lruvec)) {
> if (lruvec)
> @@ -1002,13 +979,6 @@ static bool too_many_isolated(pg_data_t *pgdat)
>
> lruvec_memcg_debug(lruvec, page);
>
> - /* Try get exclusive access under lock */
> - if (!skip_updated) {
> - skip_updated = true;
> - if (test_and_set_skip(cc, page, low_pfn))
> - goto isolate_abort;
> - }
> -
> /*
> * Page become compound since the non-locked check,
> * and it's on LRU. It can only be a THP so the order
> @@ -1094,15 +1064,9 @@ static bool too_many_isolated(pg_data_t *pgdat)
> if (unlikely(low_pfn > end_pfn))
> low_pfn = end_pfn;
>
> - page = NULL;
> -
> isolate_abort:
> if (lruvec)
> unlock_page_lruvec_irqrestore(lruvec, flags);
> - if (page) {
> - SetPageLRU(page);
> - put_page(page);
> - }
>
> /*
> * Updated the cached scanner pfn once the pageblock has been scanned
>
