Re: [PATCH] mm:remove useless code

[Hao Lee <haolee.swjtu@gmail.com>]

On Wed, Dec 23, 2020 at 3:13 PM hui yang <yanghui.def@gmail.com> wrote:
>
> From: YangHui <yanghui.def@gmail.com>
>
> remove useless code
>
> Signed-off-by: YangHui <yanghui.def@gmail.com>
> ---
>  mm/vmscan.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 257cba7..af1d176 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1234,8 +1234,8 @@ static unsigned int shrink_page_list(struct list_head *page_list,
>                  * Try to allocate it some swap space here.
>                  * Lazyfree page could be freed directly
>                  */
> -               if (PageAnon(page) && PageSwapBacked(page)) {
> -                       if (!PageSwapCache(page)) {
> +               if (PageAnon(page) && PageSwapBacked(page) &&
> +                       !PageSwapCache(page)) {

I think this is incorrect.

Consider this scenario:

PageAnon(page) && PageSwapBacked(page) is true
!PageSwapCache(page) is false

In original logic, this snippet will run into the first if branch which
does nothing actually. However, in your logic, this snippet will run
into else if branch.

Regards,
Hao Lee

>                                 if (!(sc->gfp_mask & __GFP_IO))
>                                         goto keep_locked;
>                                 if (PageTransHuge(page)) {
> @@ -1270,7 +1270,6 @@ static unsigned int shrink_page_list(struct list_head *page_list,
>
>                                 /* Adding to swap updated mapping */
>                                 mapping = page_mapping(page);
> -                       }
>                 } else if (unlikely(PageTransHuge(page))) {
>                         /* Split file THP */
>                         if (split_huge_page_to_list(page, page_list))
> --
> 2.7.4
>
>