Re: [PATCH v17 01/21] mm/vmscan: remove unnecessary lruvec adding

2020-08-05 Thread Alex Shi



在 2020/7/25 下午8:59, Alex Shi 写道:
> We don't have to add a freeable page into lru and then remove from it.
> This change saves a couple of actions and makes the moving more clear.
> 
> The SetPageLRU needs to be kept here for list intergrity.
> Otherwise:
>  #0 mave_pages_to_lru  #1 release_pages
>if (put_page_testzero())
>  if !put_page_testzero
>  !PageLRU //skip lru_lock
>list_add(>lru,)
>list_add(>lru,) //corrupt

The race comments should be corrected to this:
/*
 * The SetPageLRU needs to be kept here for list intergrity.
 * Otherwise:
 *   #0 mave_pages_to_lru #1 release_pages
 *   if !put_page_testzero
 *if (put_page_testzero())
 *  !PageLRU //skip lru_lock
 * SetPageLRU()
 * list_add(>lru,)
 *list_add(>lru,)
 */

> 
> [a...@linux-foundation.org: coding style fixes]
> Signed-off-by: Alex Shi 
> Cc: Andrew Morton 
> Cc: Johannes Weiner 
> Cc: Tejun Heo 
> Cc: Matthew Wilcox 
> Cc: Hugh Dickins 
> Cc: linux...@kvack.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  mm/vmscan.c | 37 -
>  1 file changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 749d239c62b2..ddb29d813d77 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1856,26 +1856,29 @@ static unsigned noinline_for_stack 
> move_pages_to_lru(struct lruvec *lruvec,
>   while (!list_empty(list)) {
>   page = lru_to_page(list);
>   VM_BUG_ON_PAGE(PageLRU(page), page);
> + list_del(>lru);
>   if (unlikely(!page_evictable(page))) {
> - list_del(>lru);
>   spin_unlock_irq(>lru_lock);
>   putback_lru_page(page);
>   spin_lock_irq(>lru_lock);
>   continue;
>   }
> - lruvec = mem_cgroup_page_lruvec(page, pgdat);
>  
> + /*
> +  * The SetPageLRU needs to be kept here for list intergrity.
> +  * Otherwise:
> +  *   #0 mave_pages_to_lru #1 release_pages
> +  *if (put_page_testzero())
> +  *   if !put_page_testzero
> +  *  !PageLRU //skip lru_lock
> +  *list_add(>lru,)
> +  * list_add(>lru,) //corrupt
> +  */

/*
 * The SetPageLRU needs to be kept here for list intergrity.
 * Otherwise:
 *   #0 mave_pages_to_lru #1 release_pages
 *   if !put_page_testzero
 *if (put_page_testzero())
 *  !PageLRU //skip lru_lock
 * SetPageLRU()
 * list_add(>lru,)
 *list_add(>lru,)
 */

>   SetPageLRU(page);
> - lru = page_lru(page);
>  
> - nr_pages = hpage_nr_pages(page);
> - update_lru_size(lruvec, lru, page_zonenum(page), nr_pages);
> - list_move(>lru, >lists[lru]);
> -
> - if (put_page_testzero(page)) {
> + if (unlikely(put_page_testzero(page))) {
>   __ClearPageLRU(page);
>   __ClearPageActive(page);
> - del_page_from_lru_list(page, lruvec, lru);
>  
>   if (unlikely(PageCompound(page))) {
>   spin_unlock_irq(>lru_lock);
> @@ -1883,11 +1886,19 @@ static unsigned noinline_for_stack 
> move_pages_to_lru(struct lruvec *lruvec,
>   spin_lock_irq(>lru_lock);
>   } else
>   list_add(>lru, _to_free);
> - } else {
> - nr_moved += nr_pages;
> - if (PageActive(page))
> - workingset_age_nonresident(lruvec, nr_pages);
> +
> + continue;
>   }
> +
> + lruvec = mem_cgroup_page_lruvec(page, pgdat);
> + lru = page_lru(page);
> + nr_pages = hpage_nr_pages(page);
> +
> + update_lru_size(lruvec, lru, page_zonenum(page), nr_pages);
> + list_add(>lru, >lists[lru]);
> + nr_moved += nr_pages;
> + if (PageActive(page))
> + workingset_age_nonresident(lruvec, nr_pages);
>   }
>  
>   /*
> 


[PATCH v17 01/21] mm/vmscan: remove unnecessary lruvec adding

2020-07-25 Thread Alex Shi
We don't have to add a freeable page into lru and then remove from it.
This change saves a couple of actions and makes the moving more clear.

The SetPageLRU needs to be kept here for list intergrity.
Otherwise:
 #0 mave_pages_to_lru  #1 release_pages
   if (put_page_testzero())
 if !put_page_testzero
 !PageLRU //skip lru_lock
   list_add(>lru,)
   list_add(>lru,) //corrupt

[a...@linux-foundation.org: coding style fixes]
Signed-off-by: Alex Shi 
Cc: Andrew Morton 
Cc: Johannes Weiner 
Cc: Tejun Heo 
Cc: Matthew Wilcox 
Cc: Hugh Dickins 
Cc: linux...@kvack.org
Cc: linux-kernel@vger.kernel.org
---
 mm/vmscan.c | 37 -
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 749d239c62b2..ddb29d813d77 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1856,26 +1856,29 @@ static unsigned noinline_for_stack 
move_pages_to_lru(struct lruvec *lruvec,
while (!list_empty(list)) {
page = lru_to_page(list);
VM_BUG_ON_PAGE(PageLRU(page), page);
+   list_del(>lru);
if (unlikely(!page_evictable(page))) {
-   list_del(>lru);
spin_unlock_irq(>lru_lock);
putback_lru_page(page);
spin_lock_irq(>lru_lock);
continue;
}
-   lruvec = mem_cgroup_page_lruvec(page, pgdat);
 
+   /*
+* The SetPageLRU needs to be kept here for list intergrity.
+* Otherwise:
+*   #0 mave_pages_to_lru #1 release_pages
+*if (put_page_testzero())
+*   if !put_page_testzero
+*  !PageLRU //skip lru_lock
+*list_add(>lru,)
+* list_add(>lru,) //corrupt
+*/
SetPageLRU(page);
-   lru = page_lru(page);
 
-   nr_pages = hpage_nr_pages(page);
-   update_lru_size(lruvec, lru, page_zonenum(page), nr_pages);
-   list_move(>lru, >lists[lru]);
-
-   if (put_page_testzero(page)) {
+   if (unlikely(put_page_testzero(page))) {
__ClearPageLRU(page);
__ClearPageActive(page);
-   del_page_from_lru_list(page, lruvec, lru);
 
if (unlikely(PageCompound(page))) {
spin_unlock_irq(>lru_lock);
@@ -1883,11 +1886,19 @@ static unsigned noinline_for_stack 
move_pages_to_lru(struct lruvec *lruvec,
spin_lock_irq(>lru_lock);
} else
list_add(>lru, _to_free);
-   } else {
-   nr_moved += nr_pages;
-   if (PageActive(page))
-   workingset_age_nonresident(lruvec, nr_pages);
+
+   continue;
}
+
+   lruvec = mem_cgroup_page_lruvec(page, pgdat);
+   lru = page_lru(page);
+   nr_pages = hpage_nr_pages(page);
+
+   update_lru_size(lruvec, lru, page_zonenum(page), nr_pages);
+   list_add(>lru, >lists[lru]);
+   nr_moved += nr_pages;
+   if (PageActive(page))
+   workingset_age_nonresident(lruvec, nr_pages);
}
 
/*
-- 
1.8.3.1