| Message ID | 1608707589-6067-1-git-send-email-yanghui.def@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm:remove useless code | expand |
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 > >
On Wed, Dec 23, 2020 at 12:43 PM hui yang <yanghui.def@gmail.com> wrote: > > From: YangHui <yanghui.def@gmail.com> > > remove useless code Change logs need to be more descriptive. > > 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)) { This will change the original behaviour of the code. > 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 > >
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)) { 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))