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 5E131C48286 for ; Thu, 1 Feb 2024 23:21:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C9DC76B0071; Thu, 1 Feb 2024 18:21:03 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id C4E526B0078; Thu, 1 Feb 2024 18:21:03 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B15F46B007D; Thu, 1 Feb 2024 18:21:03 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 9D4846B0071 for ; Thu, 1 Feb 2024 18:21:03 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 45DECA1BBE for ; Thu, 1 Feb 2024 23:21:03 +0000 (UTC) X-FDA: 81744807606.26.53892EF Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.13]) by imf11.hostedemail.com (Postfix) with ESMTP id EF7E44000C for ; Thu, 1 Feb 2024 23:21:00 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=WJ7HR+3D; spf=none (imf11.hostedemail.com: domain of tim.c.chen@linux.intel.com has no SPF policy when checking 198.175.65.13) smtp.mailfrom=tim.c.chen@linux.intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706829661; 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=h9znlnImZtydVs8z5rzKTuHVOfgfLLo/g2TEMYfkVkA=; b=b2zApSRoPlvDxJcQfuFBLIj2jBGCHJMG2Paf8CWpyLRcZJBR+NwuLKOAjM/Zbb/32ciZgk JMen3f0g9hs0+/8nVGQMw484B9XFPceQkoXbM/W3kepkezimkhGEDXUTWW06/dq0Awoq02 CYyQwyf1dxiD/JiJOs/DpA+iPyAHGqo= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706829661; a=rsa-sha256; cv=none; b=WZBK4+Y4x/Z/AduD4sx92F0BWWugWTdOlhmbJKuVyEY2/YekrAGph8e/j8WoxmP/cscROA +5m0xTJolFg/Yo2D7vIZDTfdXqZAjXkk1oezeaFM7pRbk+m9QJtQlXBNjxB4eqzcdojxs6 U2Tgx7jkEPTjdSoXqkg+rEJELiHTfzs= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=WJ7HR+3D; spf=none (imf11.hostedemail.com: domain of tim.c.chen@linux.intel.com has no SPF policy when checking 198.175.65.13) smtp.mailfrom=tim.c.chen@linux.intel.com; dmarc=pass (policy=none) header.from=intel.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1706829662; x=1738365662; h=message-id:subject:from:to:cc:date:in-reply-to: references:content-transfer-encoding:mime-version; bh=l9z7Z/CPSJRsgaZ/pWgOCZAEvFTdT/WuuGesqxR6Q/s=; b=WJ7HR+3Dpk4WM5W9Uhk6bzBslhsFK2xrf4vRQdPJ3Q2CG1jU1iH1ELJD jRdvEsKRYiC5XDfIGRuWXxjWBmgFdVHfmrL3bg1pj1MAi+JS29FwCQqs+ Bvm3MbhXsBCSvL/drVw3WHwce/cv9OtDKmGaG4YYy5+gr9cO3ctBQrHmz YG66qR39ceVhd63pkIbnw7vKlOZ2efr5IzgGW5hxJ1RjMvPD8UNKf7Oid uo2jrxFgQytYxYzJ12lYM1Bdl2V1xblq5VYQm9BlxTKAMbUatMF0T8t9k VZYqfXABRK7tzZBY4HA9ZrQX2EDZb91TFYFh7ThIKCTUmy2RAgPdjzVkM g==; X-IronPort-AV: E=McAfee;i="6600,9927,10971"; a="11133230" X-IronPort-AV: E=Sophos;i="6.05,236,1701158400"; d="scan'208";a="11133230" Received: from orviesa004.jf.intel.com ([10.64.159.144]) by orvoesa105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Feb 2024 15:21:00 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.05,236,1701158400"; d="scan'208";a="4654575" Received: from xzhou1-mobl.amr.corp.intel.com (HELO [10.212.155.8]) ([10.212.155.8]) by orviesa004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Feb 2024 15:20:59 -0800 Message-ID: <7f19b4d69ff20efe8260a174c7866b4819532b1f.camel@linux.intel.com> Subject: Re: [PATCH v2] mm: swap: async free swap slot cache entries From: Tim Chen To: "Huang, Ying" , Chris Li Cc: Andrew Morton , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Wei =?UTF-8?Q?Xu=EF=BF=BC?= , Yu =?UTF-8?Q?Zhao=EF=BF=BC?= , Greg Thelen , Chun-Tse Shao , Suren =?UTF-8?Q?Baghdasaryan=EF=BF=BC?= , Yosry =?UTF-8?Q?Ahmed=EF=BF=BC?= , Brain Geffon , Minchan Kim , Michal Hocko , Mel Gorman , Nhat Pham , Johannes Weiner , Kairui Song , Zhongkun He , Kemeng Shi , Barry Song Date: Thu, 01 Feb 2024 15:20:58 -0800 In-Reply-To: <87sf2ceoks.fsf@yhuang6-desk2.ccr.corp.intel.com> References: <20240131-async-free-v2-1-525f03e07184@kernel.org> <87sf2ceoks.fsf@yhuang6-desk2.ccr.corp.intel.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: EF7E44000C X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: yefhrce5j7owyqedto6t7kfbtfg39i41 X-HE-Tag: 1706829660-561843 X-HE-Meta: U2FsdGVkX1+d+obyXXYc3evTvttBBsfnWVzbOISl4Eo1tiVXWPNdwt2Ce0eakK+tTimUTIgcYwpx4aBJm8VJt7Cq4JS00S/SrzKHUu6zE/JaiOKIdz5OSDGzQKJin8CITsCyhkN1jRrMkUc2a2YrDsuY3RiDDuCVby/JThnuyVBHzfwahsZqWJK84vR+BNQP/XAbLSTWBiTvEKsKLwTs0nYBsZzHhnlJMcRaL9b0d2Od7ICBm6ipyA0exXqle+CZFtl3C9ldVzO2CZWq8dm1bLHQ4Q6Y4fK7qrUCZZEUzsMj2mtdgYWyqPmd4exULGzaNP7nCRVX0OWDuMTtzAH8v95fKHQ3EK3pAyzWZ368OGZKg6w5YB97/jdUWeRaxTjhcdGUMhNv3s2Eh3u5Lboz51pID0hzwKI2OIN1O3KEibBzlaDUfBvbWrVah3Cw2NXF6TVg/m4cP1ZLLA2AGGqKXDWOUn1nn7brLAbVmWcvxyU8xxSyc2fvlGkjogUCh4u62sYAhm06vSzK/kXdnlf29+g3ngU550uQ8JoDS7E4PEj/QB+YAnzGhy0CKwVq0JSZ7S111bAdu67WkPavFBR1qVzHhH+Wx7fnYrBgeFRLysz6C+hHmX2dli7xuj/e4Fw9Sn8/NnthAHCRPSMK4Vw21x1Hs/DEuIOmLvXI9j9YS9KXQX0olcHAPMJaGRfg8GCE4XY4YXxDGzH2eqVCMwwKnV+H2MO3TCoJEGwtC3Wzs0xc7Mzi6kBw1l/povmjwj6lGcznRtc60DGe0tZEWVyYtFkme6+Jct9HmRsD+1Cjwl9mirIf7aJghzqpd1pgqsAxQuNyqON6Y5yiS9wqiaIPe+EmsksmfDelKtNxpHjxKGcCkgV3J51qEdnexPzNGZIUhrQA5dhrWaVqaH3hVhReZvlT5AsauV/TJLt2bJ7iKM9JtdA5Bju1fYJhm2lFFI27l0xcXakq7iPkTIuKkWp CmVN0q8D I8ZPqifwJfWxfogwj1athv1ogxPy9stugGcHBUoLro18Klw7XPh9147/2jKZJ+vLQ9Y8c4v7nad4ATFVSuIWgPuRWvdv1XsvrkGk5O8L1JN0uFfhlgx6pB6mF9UIHj0TQXVITba1Zo/l+63RexlH7u9LEj1c0ASD0CXt17IB4c78cZp0iWXvHN6w5lQCA/UZ0CNuK 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: On Thu, 2024-02-01 at 13:33 +0800, Huang, Ying wrote: > Chris Li writes: >=20 > >=20 > > Changes in v2: > > - Add description of the impact of time changing suggest by Ying. > > - Remove create_workqueue() and use schedule_work() > > - Link to v1: https://lore.kernel.org/r/20231221-async-free-v1-1-94b277= 992cb0@kernel.org > > --- > > include/linux/swap_slots.h | 1 + > > mm/swap_slots.c | 29 +++++++++++++++++++++-------- > > 2 files changed, 22 insertions(+), 8 deletions(-) > >=20 > > diff --git a/include/linux/swap_slots.h b/include/linux/swap_slots.h > > index 15adfb8c813a..67bc8fa30d63 100644 > > --- a/include/linux/swap_slots.h > > +++ b/include/linux/swap_slots.h > > @@ -19,6 +19,7 @@ struct swap_slots_cache { > > spinlock_t free_lock; /* protects slots_ret, n_ret */ > > swp_entry_t *slots_ret; > > int n_ret; > > + struct work_struct async_free; > > }; > > =20 > > void disable_swap_slots_cache_lock(void); > > diff --git a/mm/swap_slots.c b/mm/swap_slots.c > > index 0bec1f705f8e..71d344564e55 100644 > > --- a/mm/swap_slots.c > > +++ b/mm/swap_slots.c > > @@ -44,6 +44,7 @@ static DEFINE_MUTEX(swap_slots_cache_mutex); > > static DEFINE_MUTEX(swap_slots_cache_enable_mutex); > > =20 > > static void __drain_swap_slots_cache(unsigned int type); > > +static void swapcache_async_free_entries(struct work_struct *data); > > =20 > > #define use_swap_slot_cache (swap_slot_cache_active && swap_slot_cache= _enabled) > > #define SLOTS_CACHE 0x1 > > @@ -149,6 +150,7 @@ static int alloc_swap_slot_cache(unsigned int cpu) > > spin_lock_init(&cache->free_lock); > > cache->lock_initialized =3D true; > > } > > + INIT_WORK(&cache->async_free, swapcache_async_free_entries); > > cache->nr =3D 0; > > cache->cur =3D 0; > > cache->n_ret =3D 0; > > @@ -269,6 +271,20 @@ static int refill_swap_slots_cache(struct swap_slo= ts_cache *cache) > > return cache->nr; > > } > > =20 > > +static void swapcache_async_free_entries(struct work_struct *data) > > +{ > > + struct swap_slots_cache *cache; > > + > > + cache =3D container_of(data, struct swap_slots_cache, async_free); > > + spin_lock_irq(&cache->free_lock); > > + /* Swap slots cache may be deactivated before acquiring lock */ > > + if (cache->slots_ret) { > > + swapcache_free_entries(cache->slots_ret, cache->n_ret); > > + cache->n_ret =3D 0; > > + } > > + spin_unlock_irq(&cache->free_lock); > > +} > > + > > void free_swap_slot(swp_entry_t entry) > > { > > struct swap_slots_cache *cache; > > @@ -282,17 +298,14 @@ void free_swap_slot(swp_entry_t entry) > > goto direct_free; > > } > > if (cache->n_ret >=3D SWAP_SLOTS_CACHE_SIZE) { > > - /* > > - * Return slots to global pool. > > - * The current swap_map value is SWAP_HAS_CACHE. > > - * Set it to 0 to indicate it is available for > > - * allocation in global pool > > - */ > > - swapcache_free_entries(cache->slots_ret, cache->n_ret); > > - cache->n_ret =3D 0; > > + spin_unlock_irq(&cache->free_lock); > > + schedule_work(&cache->async_free); > > + goto direct_free; > > } > > cache->slots_ret[cache->n_ret++] =3D entry; > > spin_unlock_irq(&cache->free_lock); > > + if (cache->n_ret >=3D SWAP_SLOTS_CACHE_SIZE) > > + schedule_work(&cache->async_free); I have some concerns about the current patch with the change above. We could hit the direct_free path very often. By delaying the freeing of entries in the return cache, we have to do more freeing of swap entry one at a time. When we try to free an entry, we can find the return cache still full, waiting t= o be freed. So we have fewer batch free of swap entries, resulting in an increase in number of sis->lock acquisitions overall. This could have the effect of reducing swap throughput overall when swap is under heavy operations and sis->lock is contended. Tim >=20 > > } else { > > direct_free: > > swapcache_free_entries(&entry, 1); > >=20 > > --- > > base-commit: eacce8189e28717da6f44ee492b7404c636ae0de > > change-id: 20231216-async-free-bef392015432 > >=20 > > Best regards, >=20