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=-6.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=ham 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 E216DC10DCE for ; Wed, 18 Mar 2020 18:06:43 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 93BCD20753 for ; Wed, 18 Mar 2020 18:06:43 +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="nlKZMCT6" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 93BCD20753 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 25BD96B0071; Wed, 18 Mar 2020 14:06:43 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 20DBF6B0073; Wed, 18 Mar 2020 14:06:43 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0FCBB6B0074; Wed, 18 Mar 2020 14:06:43 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0098.hostedemail.com [216.40.44.98]) by kanga.kvack.org (Postfix) with ESMTP id ECCEB6B0071 for ; Wed, 18 Mar 2020 14:06:42 -0400 (EDT) Received: from smtpin24.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id A4BF44995F9 for ; Wed, 18 Mar 2020 18:06:42 +0000 (UTC) X-FDA: 76609263444.24.base44_74b39c73b4d2f X-HE-Tag: base44_74b39c73b4d2f X-Filterd-Recvd-Size: 12781 Received: from mail-qv1-f65.google.com (mail-qv1-f65.google.com [209.85.219.65]) by imf31.hostedemail.com (Postfix) with ESMTP for ; Wed, 18 Mar 2020 18:06:41 +0000 (UTC) Received: by mail-qv1-f65.google.com with SMTP id q73so3987595qvq.2 for ; Wed, 18 Mar 2020 11:06:41 -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=NJcNnn0o+YSBpq6FW4tk05aWelvsn8Udpr0Qn8KERXk=; b=nlKZMCT6g0xDk9hquAeci9BSbdC/XBlqPwmdhszHMtoT4WMadC+e9ZztEPrt1hcFFG GX1dzNeXhp7/cd6msTF0Y7Zbl1DNx6ONGSvr/m86+OYeaGCXZ0hjC8G4Tbn4zQhtP9Lr VFMEzZoqUrjtDM9RcNGwuJpj3Iu+e538ZvteSSUyIa3sJgEuf5Vr4+IfbX/8qFMzSG8y febMBFCPKoNc6R8AyMkbHT4e0xTruatgp9QU/78AI0kLLRXbCW4MFhtGA7D+M6Nh8y0n TrH6jYoc9t9GjjDt/zsfvKRbnVU6abnWbBP1QA8fdZ0LaLzYK7IPE3ry800NqvRXOFv5 kTjw== 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=NJcNnn0o+YSBpq6FW4tk05aWelvsn8Udpr0Qn8KERXk=; b=JBswSoMBm1QZGpIbj1E9HPqNyvLQVWjcYf5i6BA3RIMoDoM2JEiTKaJ7nAARiZNEuD TlE+etTEN0Db4jBeiidujzKmdIxml+mSjyOq/60FjIcgA0ShMalyUk8IjCJuJwI5bRxt 05ir0HXf6379iey2V9os0zFT3z9sqJ/p0JJoXcebpbmMNkqE/kluEpy8kL4sQiZ+h3kJ AAUvyUGVkaCy35zt2B5IxE7JDgpOxX5jFCpkY8TTp1nu/t53r7yTgiRqRpnlCNFZnAQK Qg0ZRjHxtDgmYcSk3DV/mBHYrtHLAK/3HO20EpscnR7YZxmsHFDPUUMVvxlOufM8q0+V jQdQ== X-Gm-Message-State: ANhLgQ3KXgG4bLm+83olmzwr0V6JM3YiD02kyjE0yfVbxWVa03StSr1O n1z84sf7SBDKr8rVqYSOY4+aGA== X-Google-Smtp-Source: ADFU+vuSWzLvY/WI+ENONeiPUhBIwOQyGC5JFzs4GBMdXVgaoy6XOkD+aT+lpAK2j+JKYvFkRbgxnQ== X-Received: by 2002:a05:6214:1703:: with SMTP id db3mr5732562qvb.28.1584554801000; Wed, 18 Mar 2020 11:06:41 -0700 (PDT) Received: from localhost ([2620:10d:c091:480::1:a9a9]) by smtp.gmail.com with ESMTPSA id 128sm4643598qki.103.2020.03.18.11.06.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 18 Mar 2020 11:06:40 -0700 (PDT) Date: Wed, 18 Mar 2020 14:06:38 -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 v3 3/9] mm/workingset: extend the workingset detection for anon LRU Message-ID: <20200318180638.GC154135@cmpxchg.org> References: <1584423717-3440-1-git-send-email-iamjoonsoo.kim@lge.com> <1584423717-3440-4-git-send-email-iamjoonsoo.kim@lge.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1584423717-3440-4-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 Tue, Mar 17, 2020 at 02:41:51PM +0900, js1304@gmail.com wrote: > From: Joonsoo Kim > > In the following patch, workingset detection will be applied to > anonymous LRU. To prepare it, this patch adds some code to > distinguish/handle the both LRUs. > > Signed-off-by: Joonsoo Kim This looks all correct to me, but I would ask for some style fixes. With those applied, please feel free to add Acked-by: Johannes Weiner > @@ -304,10 +308,10 @@ enum lruvec_flags { > struct lruvec { > struct list_head lists[NR_LRU_LISTS]; > struct zone_reclaim_stat reclaim_stat; > - /* Evictions & activations on the inactive file list */ > - atomic_long_t inactive_age; > + /* Evictions & activations on the inactive list */ > + atomic_long_t inactive_age[2]; Can you please add anon=0, file=1 to the comment? > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1431,10 +1431,14 @@ static char *memory_stat_format(struct mem_cgroup *memcg) > seq_buf_printf(&s, "%s %lu\n", vm_event_name(PGMAJFAULT), > memcg_events(memcg, PGMAJFAULT)); > > - seq_buf_printf(&s, "workingset_refault %lu\n", > - memcg_page_state(memcg, WORKINGSET_REFAULT)); > - seq_buf_printf(&s, "workingset_activate %lu\n", > - memcg_page_state(memcg, WORKINGSET_ACTIVATE)); > + seq_buf_printf(&s, "workingset_refault_anon %lu\n", > + memcg_page_state(memcg, WORKINGSET_REFAULT_ANON)); > + seq_buf_printf(&s, "workingset_refault_file %lu\n", > + memcg_page_state(memcg, WORKINGSET_REFAULT_FILE)); > + seq_buf_printf(&s, "workingset_activate_anon %lu\n", > + memcg_page_state(memcg, WORKINGSET_ACTIVATE_ANON)); > + seq_buf_printf(&s, "workingset_activate_file %lu\n", > + memcg_page_state(memcg, WORKINGSET_ACTIVATE_FILE)); > seq_buf_printf(&s, "workingset_nodereclaim %lu\n", > memcg_page_state(memcg, WORKINGSET_NODERECLAIM)); > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index c932141..0493c25 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -2716,7 +2716,10 @@ static bool shrink_node(pg_data_t *pgdat, struct scan_control *sc) > if (!sc->force_deactivate) { > unsigned long refaults; > > - if (inactive_is_low(target_lruvec, LRU_INACTIVE_ANON)) > + refaults = lruvec_page_state(target_lruvec, > + WORKINGSET_ACTIVATE_ANON); > + if (refaults != target_lruvec->refaults[0] || > + inactive_is_low(target_lruvec, LRU_INACTIVE_ANON)) > sc->may_deactivate |= DEACTIVATE_ANON; > else > sc->may_deactivate &= ~DEACTIVATE_ANON; > @@ -2727,8 +2730,8 @@ static bool shrink_node(pg_data_t *pgdat, struct scan_control *sc) > * rid of any stale active pages quickly. > */ > refaults = lruvec_page_state(target_lruvec, > - WORKINGSET_ACTIVATE); > - if (refaults != target_lruvec->refaults || > + WORKINGSET_ACTIVATE_FILE); > + if (refaults != target_lruvec->refaults[1] || > inactive_is_low(target_lruvec, LRU_INACTIVE_FILE)) > sc->may_deactivate |= DEACTIVATE_FILE; > else > @@ -3007,8 +3010,10 @@ static void snapshot_refaults(struct mem_cgroup *target_memcg, pg_data_t *pgdat) > unsigned long refaults; > > target_lruvec = mem_cgroup_lruvec(target_memcg, pgdat); > - refaults = lruvec_page_state(target_lruvec, WORKINGSET_ACTIVATE); > - target_lruvec->refaults = refaults; > + refaults = lruvec_page_state(target_lruvec, WORKINGSET_ACTIVATE_ANON); > + target_lruvec->refaults[0] = refaults; > + refaults = lruvec_page_state(target_lruvec, WORKINGSET_ACTIVATE_FILE); > + target_lruvec->refaults[1] = refaults; > } > > /* > diff --git a/mm/vmstat.c b/mm/vmstat.c > index 78d5337..3cdf8e9 100644 > --- a/mm/vmstat.c > +++ b/mm/vmstat.c > @@ -1146,8 +1146,10 @@ const char * const vmstat_text[] = { > "nr_isolated_anon", > "nr_isolated_file", > "workingset_nodes", > - "workingset_refault", > - "workingset_activate", > + "workingset_refault_anon", > + "workingset_refault_file", > + "workingset_activate_anon", > + "workingset_activate_file", > "workingset_restore", > "workingset_nodereclaim", > "nr_anon_pages", > diff --git a/mm/workingset.c b/mm/workingset.c > index 474186b..5fb8f85 100644 > --- a/mm/workingset.c > +++ b/mm/workingset.c > @@ -15,6 +15,7 @@ > #include > #include > #include > +#include Please keep the line-length sorting of the includes intact. If that's not possible, please separate the mm_inline.h include with an empty line into its own section. This makes it much easier to scan for specific headers, helps avoid duplicate includes, and is generally just easier on the eyes. > @@ -156,7 +157,7 @@ > * > * Implementation > * > - * For each node's file LRU lists, a counter for inactive evictions > + * For each node's anon/file LRU lists, a counter for inactive evictions > * and activations is maintained (node->inactive_age). > * > * On eviction, a snapshot of this counter (along with some bits to > @@ -213,7 +214,8 @@ static void unpack_shadow(void *shadow, int *memcgidp, pg_data_t **pgdat, > *workingsetp = workingset; > } > > -static void advance_inactive_age(struct mem_cgroup *memcg, pg_data_t *pgdat) > +static void advance_inactive_age(struct mem_cgroup *memcg, pg_data_t *pgdat, > + int is_file) bool file, please. > { > /* > * Reclaiming a cgroup means reclaiming all its children in a > @@ -230,7 +232,7 @@ static void advance_inactive_age(struct mem_cgroup *memcg, pg_data_t *pgdat) > struct lruvec *lruvec; > > lruvec = mem_cgroup_lruvec(memcg, pgdat); > - atomic_long_inc(&lruvec->inactive_age); > + atomic_long_inc(&lruvec->inactive_age[is_file]); > } while (memcg && (memcg = parent_mem_cgroup(memcg))); > } > > @@ -248,18 +250,19 @@ void *workingset_eviction(struct page *page, struct mem_cgroup *target_memcg) > unsigned long eviction; > struct lruvec *lruvec; > int memcgid; > + int is_file = page_is_file_cache(page); Please keep the line-length ordering intact here as well. > /* Page is fully exclusive and pins page->mem_cgroup */ > VM_BUG_ON_PAGE(PageLRU(page), page); > VM_BUG_ON_PAGE(page_count(page), page); > VM_BUG_ON_PAGE(!PageLocked(page), page); > > - advance_inactive_age(page_memcg(page), pgdat); > + advance_inactive_age(page_memcg(page), pgdat, is_file); > > lruvec = mem_cgroup_lruvec(target_memcg, pgdat); > /* XXX: target_memcg can be NULL, go through lruvec */ > memcgid = mem_cgroup_id(lruvec_memcg(lruvec)); > - eviction = atomic_long_read(&lruvec->inactive_age); > + eviction = atomic_long_read(&lruvec->inactive_age[is_file]); > return pack_shadow(memcgid, pgdat, eviction, PageWorkingset(page)); > } > > @@ -278,13 +281,16 @@ void workingset_refault(struct page *page, void *shadow) > struct lruvec *eviction_lruvec; > unsigned long refault_distance; > struct pglist_data *pgdat; > - unsigned long active_file; > + unsigned long active; > struct mem_cgroup *memcg; > unsigned long eviction; > struct lruvec *lruvec; > unsigned long refault; > bool workingset; > int memcgid; > + int is_file = page_is_file_cache(page); > + enum lru_list active_lru = page_lru_base_type(page) + LRU_ACTIVE; > + enum node_stat_item workingset_stat; Please use bool file and keep line-length ordering. The workingset_stat variable doesn't seem helpful, see below: > unpack_shadow(shadow, &memcgid, &pgdat, &eviction, &workingset); > > @@ -309,8 +315,8 @@ void workingset_refault(struct page *page, void *shadow) > if (!mem_cgroup_disabled() && !eviction_memcg) > goto out; > eviction_lruvec = mem_cgroup_lruvec(eviction_memcg, pgdat); > - refault = atomic_long_read(&eviction_lruvec->inactive_age); > - active_file = lruvec_page_state(eviction_lruvec, NR_ACTIVE_FILE); > + refault = atomic_long_read(&eviction_lruvec->inactive_age[is_file]); > + active = lruvec_page_state(eviction_lruvec, active_lru); > > /* > * Calculate the refault distance > @@ -341,19 +347,21 @@ void workingset_refault(struct page *page, void *shadow) > memcg = page_memcg(page); > lruvec = mem_cgroup_lruvec(memcg, pgdat); > > - inc_lruvec_state(lruvec, WORKINGSET_REFAULT); > + workingset_stat = WORKINGSET_REFAULT_BASE + is_file; > + inc_lruvec_state(lruvec, workingset_stat); inc_lruvec_state(lruvec, WORKINGSET_REFAULT_BASE + file); > /* > * Compare the distance to the existing workingset size. We > * don't act on pages that couldn't stay resident even if all > * the memory was available to the page cache. > */ > - if (refault_distance > active_file) > + if (refault_distance > active) > goto out; > > SetPageActive(page); > - advance_inactive_age(memcg, pgdat); > - inc_lruvec_state(lruvec, WORKINGSET_ACTIVATE); > + advance_inactive_age(memcg, pgdat, is_file); > + workingset_stat = WORKINGSET_ACTIVATE_BASE + is_file; > + inc_lruvec_state(lruvec, workingset_stat); inc_lruvec_state(lruvec, WORKINGSET_ACTIVATE_BASE + file); > /* Page was active prior to eviction */ > if (workingset) { > @@ -371,6 +379,7 @@ void workingset_refault(struct page *page, void *shadow) > void workingset_activation(struct page *page) > { > struct mem_cgroup *memcg; > + int is_file = page_is_file_cache(page); bool file & length-ordering