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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 84DE1C76196 for ; Thu, 6 Apr 2023 17:32:05 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DEECC6B0075; Thu, 6 Apr 2023 13:32:04 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D77A36B0078; Thu, 6 Apr 2023 13:32:04 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C3F286B007B; Thu, 6 Apr 2023 13:32:04 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id B1C9A6B0075 for ; Thu, 6 Apr 2023 13:32:04 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 8A98FACB5B for ; Thu, 6 Apr 2023 17:32:04 +0000 (UTC) X-FDA: 80651659368.25.1A190BD Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by imf06.hostedemail.com (Postfix) with ESMTP id CABA4180012 for ; Thu, 6 Apr 2023 17:32:01 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=Vi3Yx49P; dmarc=pass (policy=none) header.from=intel.com; spf=none (imf06.hostedemail.com: domain of tim.c.chen@linux.intel.com has no SPF policy when checking 192.55.52.43) smtp.mailfrom=tim.c.chen@linux.intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1680802322; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=CN9gmLyg7M7M+aCZm2EmTzkx0yyYniDjnt1XUA/Dbxc=; b=W6ZXL+NTuXqf69nGnzJaH5QOgBcksLbdyuE4TQqxEEkWVMImdO2bwJan6Vj2Make/dNkOm 9Q0j5SKDSnFN2QP0ZCxgtpQfqPhS8PywlxttsrfFxOSy1qhE1nHm7PftWk9yyKW5QmKF9d JlV+hBJcFOHHuH/O1c4lNyJC+bB1mXI= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=Vi3Yx49P; dmarc=pass (policy=none) header.from=intel.com; spf=none (imf06.hostedemail.com: domain of tim.c.chen@linux.intel.com has no SPF policy when checking 192.55.52.43) smtp.mailfrom=tim.c.chen@linux.intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1680802322; a=rsa-sha256; cv=none; b=HDayAHQn17FNdSZp/bSHYpEqVLqijnTDZlPDcHX87JseRnMUfTNW7KoWze3it8wa2mxOj8 3xSfJc2heIVi4jBsS0CjgLLsp/XwNGO65+0u4xbzc1LdrCw9zYgG/Ff79+MKxA0igNciPl D2iZ/tDPk/uD5EjkJwHIh/sGcjVd+hY= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1680802321; x=1712338321; h=message-id:subject:from:to:cc:date:in-reply-to: references:content-transfer-encoding:mime-version; bh=T6fnB87Tnl00OWMznGSXjLswX9ii6nq9ScLgD4rSTI0=; b=Vi3Yx49PJqD6IOIngXWkc+pMczdYOBJvcMpsmmt/zbbUKjyIUhK8MSop J3dvrjVFhArYTfa5FfcTzagoBjxPO6kLkVQHfpyW+FjRSTCj+zs4TTiSZ XVDSaD3gUBxyE8V/VUEySbSAt7a1s4xIQ+DOiUNe36dSweUf8+TfQzSBf c+FYJPzS0LWfHlkyFnlMvRLVCT+XKX28FdFhas05FclZGZDDFicmELrBd hBptJH2gOu45IdeaFyxNxZhjZwOrQdGoJt5j+Hq1to7HgRecKY7eBEvlo Dcev0AWnbZHqa1vHki2k2aPICIkfppp/fVobtoicr/47yQgav0ThQopIt A==; X-IronPort-AV: E=McAfee;i="6600,9927,10672"; a="429086490" X-IronPort-AV: E=Sophos;i="5.98,323,1673942400"; d="scan'208";a="429086490" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Apr 2023 10:31:56 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10672"; a="861477678" X-IronPort-AV: E=Sophos;i="5.98,323,1673942400"; d="scan'208";a="861477678" Received: from ticela-az-114.amr.corp.intel.com (HELO [10.251.3.106]) ([10.251.3.106]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 06 Apr 2023 10:31:54 -0700 Message-ID: <7ce03e4323b95c1e8fd3faed32c9b285162fe5a8.camel@linux.intel.com> Subject: Re: [PATCH v5 2/2] mm: vmscan: refactor reclaim_state helpers From: Tim Chen To: Yosry Ahmed , Andrew Morton , Alexander Viro , "Darrick J. Wong" , Christoph Lameter , David Rientjes , Joonsoo Kim , Vlastimil Babka , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com>, "Matthew Wilcox (Oracle)" , Miaohe Lin , David Hildenbrand , Johannes Weiner , Peter Xu , NeilBrown , Shakeel Butt , Michal Hocko , Yu Zhao , Dave Chinner Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-xfs@vger.kernel.org, linux-mm@kvack.org Date: Thu, 06 Apr 2023 10:31:53 -0700 In-Reply-To: <20230405185427.1246289-3-yosryahmed@google.com> References: <20230405185427.1246289-1-yosryahmed@google.com> <20230405185427.1246289-3-yosryahmed@google.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.44.4 (3.44.4-2.fc36) MIME-Version: 1.0 X-Rspamd-Queue-Id: CABA4180012 X-Rspamd-Server: rspam09 X-Rspam-User: X-Stat-Signature: s9gcnd1b8crsmw4m9fqwr5jbzkfrjwrk X-HE-Tag: 1680802321-200596 X-HE-Meta: U2FsdGVkX19qVcU8xhdwAwwe1HLmLnb40RSNdBfKwdAOuqAac6ZlDWw5bwWQUsbYNeGrQLCq08vHV9+b7kSiBaHbkokzEROvO6tNnAphdViqvRlC4ODJWyVNL9tSwQdiHMBkEbu2Ho75h0NpfLa5ho4cA+09zpQvE2W07QRqr7Wh4xBIkdHkiH5eEm9zdwtlcCpkqlCUM8cTzI1d7LUtPIcMWMg2IBcd9L/JSHFJwk4J2wwq/nrsYSQjemPoCmudHZLKA+3gPItlcnb4fEqhdBDUryzNZfRBmW/eRv5+cQ5ZRmR286YF312pevXgpLHoYUE/gIyTboO8c9jdN7YGOClHBj3HxqDMo0Wfu3Af+Sln8YCM3Swf2TikZmMoBzZt1ccdpDdLRR73H0CTY9f+zU8sxd89QVUoFgrnRDRzzthh0qyj12yMUpD139HwIlIziRe4gZG5Nn7u5LSWhLBIvhnDT7i6mGxLyWk1GRL9BpkcGUXtG5Aw7Lcyo2eMt3OZdxuBK2ql3gtNTdAKl6Qv2Zdyz3O4vZ90CakfrnQBxGAU6YQCKW88evDKuTg+lZ/s+OT7cra2tkswXhYN2JQ4CkXgp6+1oTUc086upjpwd3oNHveAG54gynj8du3cQNVvHLcREwPb6tnjkFeAHa0Us5qTdI53vazRQaDnrW+qGJ+RpwHyQm3pp4wemtqNn6+wzyRUx6BL2VAVwsFaHHDnZUm/a02vBr8d47zAT2wLrmbAws659COv/wleb0h45+w0lEmT2ZMLBfw5WPr5vWo7sDZ9wi5qrHEHAbwBBbWRewrXOHX+5RXyUzeTvLa9SIQn1gnod7Fr3MIrTElDbM+MgwgUjEslrHy1xuk+kvXcMerPIpOalIttJasqnED9zurxl3SWvrvUz8ShPXHpLazoBJyqcI1QGw0bDpd/24WUPqR0yN9qVQDBTwGjuZVohcSRA3spnZ5llTHdViYO3h0 mT7dgGLp 6JG5/L6XPiuIs4mtm3G4r7XhSsEYygxZehbz06/SdFMJRlbacTAAuvc0loMcZffmXxIjJ/fBZowN2UVEo4Cachm8sPkhsJurHHd5uMj9sTmyYNybwG1QJSFhvYtsCdCgYm0WQKWtgbVPADKFpiThD6D99l3GkTAJ3nONAAQWUXRGeUwFyZtqRwa0PSo601ROfMn5ITZmrEHR9nWDeKncgsx9bX2KD8nIeLXVsmssDm8Ya7Scx5FacALmvctv5wmHYIzd/si+Mnw6xhMqz/brVDZHRAMqrLezYgmgh 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 Wed, 2023-04-05 at 18:54 +0000, Yosry Ahmed wrote: > During reclaim, we keep track of pages reclaimed from other means than > LRU-based reclaim through scan_control->reclaim_state->reclaimed_slab, > which we stash a pointer to in current task_struct. >=20 > However, we keep track of more than just reclaimed slab pages through > this. We also use it for clean file pages dropped through pruned inodes, > and xfs buffer pages freed. Rename reclaimed_slab to reclaimed, and add > a helper function that wraps updating it through current, so that future > changes to this logic are contained within mm/vmscan.c. >=20 > Additionally, add a flush_reclaim_state() helper to wrap using > reclaim_state->reclaimed to updated sc->nr_reclaimed, and use that > helper to add an elaborate comment about why we only do the update for > global reclaim. >=20 > Finally, move set_task_reclaim_state() next to flush_reclaim_state() so > that all reclaim_state helpers are in close proximity for easier > readability. >=20 > Signed-off-by: Yosry Ahmed > --- > fs/inode.c | 3 +- > fs/xfs/xfs_buf.c | 3 +- > include/linux/swap.h | 17 +++++++++- > mm/slab.c | 3 +- > mm/slob.c | 6 ++-- > mm/slub.c | 5 ++- > mm/vmscan.c | 75 ++++++++++++++++++++++++++++++++------------ > 7 files changed, 78 insertions(+), 34 deletions(-) >=20 > diff --git a/fs/inode.c b/fs/inode.c > index 4558dc2f13557..e60fcc41faf17 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -864,8 +864,7 @@ static enum lru_status inode_lru_isolate(struct list_= head *item, > __count_vm_events(KSWAPD_INODESTEAL, reap); > else > __count_vm_events(PGINODESTEAL, reap); > - if (current->reclaim_state) > - current->reclaim_state->reclaimed_slab +=3D reap; > + mm_account_reclaimed_pages(reap); > } > iput(inode); > spin_lock(lru_lock); > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c > index 54c774af6e1c6..15d1e5a7c2d34 100644 > --- a/fs/xfs/xfs_buf.c > +++ b/fs/xfs/xfs_buf.c > @@ -286,8 +286,7 @@ xfs_buf_free_pages( > if (bp->b_pages[i]) > __free_page(bp->b_pages[i]); > } > - if (current->reclaim_state) > - current->reclaim_state->reclaimed_slab +=3D bp->b_page_count; > + mm_account_reclaimed_pages(bp->b_page_count); > =20 > if (bp->b_pages !=3D bp->b_page_array) > kmem_free(bp->b_pages); > diff --git a/include/linux/swap.h b/include/linux/swap.h > index 209a425739a9f..e131ac155fb95 100644 > --- a/include/linux/swap.h > +++ b/include/linux/swap.h > @@ -153,13 +153,28 @@ union swap_header { > * memory reclaim > */ > struct reclaim_state { > - unsigned long reclaimed_slab; > + /* pages reclaimed outside of LRU-based reclaim */ > + unsigned long reclaimed; > #ifdef CONFIG_LRU_GEN > /* per-thread mm walk data */ > struct lru_gen_mm_walk *mm_walk; > #endif > }; > =20 > +/* > + * mm_account_reclaimed_pages(): account reclaimed pages outside of LRU-= based > + * reclaim > + * @pages: number of pages reclaimed > + * > + * If the current process is undergoing a reclaim operation, increment t= he > + * number of reclaimed pages by @pages. > + */ > +static inline void mm_account_reclaimed_pages(unsigned long pages) > +{ > + if (current->reclaim_state) > + current->reclaim_state->reclaimed +=3D pages; > +} > + > #ifdef __KERNEL__ > =20 > struct address_space; > diff --git a/mm/slab.c b/mm/slab.c > index dabc2a671fc6f..64bf1de817b24 100644 > --- a/mm/slab.c > +++ b/mm/slab.c > @@ -1392,8 +1392,7 @@ static void kmem_freepages(struct kmem_cache *cache= p, struct slab *slab) > smp_wmb(); > __folio_clear_slab(folio); > =20 > - if (current->reclaim_state) > - current->reclaim_state->reclaimed_slab +=3D 1 << order; > + mm_account_reclaimed_pages(1 << order); > unaccount_slab(slab, order, cachep); > __free_pages(&folio->page, order); > } > diff --git a/mm/slob.c b/mm/slob.c > index fe567fcfa3a39..79cc8680c973c 100644 > --- a/mm/slob.c > +++ b/mm/slob.c > @@ -61,7 +61,7 @@ > #include > =20 > #include > -#include /* struct reclaim_state */ > +#include /* mm_account_reclaimed_pages() */ > #include > #include > #include > @@ -211,9 +211,7 @@ static void slob_free_pages(void *b, int order) > { > struct page *sp =3D virt_to_page(b); > =20 > - if (current->reclaim_state) > - current->reclaim_state->reclaimed_slab +=3D 1 << order; > - > + mm_account_reclaimed_pages(1 << order); > mod_node_page_state(page_pgdat(sp), NR_SLAB_UNRECLAIMABLE_B, > -(PAGE_SIZE << order)); > __free_pages(sp, order); > diff --git a/mm/slub.c b/mm/slub.c > index 39327e98fce34..7aa30eef82350 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -11,7 +11,7 @@ > */ > =20 > #include > -#include /* struct reclaim_state */ > +#include /* mm_account_reclaimed_pages() */ > #include > #include > #include > @@ -2063,8 +2063,7 @@ static void __free_slab(struct kmem_cache *s, struc= t slab *slab) > /* Make the mapping reset visible before clearing the flag */ > smp_wmb(); > __folio_clear_slab(folio); > - if (current->reclaim_state) > - current->reclaim_state->reclaimed_slab +=3D pages; > + mm_account_reclaimed_pages(pages); > unaccount_slab(slab, order, s); > __free_pages(&folio->page, order); > } > diff --git a/mm/vmscan.c b/mm/vmscan.c > index c82bd89f90364..049e39202e6ce 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -188,18 +188,6 @@ struct scan_control { > */ > int vm_swappiness =3D 60; > =20 > -static void set_task_reclaim_state(struct task_struct *task, > - struct reclaim_state *rs) > -{ > - /* Check for an overwrite */ > - WARN_ON_ONCE(rs && task->reclaim_state); > - > - /* Check for the nulling of an already-nulled member */ > - WARN_ON_ONCE(!rs && !task->reclaim_state); > - > - task->reclaim_state =3D rs; > -} > - > LIST_HEAD(shrinker_list); > DECLARE_RWSEM(shrinker_rwsem); > =20 > @@ -511,6 +499,59 @@ static bool writeback_throttling_sane(struct scan_co= ntrol *sc) > } > #endif > =20 > +static void set_task_reclaim_state(struct task_struct *task, > + struct reclaim_state *rs) > +{ > + /* Check for an overwrite */ > + WARN_ON_ONCE(rs && task->reclaim_state); > + > + /* Check for the nulling of an already-nulled member */ > + WARN_ON_ONCE(!rs && !task->reclaim_state); > + > + task->reclaim_state =3D rs; > +} > + > +/* > + * flush_reclaim_state(): add pages reclaimed outside of LRU-based recla= im to > + * scan_control->nr_reclaimed. > + */ > +static void flush_reclaim_state(struct scan_control *sc, > + struct reclaim_state *rs) > +{ > + /* > + * Currently, reclaim_state->reclaimed includes three types of pages > + * freed outside of vmscan: > + * (1) Slab pages. > + * (2) Clean file pages from pruned inodes. > + * (3) XFS freed buffer pages. > + * > + * For all of these cases, we have no way of finding out whether these > + * pages were related to the memcg under reclaim. For example, a freed > + * slab page could have had only a single object charged to the memcg Minor nits: s/could have had/could have > + * under reclaim. Also, populated inodes are not on shrinker LRUs > + * anymore except on highmem systems. > + * > + * Instead of over-reporting the reclaimed pages in a memcg reclaim, > + * only count such pages in global reclaim. This prevents unnecessary May be clearer to say: This prevents under-reclaimaing the target memcg, and unnecessary > + * retries during memcg charging and false positive from proactive > + * reclaim (memory.reclaim). > + * Tim