在 2020/8/13 下午12:02, Alexander Duyck 写道:
> From: Alexander Duyck <[email protected]>
> 
> 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 <[email protected]>

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
> 

Reply via email to