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 5A519C47DD9 for ; Wed, 27 Mar 2024 07:31:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DC2EC6B009E; Wed, 27 Mar 2024 03:31:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D726D6B00A0; Wed, 27 Mar 2024 03:31:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BEBCA6B00A1; Wed, 27 Mar 2024 03:31:22 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id AE77A6B00A0 for ; Wed, 27 Mar 2024 03:31:22 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 55B21120B10 for ; Wed, 27 Mar 2024 07:31:22 +0000 (UTC) X-FDA: 81941998404.01.7FA85F5 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.18]) by imf01.hostedemail.com (Postfix) with ESMTP id 1AF2C40023 for ; Wed, 27 Mar 2024 07:31:19 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b="HgZsV//g"; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf01.hostedemail.com: domain of ying.huang@intel.com designates 198.175.65.18 as permitted sender) smtp.mailfrom=ying.huang@intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1711524680; 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=4e+E2o7iU+2H0cim6tLTyvIwpGRSqyggm11ChumewV8=; b=3ROT5GkK3taYisuUBM5xlYgE+OMKf/Utg9tWDE/9z16NWV8+Z237Ap8o05xLLL65GXaxwH 5MyIcIeQrLIqV+Zl66Su4+pBCh/COoy+XHl9T04Ui62C2DoC/rW7B6eQ4HYsaWT0b+zeLM JTZwp50iEJeLfTJZ68Z/uZrTM6bT4B4= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b="HgZsV//g"; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf01.hostedemail.com: domain of ying.huang@intel.com designates 198.175.65.18 as permitted sender) smtp.mailfrom=ying.huang@intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1711524680; a=rsa-sha256; cv=none; b=brGfXWDufX/98c7ReMcIXsa7FD7cd3urb4H7PFjwySBD8Lb7uk6WPQXQduX4Neoxr/1cuZ GtTe21eKXhTk5z0fpVxWlQpxinxi/be66vWoXjZGtB4RV8+I/ZF5ritI36P6P54pO09xKn RzOFtKEKma1Y37COdwQx5sVN75vq338= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1711524680; x=1743060680; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version:content-transfer-encoding; bh=jA/U0eowmhkvePD2PDKZbHPIlUMQ8vsrUVqd0Nrq2g0=; b=HgZsV//gbOOyFA7bCGIeJ7lDgFc7vjI+EJmbCSvZu4UVwjr/ClA2fZ0f 1mR38UeB3qD9UDtXWFUfb920owGKDF5Nv3EtO9eH+za63GVZvKBIAPxpy hHGC2i6SAl00foyLHXZ4V4UTO1tTxIYe8TV8xyS1hAxYtXNQqjGm03huM SKCVpWBDsefoUKRFPQ8cgl6wwvsWuIixsKqzHczNsFO8dwOKFBzdSc2UJ j7sr/q2+rgklq5tErwT1IENNV7IJomxNzE8vRnV8sBikvouCBnkTFG5f+ CVSNpHVDlTsB437G1H3+pqJsygV5LG7DJZ+3etD0UU7u9Sb0/KCkt5cA6 A==; X-CSE-ConnectionGUID: USyBV9zjTESsoNK2J8/yzQ== X-CSE-MsgGUID: mgW3lfB5SxOk1D0aseZ9vA== X-IronPort-AV: E=McAfee;i="6600,9927,11025"; a="6739517" X-IronPort-AV: E=Sophos;i="6.07,158,1708416000"; d="scan'208";a="6739517" Received: from fmviesa008.fm.intel.com ([10.60.135.148]) by orvoesa110.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Mar 2024 00:31:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.07,158,1708416000"; d="scan'208";a="16305356" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by fmviesa008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Mar 2024 00:31:15 -0700 From: "Huang, Ying" To: Kairui Song Cc: linux-mm@kvack.org, Chris Li , Minchan Kim , Barry Song , Ryan Roberts , Yu Zhao , SeongJae Park , David Hildenbrand , Yosry Ahmed , Johannes Weiner , Matthew Wilcox , Nhat Pham , Chengming Zhou , Andrew Morton , linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH 04/10] mm/swap: remove cache bypass swapin In-Reply-To: (Kairui Song's message of "Wed, 27 Mar 2024 14:55:53 +0800") References: <20240326185032.72159-1-ryncsn@gmail.com> <20240326185032.72159-5-ryncsn@gmail.com> <87v858mbjh.fsf@yhuang6-desk2.ccr.corp.intel.com> Date: Wed, 27 Mar 2024 15:29:21 +0800 Message-ID: <87msqkm8tq.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: 11fsh4mpbqu5m9b1qi3ww3wppxgos8sj X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 1AF2C40023 X-HE-Tag: 1711524679-939254 X-HE-Meta: U2FsdGVkX1+9AIl4drixIFQAXPudNAzLhvKY9Da2ZvIm1gpBLXAgJS91rKpsD83efkCgMkx89szliDfIhKED1PHM9ux4hgmkVYjznzqjaQN3KDfb1bdPlPWbspnIajjj8kG+yaJMUFkBgKV7MGCi3zVPrgGfzFH6Gujf2hQrwXCUT7TcVyzo1qQFkDDORvCLy55K7ARHK1QDk+/4THvQOzpqb7AUPAIIjYLJ/mrKyqX33EZK9vVjBJLQg2JWjuYxvdpFpO7nERnylq4qHmsG8HBTaGS5HHjyKiHCc2KAsGsBzVIhFiX1K6+sA1hgU4uUcyeCuVilnDlgFdFK8xCjFyCKscEKtEXtiDpHBhT/czCLK2ZbX78iAtBlS1n/Q9D48enuge5WrQ16B92E1FmJnph68JXLm3JGsaW1VZotyUXBHqJXZfIUADyZs2GjPWvqA2rfpvCBAR+152FBVchOfnjDXOWeKlksGOmUYHDJZoU+bzVivDs7fXtevaQQjGtt/VcmBR3d/77Q0DATHWazdOPIVSGnLM8wktcw8kX9eL3JQxwo0eg/vXpsGAPv4mhW3RQivVKVQ+YPIvE0rhKa7oVg/qkhjf/MKQMuR88bWZdCBEmAgnmzizYcYZKVnOIuX+x55eyzAjEm4cqcPlAh/dxnOo8B/lNpBNTjInVL4baToGkGWVdy1Jz591rHvnBq+8/Usd5WM60sK6/msjl78tAFOxY+HRh5tzaHSVi0Jc4WZOEdCWa/uwJtKio9p9Ey/b6RgMREEFVBz9jrjzNXjU0f4kuPFRAc06QMbtkFDnCjEV2lYN4PAlbnq9Nkv2WKh6EFPyI/Gh/0RAdtVZI7NQufVB7ckV80 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: List-Subscribe: List-Unsubscribe: Kairui Song writes: > On Wed, Mar 27, 2024 at 2:32=E2=80=AFPM Huang, Ying wrote: >> >> Kairui Song writes: >> >> > From: Kairui Song >> > >> > We used to have the cache bypass swapin path for better performance, >> > but by removing it, more optimization can be applied and have >> > an even better overall performance and less hackish. >> > >> > And these optimizations are not easily doable or not doable at all >> > without this. >> > >> > This patch simply removes it, and the performance will drop heavily >> > for simple swapin, things won't get this worse for real workloads >> > but still observable. Following commits will fix this and archive a >> > better performance. >> > >> > Swapout/in 30G zero pages from ZRAM (This mostly measures overhead >> > of swap path itself, because zero pages are not compressed but simply >> > recorded in ZRAM, and performance drops more as SWAP device is getting >> > full): >> > >> > Test result of sequential swapin/out: >> > >> > Before (us) After (us) >> > Swapout: 33619409 33624641 >> > Swapin: 32393771 41614858 (-28.4%) >> > Swapout (THP): 7817909 7795530 >> > Swapin (THP) : 32452387 41708471 (-28.4%) >> > >> > Signed-off-by: Kairui Song >> > --- >> > mm/memory.c | 18 ++++------------- >> > mm/swap.h | 10 +++++----- >> > mm/swap_state.c | 53 ++++++++++--------------------------------------- >> > mm/swapfile.c | 13 ------------ >> > 4 files changed, 19 insertions(+), 75 deletions(-) >> > >> > diff --git a/mm/memory.c b/mm/memory.c >> > index dfdb620a9123..357d239ee2f6 100644 >> > --- a/mm/memory.c >> > +++ b/mm/memory.c >> > @@ -3932,7 +3932,6 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) >> > struct page *page; >> > struct swap_info_struct *si =3D NULL; >> > rmap_t rmap_flags =3D RMAP_NONE; >> > - bool need_clear_cache =3D false; >> > bool exclusive =3D false; >> > swp_entry_t entry; >> > pte_t pte; >> > @@ -4000,14 +3999,9 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) >> > if (!folio) { >> > if (data_race(si->flags & SWP_SYNCHRONOUS_IO) && >> > __swap_count(entry) =3D=3D 1) { >> > - /* skip swapcache and readahead */ >> > folio =3D swapin_direct(entry, GFP_HIGHUSER_MOVA= BLE, vmf); >> > - if (PTR_ERR(folio) =3D=3D -EBUSY) >> > - goto out; >> > - need_clear_cache =3D true; >> > } else { >> > folio =3D swapin_readahead(entry, GFP_HIGHUSER_M= OVABLE, vmf); >> > - swapcache =3D folio; >> > } >> > >> > if (!folio) { >> > @@ -4023,6 +4017,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) >> > goto unlock; >> > } >> > >> > + swapcache =3D folio; >> > page =3D folio_file_page(folio, swp_offset(entry)); >> > >> > /* Had to read the page from swap area: Major fault */ >> > @@ -4187,7 +4182,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) >> > vmf->orig_pte =3D pte; >> > >> > /* ksm created a completely new copy */ >> > - if (unlikely(folio !=3D swapcache && swapcache)) { >> > + if (unlikely(folio !=3D swapcache)) { >> > folio_add_new_anon_rmap(folio, vma, vmf->address); >> > folio_add_lru_vma(folio, vma); >> > } else { >> > @@ -4201,7 +4196,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) >> > arch_do_swap_page(vma->vm_mm, vma, vmf->address, pte, vmf->orig_= pte); >> > >> > folio_unlock(folio); >> > - if (folio !=3D swapcache && swapcache) { >> > + if (folio !=3D swapcache) { >> > /* >> > * Hold the lock to avoid the swap entry to be reused >> > * until we take the PT lock for the pte_same() check >> > @@ -4227,9 +4222,6 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) >> > if (vmf->pte) >> > pte_unmap_unlock(vmf->pte, vmf->ptl); >> > out: >> > - /* Clear the swap cache pin for direct swapin after PTL unlock */ >> > - if (need_clear_cache) >> > - swapcache_clear(si, entry); >> > if (si) >> > put_swap_device(si); >> > return ret; >> > @@ -4240,12 +4232,10 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) >> > folio_unlock(folio); >> > out_release: >> > folio_put(folio); >> > - if (folio !=3D swapcache && swapcache) { >> > + if (folio !=3D swapcache) { >> > folio_unlock(swapcache); >> > folio_put(swapcache); >> > } >> > - if (need_clear_cache) >> > - swapcache_clear(si, entry); >> > if (si) >> > put_swap_device(si); >> > return ret; >> > diff --git a/mm/swap.h b/mm/swap.h >> > index aee134907a70..ac9573b03432 100644 >> > --- a/mm/swap.h >> > +++ b/mm/swap.h >> > @@ -41,7 +41,6 @@ void __delete_from_swap_cache(struct folio *folio, >> > void delete_from_swap_cache(struct folio *folio); >> > void clear_shadow_from_swap_cache(int type, unsigned long begin, >> > unsigned long end); >> > -void swapcache_clear(struct swap_info_struct *si, swp_entry_t entry); >> > struct folio *swap_cache_get_folio(swp_entry_t entry, >> > struct vm_area_struct *vma, unsigned long addr); >> > struct folio *filemap_get_incore_folio(struct address_space *mapping, >> > @@ -100,14 +99,15 @@ static inline struct folio *swapin_readahead(swp_= entry_t swp, gfp_t gfp_mask, >> > { >> > return NULL; >> > } >> > - >> > -static inline int swap_writepage(struct page *p, struct writeback_con= trol *wbc) >> > +static inline struct folio *swapin_direct(swp_entry_t entry, gfp_t fl= ag, >> > + struct vm_fault *vmf); >> > { >> > - return 0; >> > + return NULL; >> > } >> > >> > -static inline void swapcache_clear(struct swap_info_struct *si, swp_e= ntry_t entry) >> > +static inline int swap_writepage(struct page *p, struct writeback_con= trol *wbc) >> > { >> > + return 0; >> > } >> > >> > static inline struct folio *swap_cache_get_folio(swp_entry_t entry, >> > diff --git a/mm/swap_state.c b/mm/swap_state.c >> > index 2a9c6bdff5ea..49ef6250f676 100644 >> > --- a/mm/swap_state.c >> > +++ b/mm/swap_state.c >> > @@ -880,61 +880,28 @@ static struct folio *swap_vma_readahead(swp_entr= y_t targ_entry, gfp_t gfp_mask, >> > } >> > >> > /** >> > - * swapin_direct - swap in folios skipping swap cache and readahead >> > + * swapin_direct - swap in folios skipping readahead >> > * @entry: swap entry of this memory >> > * @gfp_mask: memory allocation flags >> > * @vmf: fault information >> > * >> > - * Returns the struct folio for entry and addr after the swap entry i= s read >> > - * in. >> > + * Returns the folio for entry after it is read in. >> > */ >> > struct folio *swapin_direct(swp_entry_t entry, gfp_t gfp_mask, >> > struct vm_fault *vmf) >> > { >> > - struct vm_area_struct *vma =3D vmf->vma; >> > + struct mempolicy *mpol; >> > struct folio *folio; >> > - void *shadow =3D NULL; >> > - >> > - /* >> > - * Prevent parallel swapin from proceeding with >> > - * the cache flag. Otherwise, another thread may >> > - * finish swapin first, free the entry, and swapout >> > - * reusing the same entry. It's undetectable as >> > - * pte_same() returns true due to entry reuse. >> > - */ >> > - if (swapcache_prepare(entry)) { >> > - /* Relax a bit to prevent rapid repeated page faults */ >> > - schedule_timeout_uninterruptible(1); >> > - return ERR_PTR(-EBUSY); >> > - } >> > - >> > - /* skip swapcache */ >> > - folio =3D vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0, >> > - vma, vmf->address, false); >> > - if (folio) { >> > - __folio_set_locked(folio); >> > - __folio_set_swapbacked(folio); >> > - >> > - if (mem_cgroup_swapin_charge_folio(folio, >> > - vma->vm_mm, GFP_KERNEL, >> > - entry)) { >> > - folio_unlock(folio); >> > - folio_put(folio); >> > - return NULL; >> > - } >> > - mem_cgroup_swapin_uncharge_swap(entry); >> > - >> > - shadow =3D get_shadow_from_swap_cache(entry); >> > - if (shadow) >> > - workingset_refault(folio, shadow); >> > + bool page_allocated; >> > + pgoff_t ilx; >> > >> > - folio_add_lru(folio); >> > + mpol =3D get_vma_policy(vmf->vma, vmf->address, 0, &ilx); >> > + folio =3D __read_swap_cache_async(entry, gfp_mask, mpol, ilx, >> > + &page_allocated, false); >> > + mpol_cond_put(mpol); >> > >> > - /* To provide entry to swap_read_folio() */ >> > - folio->swap =3D entry; >> > + if (page_allocated) >> > swap_read_folio(folio, true, NULL); >> > - folio->private =3D NULL; >> > - } >> > >> > return folio; >> > } >> >> This looks similar as read_swap_cache_async(). Can we merge them? > > Yes, that's doable. But I may have to split it out again for later > optimizations though. > >> >> And, we should avoid to readahead in swapin_readahead() or >> swap_vma_readahead() for SWP_SYNCHRONOUS_IO anyway. So, it appears that >> we can change and use swapin_readahead() directly? > > Good point, SWP_SYNCHRONOUS_IO check can be extended more after this > series, but readahead optimization could be another series (like the > previous one which tried to unify readahead for shmem/anon), so I > thought it's better to keep it untouched for now. Just want to check whether we can reduce the special processing for SWP_SYNCHRONOUS_IO as much as possible. -- Best Regards, Huang, Ying