在 2020/7/18 上午6:03, Alexander Duyck 写道:

quoted

index 129c532357a4..9fb906fbaed5 100644

--- a/mm/swap.c
+++ b/mm/swap.c

@@ -209,19 +209,12 @@ static void pagevec_lru_move_fn(struct pagevec *pvec,

        for (i = 0; i < pagevec_count(pvec); i++) {
                struct page *page = pvec->pages[i];
-               struct lruvec *new_lruvec;
-
-               new_lruvec = mem_cgroup_page_lruvec(page, page_pgdat(page));
-               if (lruvec != new_lruvec) {
-                       if (lruvec)
-                               unlock_page_lruvec_irqrestore(lruvec, flags);
-                       lruvec = lock_page_lruvec_irqsave(page, &flags);
-               }

                /* block memcg migration during page moving between lru */
                if (!TestClearPageLRU(page))
                        continue;

+               lruvec = relock_page_lruvec_irqsave(page, lruvec, &flags);
                (*move_fn)(page, lruvec);

                SetPageLRU(page);

So looking at this I realize that patch 18 probably should have
ordered this the same way with the TestClearPageLRU happening before
you fetched the new_lruvec. Otherwise I think you are potentially
exposed to the original issue you were fixing the the previous patch
that added the call to TestClearPageLRU.

Good catch. It's better to be aligned in next version.
Thanks!

quoted

@@ -866,17 +859,12 @@ void release_pages(struct page **pages, int nr)
                }

                if (PageLRU(page)) {
-                       struct lruvec *new_lruvec;
-
-                       new_lruvec = mem_cgroup_page_lruvec(page,
-                                                       page_pgdat(page));
-                       if (new_lruvec != lruvec) {
-                               if (lruvec)
-                                       unlock_page_lruvec_irqrestore(lruvec,
-                                                                       flags);
+                       struct lruvec *pre_lruvec = lruvec;
+
+                       lruvec = relock_page_lruvec_irqsave(page, lruvec,
+                                                                       &flags);
+                       if (pre_lruvec != lruvec)

So this doesn't really read right. I suppose "pre_lruvec" should
probably be "prev_lruvec" since I assume you mean "previous" not
"before".

yes, it's previous, I will rename it.
Thanks
Alex