From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 67806C10DCE for ; Thu, 12 Mar 2020 15:14:28 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 231D920674 for ; Thu, 12 Mar 2020 15:14:28 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=cmpxchg-org.20150623.gappssmtp.com header.i=@cmpxchg-org.20150623.gappssmtp.com header.b="XZbNSmnY" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 231D920674 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=cmpxchg.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id C26096B0006; Thu, 12 Mar 2020 11:14:27 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BD71A6B0007; Thu, 12 Mar 2020 11:14:27 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AEC3C6B0008; Thu, 12 Mar 2020 11:14:27 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0058.hostedemail.com [216.40.44.58]) by kanga.kvack.org (Postfix) with ESMTP id 97DB06B0006 for ; Thu, 12 Mar 2020 11:14:27 -0400 (EDT) Received: from smtpin08.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 5969E181AEF00 for ; Thu, 12 Mar 2020 15:14:27 +0000 (UTC) X-FDA: 76587056574.08.brake66_9ae14304ea30 X-HE-Tag: brake66_9ae14304ea30 X-Filterd-Recvd-Size: 6112 Received: from mail-qk1-f194.google.com (mail-qk1-f194.google.com [209.85.222.194]) by imf45.hostedemail.com (Postfix) with ESMTP for ; Thu, 12 Mar 2020 15:14:26 +0000 (UTC) Received: by mail-qk1-f194.google.com with SMTP id u25so6778474qkk.3 for ; Thu, 12 Mar 2020 08:14:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=2UkcErBA53F3BNjpYaCxrTKEMvELmNBoZLVc53UTNuI=; b=XZbNSmnYbFATBaIbpTUipLV83lZIYT3fG6Lz8aAQdvdznkQDhdjG+RaQZWCjN3owOo hO1fMsqQrMw5i7+h6CpTEUzGA3Z4+DExyBSGEy194RLf6DIxLyBMNkTvAzMxkVV04sRq eoqa6wP3Iwq1urq33qErLwSuawBElpP/xXauoUTAksGcIE+FXUYIEKCPf8Xbn+nGyaK8 CU8EDjc+ZuXcxLMyZn4d9mYORoq3H3S0z0HAjZayMJCdknd0QOj38zL6fp4/uykf5V6q m8mlHJH41SKwZGMBxBmViR49ndlQ8YKiwFlDqMdWYFIsfBzjo7UsRAn8YSi1Ulh6cP8Q AcCA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=2UkcErBA53F3BNjpYaCxrTKEMvELmNBoZLVc53UTNuI=; b=qonzz6bb3r9iY9mH4C3XFaglXWhp6VkieHbAu9EwSPosXmKEwhIc98I4HBgLdsN6hV QZY+NZNgmEJsDmDYQVYOFulwH4+rWHgNpIINDiaGY1UD8Rwlu0mBeAI1ZyDWs4cwsTbx 3qIH+0ghqy6ioygUa9FKT2L6Q1pVTv490flfgkNbtI30sPeubVbx7ALq7evo6jJcdsVe cxe1liGTTze8N4H6pYRH72SUYRaodoh4SWUoa2d9sqGMWf4QOTyikk/Bygzqk+wwo8YJ xwhGsp5+Rkp6KSFGxGJVTH9pbdwBmWrbgzTz2yqF2Azq5P+ESaEdAlGwstZtfkTBR5yy Uxmg== X-Gm-Message-State: ANhLgQ1rb5DKVj9/sK9JUZ3qZA8JGRJFTHVZwtf6MfXKbNyEw+979maU Q9utOEycYFJTLlZOQq49SuuruA== X-Google-Smtp-Source: ADFU+vtLoe18dnuHtbiTmsGIJmCiYFC7FTXBUsqpOvPlnhGUpTSjH4ukDMQ8Cj7sng4y+MmYN3cyxQ== X-Received: by 2002:a37:2fc3:: with SMTP id v186mr8250896qkh.311.1584026065613; Thu, 12 Mar 2020 08:14:25 -0700 (PDT) Received: from localhost ([2620:10d:c091:480::ef6a]) by smtp.gmail.com with ESMTPSA id y5sm10401364qkb.123.2020.03.12.08.14.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 Mar 2020 08:14:24 -0700 (PDT) Date: Thu, 12 Mar 2020 11:14:23 -0400 From: Johannes Weiner To: js1304@gmail.com Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Michal Hocko , Hugh Dickins , Minchan Kim , Vlastimil Babka , Mel Gorman , kernel-team@lge.com, Joonsoo Kim Subject: Re: [PATCH v2 2/9] mm/vmscan: protect the workingset on anonymous LRU Message-ID: <20200312151423.GH29835@cmpxchg.org> References: <1582175513-22601-1-git-send-email-iamjoonsoo.kim@lge.com> <1582175513-22601-3-git-send-email-iamjoonsoo.kim@lge.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1582175513-22601-3-git-send-email-iamjoonsoo.kim@lge.com> X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Feb 20, 2020 at 02:11:46PM +0900, js1304@gmail.com wrote: > @@ -1010,8 +1010,15 @@ static enum page_references page_check_references(struct page *page, > return PAGEREF_RECLAIM; > > if (referenced_ptes) { > - if (PageSwapBacked(page)) > - return PAGEREF_ACTIVATE; > + if (PageSwapBacked(page)) { > + if (referenced_page) { > + ClearPageReferenced(page); > + return PAGEREF_ACTIVATE; > + } This looks odd to me. referenced_page = TestClearPageReferenced() above, so it's already be clear. Why clear it again? > + > + SetPageReferenced(page); > + return PAGEREF_KEEP; > + } The existing file code already does: SetPageReferenced(page); if (referenced_page || referenced_ptes > 1) return PAGEREF_ACTIVATE; if (vm_flags & VM_EXEC) return PAGEREF_ACTIVATE; return PAGEREF_KEEP; The differences are: 1) referenced_ptes > 1. We did this so that heavily shared file mappings are protected a bit better than others. Arguably the same could apply for anon pages when we put them on the inactive list. 2) vm_flags & VM_EXEC. This mostly doesn't apply to anon pages. The exception would be jit code pages, but if we put anon pages on the inactive list we should protect jit code the same way we protect file executables. Seems to me you don't need to add anything. Just remove the PageSwapBacked branch and apply equal treatment to both types. > @@ -2056,6 +2063,15 @@ static void shrink_active_list(unsigned long nr_to_scan, > } > } > > + /* > + * Now, newly created anonymous page isn't appened to the > + * active list. We don't need to clear the reference bit here. > + */ > + if (PageSwapBacked(page)) { > + ClearPageReferenced(page); > + goto deactivate; > + } I don't understand this. If you don't clear the pte references, you're leaving behind stale data. You already decide here that we consider the page referenced when it reaches the end of the inactive list, regardless of what happens in between. That makes the deactivation kind of useless. And it blurs the lines between the inactive and active list. shrink_page_list() (and page_check_references()) are written with the notion that any references they look at are from the inactive list. If you carry over stale data, this can cause more subtle bugs later on. And again, I don't quite understand why anon would need different treatment here than file. > + > if (page_referenced(page, 0, sc->target_mem_cgroup, > &vm_flags)) { > nr_rotated += hpage_nr_pages(page); > @@ -2074,6 +2090,7 @@ static void shrink_active_list(unsigned long nr_to_scan, > } > } > > +deactivate: > ClearPageActive(page); /* we are de-activating */ > SetPageWorkingset(page); > list_add(&page->lru, &l_inactive); > -- > 2.7.4 >