On Mon, Jan 13, 2020 at 08:47:25PM +0800, Alex Shi wrote:

在 2020/1/13 下午5:55, Konstantin Khlebnikov 写道:

quoted

quoted

quoted

That's wrong. Here PageLRU must be checked again under lru_lock.

Hi, Konstantin,

For logical remain, we can get the lock and then release for !PageLRU.
but I still can figure out the problem scenario. Would like to give more hints?

That's trivial race: page could be isolated from lru between

if (PageLRU(page))
and
spin_lock_irq(&pgdat->lru_lock);

yes, it could be a problem. guess the following change could helpful:
I will update it in new version.

+       if (lrucare) {
+               lruvec = lock_page_lruvec_irq(page);
+               if (likely(PageLRU(page))) {
+                       ClearPageLRU(page);
+                       del_page_from_lru_list(page, lruvec, page_lru(page));
+               } else {
+                       unlock_page_lruvec_irq(lruvec);
+                       lruvec = NULL;
+               }

What about a harder race to hit like a page being on LRU list A when you
look up the lruvec, then it's removed and added to LRU list B by the
time you get the lock?  At that point, you are holding a lock on the
wrong LRU list.  I think you need to check not just that the page
is still PageLRU but also still on the same LRU list.