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 902A5C47DAF for ; Thu, 18 Jan 2024 19:24:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 112D36B0083; Thu, 18 Jan 2024 14:24:21 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 09B3F6B0087; Thu, 18 Jan 2024 14:24:21 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E222B6B0093; Thu, 18 Jan 2024 14:24:20 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id C73F16B0083 for ; Thu, 18 Jan 2024 14:24:20 -0500 (EST) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 9539D140CCC for ; Thu, 18 Jan 2024 19:24:20 +0000 (UTC) X-FDA: 81693407880.24.2C42447 Received: from mail-il1-f173.google.com (mail-il1-f173.google.com [209.85.166.173]) by imf14.hostedemail.com (Postfix) with ESMTP id D215D100019 for ; Thu, 18 Jan 2024 19:24:18 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=NxXE2lrZ; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf14.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.166.173 as permitted sender) smtp.mailfrom=nphamcs@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1705605858; 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=HGnQuehifPHb2LKeOYCksa1eWT403+1V1IHhSeK8jRc=; b=cNGTd22bsZbPcLAZutPQfxoD+j98k0+qXuNgKSUehFvGKEvEmge41bng95iUBh/f8eELoz MAlh1aSSDm9PSadXlvnKmtF7eb06AXX6kzPDtdACyAFaNs3r6PTOC7kSnBsodWaGKDmb56 FFHv9OwwFpqP/HWe84nNQpdqxg7y4mA= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=NxXE2lrZ; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf14.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.166.173 as permitted sender) smtp.mailfrom=nphamcs@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1705605858; a=rsa-sha256; cv=none; b=AIKA3WhuEA9YxRy/hxa1URZyUMqFjbhB1WIqi70O3LaSr7CGzFVDFdztNYP+P8o9nQxpKz MTc/JfPl6eCb8CgNIMIuvIlSE8Wdpsmgdb+uv/KJXhIlYtLauP1nCz7MAaG10Pv1JUwpKK 70nGeOZSayaPFYna+0oCxOs4SXQvEJs= Received: by mail-il1-f173.google.com with SMTP id e9e14a558f8ab-3608e206f27so68030015ab.1 for ; Thu, 18 Jan 2024 11:24:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1705605858; x=1706210658; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=HGnQuehifPHb2LKeOYCksa1eWT403+1V1IHhSeK8jRc=; b=NxXE2lrZIcaNyLfPg8mWN879fOcKwCjuXs4dnGJj5cc2ddywNz3wOfyXNHlb10HqZr NQR4ue0nvFtcY5cD8Q6rEdqOMaJAaoYjRJE60sz8ZaCGX3PbqbY729SNbOFoenbvlFPy bGSES2OFXnTlmNIjIfQg2TkwAzrGE1DALjVL7g/5WcEg99V6192PQ7PbXnD+/V07tq9o 6RK4QupSYAGcdf946baP5V5gr9YQQu8SU6Kp06No/YWVcGbARMPYzknEDRWet65eVwqG 4HVTkRY8Gm45IeRsIo+qY1V3h+LZVpEa12bk3qfwoCiKsQOCCc3bxjrX3oE2woklzV1r cLQA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705605858; x=1706210658; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=HGnQuehifPHb2LKeOYCksa1eWT403+1V1IHhSeK8jRc=; b=ngqeJjwXEIssQ4xFyy+6XeoX+qKSAmo76TamvaxlqMUsahwH330N2pua/4C7KT6CA4 8V5i6ObPgobseII+dDfIzpcp4hKpi7qD2Bd3gbcBknTE1bBQ2pzXXBX8m4lfOUkHKKmK yBQi4axM1upmElDEWYNzx0II7LVJQWOxCat+dfF8CrTRoVPl5XGZ0FZEun3VTc1FMub5 lxXwrf1saVlAclwAtC3hCuL/GDIzsJKm85d0S3hEDDMkAsqby7ULiv/E6xBRDGi3zqpN m93qMkmaljmZeuskF34IDn1dioWlb1n5E9cG3IaIK4a2+eMrdJW6H3z86r9q08d9EAcP KCgw== X-Gm-Message-State: AOJu0YweB3akbTW1MSPlXTDVG8SH6E7weMPnj+B154xZX/J8fZczFQz1 3Padkc6bxeLO1itFWP5YII40lxfnzDxDUsADVfaVRjVweUxfrG8Q7gIdWh31gkmWev8CD5PAxs8 U1cr/WFFBpo7SfVPX2xG5myGmigY= X-Google-Smtp-Source: AGHT+IGU4y5prybpkx5WNPp8q91ym/jzy2NlQw2CMtbrrdJoLmdVJpfqwLb8p74VO9DCq4uhLYc9qFH03zzvSFR8sjM= X-Received: by 2002:a92:d143:0:b0:35f:aba9:1031 with SMTP id t3-20020a92d143000000b0035faba91031mr1453854ilg.126.1705605857922; Thu, 18 Jan 2024 11:24:17 -0800 (PST) MIME-Version: 1.0 References: <20240117-b4-zswap-lock-optimize-v1-0-23f6effe5775@bytedance.com> <20240117-b4-zswap-lock-optimize-v1-2-23f6effe5775@bytedance.com> In-Reply-To: <20240117-b4-zswap-lock-optimize-v1-2-23f6effe5775@bytedance.com> From: Nhat Pham Date: Thu, 18 Jan 2024 11:24:06 -0800 Message-ID: Subject: Re: [PATCH 2/2] mm/zswap: split zswap rb-tree To: Chengming Zhou Cc: Andrew Morton , Yosry Ahmed , linux-kernel@vger.kernel.org, Johannes Weiner , linux-mm@kvack.org, Chris Li Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: izbp75w9b1f7axk8otfhz8nbhiearyig X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: D215D100019 X-HE-Tag: 1705605858-626302 X-HE-Meta: U2FsdGVkX1+FiadXezfv87rYhj9OhqbkEQRzQXIpUXYkGw60AEhXOtdn9cNhhUIJgHvC6yDjvivQWBieV9xuoTfTjpuPIOImfcApRuvVE10oXrAURm1Kv9dqtPcnSjZeGOIT2o8k+HujbV+59HkjUPCTtEsPdG3q4EhDmtcREu51/ya7voVmblOLK7hwYfrTHUoqLjeTUhyfoWGXt9yp86PhIrReWlnhJSlX1qWMtjLKRmn9t7Xv5mpxFvXQSqSuKCLdH8/WblvgOGg9jitshr+T45a+IlJZQx2BmCpgtFe53aH9ewfz4r4oN1VZEWTk6SYrlTXimOjeM8KF6MI78x4krULkMDDML+zTSRhMaircUhJCti/jUTKQ7WoMpZbmcuoOTiQQeSioaQJYjPfSGngP8wODkBqk2id9VZu9erjwz1d4MRBkAgO2wENbOEKufVug5pO5KJyZDhZn7JIR7wIzirQj5PkKIxh0cHDkF/F243qG7k92VaAa21ZPLgnkWWEpytrUeHp0i4UNnfTZVv3tgTzeF+Fng7qdBRjJ9YZkW4WfgA7zDiABzs0Op5g0Pm3BXZCdbbp5wGGVKyQ8JBQiiZ3HdAYzswb6cnBvipUBEn0Cpd9/RPf4AseKr7C8YnvnuFc1yLDgqL9Wx/6/7wDngPFXEbksnMM8XLo052hJSoK4rmCwjd0/hK0BoLZKPf//PHlBhfemQ2y1Q9isZkCvj27PDvXiNhnWvXroOBrRNjVo6na3qhnIOz5aWK6mhzcNk3vrFnB/vq+Exb6jvp4+trnySWK26lTq12ChCQgOLHfT1sT/Fdu3VSh4lYqaXKIQFX5Dw08UEx2w0NwwXvUvF39/l0G79+3SQj6ZBPVbZ5X06gfVUdiEA3IvZhWnzX4NmPemQn+cZjA0EnUTeGjSvMDqqVR9oZNKpyhL+oWvAfEDKv88BpVX9G5CAM4xQlch5O3vxtfVS0w4oAM +Yz77quM odTeWHcbuFRB3YoONT2LS5aND/yuJNy2omiSv+Q/ONQ4EMcCRWoa6Cf0xSbuYeDR7OQbXPfMrTjRwl5SJ6HZVTmaPiaTHsWeRNUsk4uFGG+5FZsEnvQ+tyGyRcCoQEoML/ptYZ1Svr3Vxk+DUvSM120agxKCNrwFFOK/Qp4/dsKns7xG7tKJKJZurOP3BuDfNlWKchyHlIKY1ASqWFozxl4m8SX1Aw6GwP9yw/yh3Ejl3GVqB5wZjBb+y2WmPooDX3d0WuxtDilTIl6ETTt+1i0FjD/dFEq3n5qEZCguvrxBVIALWVxnzaULPr+RyN4ZLAlPjOmHiuWUAsub74bwUi6B/GGK17uu2RAK3SlM8aNUgWJ0b5FapCxA+PXlXEZUKAKzIJu3iKNyilpnQQXGuLmAKCUxh5mnI6HNsmR86EvlvMcU761WcWkUTwQ== 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 Wed, Jan 17, 2024 at 1:23=E2=80=AFAM Chengming Zhou wrote: > > Each swapfile has one rb-tree to search the mapping of swp_entry_t to > zswap_entry, that use a spinlock to protect, which can cause heavy lock > contention if multiple tasks zswap_store/load concurrently. > > Optimize the scalability problem by splitting the zswap rb-tree into > multiple rb-trees, each corresponds to SWAP_ADDRESS_SPACE_PAGES (64M), > just like we did in the swap cache address_space splitting. > > Although this method can't solve the spinlock contention completely, it > can mitigate much of that contention. Below is the results of kernel buil= d > in tmpfs with zswap shrinker enabled: > > linux-next zswap-lock-optimize > real 1m9.181s 1m3.820s > user 17m44.036s 17m40.100s > sys 7m37.297s 4m54.622s That's really impressive, especially the sys time reduction :) Well done. > > So there are clearly improvements. > > Signed-off-by: Chengming Zhou Code looks solid too. I haven't read the xarray patch series too closely yet, but this patch series is clearly already an improvement. It is simple, with existing precedent (from swap cache), and experiments show that it works quite well to improve zswap's performance. If the xarray patch proves to be even better, we can always combine it with this approach (a per-range xarray?), or replace it with the xarray. But for now: Acked-by: Nhat Pham > --- > include/linux/zswap.h | 4 +-- > mm/swapfile.c | 2 +- > mm/zswap.c | 69 ++++++++++++++++++++++++++++++++-------------= ------ > 3 files changed, 47 insertions(+), 28 deletions(-) > > diff --git a/include/linux/zswap.h b/include/linux/zswap.h > index eca388229d9a..91895ce1fdbc 100644 > --- a/include/linux/zswap.h > +++ b/include/linux/zswap.h > @@ -30,7 +30,7 @@ struct zswap_lruvec_state { > bool zswap_store(struct folio *folio); > bool zswap_load(struct folio *folio); > void zswap_invalidate(int type, pgoff_t offset); > -int zswap_swapon(int type); > +int zswap_swapon(int type, unsigned long nr_pages); > void zswap_swapoff(int type); > void zswap_memcg_offline_cleanup(struct mem_cgroup *memcg); > void zswap_lruvec_state_init(struct lruvec *lruvec); > @@ -51,7 +51,7 @@ static inline bool zswap_load(struct folio *folio) > } > > static inline void zswap_invalidate(int type, pgoff_t offset) {} > -static inline int zswap_swapon(int type) > +static inline int zswap_swapon(int type, unsigned long nr_pages) > { > return 0; > } > diff --git a/mm/swapfile.c b/mm/swapfile.c > index 6c53ea06626b..35aa17b2a2fa 100644 > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -3164,7 +3164,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specia= lfile, int, swap_flags) > if (error) > goto bad_swap_unlock_inode; > > - error =3D zswap_swapon(p->type); > + error =3D zswap_swapon(p->type, maxpages); > if (error) > goto free_swap_address_space; > > diff --git a/mm/zswap.c b/mm/zswap.c > index d88faea85978..4a6dbc620c7c 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -239,6 +239,7 @@ struct zswap_tree { > }; > > static struct zswap_tree *zswap_trees[MAX_SWAPFILES]; > +static unsigned int nr_zswap_trees[MAX_SWAPFILES]; > > /* RCU-protected iteration */ > static LIST_HEAD(zswap_pools); > @@ -265,6 +266,10 @@ static bool zswap_has_pool; > * helpers and fwd declarations > **********************************/ > > +#define swap_zswap_tree(entry) \ > + (&zswap_trees[swp_type(entry)][swp_offset(entry) \ > + >> SWAP_ADDRESS_SPACE_SHIFT]) > + > #define zswap_pool_debug(msg, p) \ > pr_debug("%s pool %s/%s\n", msg, (p)->tfm_name, \ > zpool_get_type((p)->zpools[0])) > @@ -865,7 +870,7 @@ static enum lru_status shrink_memcg_cb(struct list_he= ad *item, struct list_lru_o > * until the entry is verified to still be alive in the tree. > */ > swpoffset =3D swp_offset(entry->swpentry); > - tree =3D zswap_trees[swp_type(entry->swpentry)]; > + tree =3D swap_zswap_tree(entry->swpentry); > list_lru_isolate(l, item); > /* > * It's safe to drop the lock here because we return either > @@ -1494,10 +1499,9 @@ static void zswap_fill_page(void *ptr, unsigned lo= ng value) > bool zswap_store(struct folio *folio) > { > swp_entry_t swp =3D folio->swap; > - int type =3D swp_type(swp); > pgoff_t offset =3D swp_offset(swp); > struct page *page =3D &folio->page; > - struct zswap_tree *tree =3D zswap_trees[type]; > + struct zswap_tree *tree =3D swap_zswap_tree(swp); > struct zswap_entry *entry, *dupentry; > struct scatterlist input, output; > struct crypto_acomp_ctx *acomp_ctx; > @@ -1569,7 +1573,7 @@ bool zswap_store(struct folio *folio) > src =3D kmap_local_page(page); > if (zswap_is_page_same_filled(src, &value)) { > kunmap_local(src); > - entry->swpentry =3D swp_entry(type, offset); > + entry->swpentry =3D swp; > entry->length =3D 0; > entry->value =3D value; > atomic_inc(&zswap_same_filled_pages); > @@ -1651,7 +1655,7 @@ bool zswap_store(struct folio *folio) > mutex_unlock(&acomp_ctx->mutex); > > /* populate entry */ > - entry->swpentry =3D swp_entry(type, offset); > + entry->swpentry =3D swp; > entry->handle =3D handle; > entry->length =3D dlen; > > @@ -1711,10 +1715,9 @@ bool zswap_store(struct folio *folio) > bool zswap_load(struct folio *folio) > { > swp_entry_t swp =3D folio->swap; > - int type =3D swp_type(swp); > pgoff_t offset =3D swp_offset(swp); > struct page *page =3D &folio->page; > - struct zswap_tree *tree =3D zswap_trees[type]; > + struct zswap_tree *tree =3D swap_zswap_tree(swp); > struct zswap_entry *entry; > u8 *dst; > > @@ -1757,7 +1760,7 @@ bool zswap_load(struct folio *folio) > > void zswap_invalidate(int type, pgoff_t offset) > { > - struct zswap_tree *tree =3D zswap_trees[type]; > + struct zswap_tree *tree =3D swap_zswap_tree(swp_entry(type, offse= t)); > struct zswap_entry *entry; > > /* find */ > @@ -1772,37 +1775,53 @@ void zswap_invalidate(int type, pgoff_t offset) > spin_unlock(&tree->lock); > } > > -int zswap_swapon(int type) > +int zswap_swapon(int type, unsigned long nr_pages) > { > - struct zswap_tree *tree; > + struct zswap_tree *trees, *tree; > + unsigned int nr, i; > > - tree =3D kzalloc(sizeof(*tree), GFP_KERNEL); > - if (!tree) { > + nr =3D DIV_ROUND_UP(nr_pages, SWAP_ADDRESS_SPACE_PAGES); > + trees =3D kvcalloc(nr, sizeof(*tree), GFP_KERNEL); > + if (!trees) { > pr_err("alloc failed, zswap disabled for swap type %d\n",= type); > return -ENOMEM; > } > > - tree->rbroot =3D RB_ROOT; > - spin_lock_init(&tree->lock); > - zswap_trees[type] =3D tree; > + for (i =3D 0; i < nr; i++) { > + tree =3D trees + i; > + tree->rbroot =3D RB_ROOT; > + spin_lock_init(&tree->lock); > + } > + > + nr_zswap_trees[type] =3D nr; > + zswap_trees[type] =3D trees; > return 0; > } > > void zswap_swapoff(int type) > { > - struct zswap_tree *tree =3D zswap_trees[type]; > - struct zswap_entry *entry, *n; > + struct zswap_tree *trees =3D zswap_trees[type]; > + unsigned int i; > > - if (!tree) > + if (!trees) > return; > > - /* walk the tree and free everything */ > - spin_lock(&tree->lock); > - rbtree_postorder_for_each_entry_safe(entry, n, &tree->rbroot, rbn= ode) > - zswap_free_entry(entry); > - tree->rbroot =3D RB_ROOT; > - spin_unlock(&tree->lock); > - kfree(tree); > + for (i =3D 0; i < nr_zswap_trees[type]; i++) { > + struct zswap_tree *tree =3D trees + i; > + struct zswap_entry *entry, *n; > + > + /* walk the tree and free everything */ > + spin_lock(&tree->lock); > + rbtree_postorder_for_each_entry_safe(entry, n, > + &tree->rbroot, > + rbnode) > + zswap_free_entry(entry); > + tree->rbroot =3D RB_ROOT; > + spin_unlock(&tree->lock); > + } > + > + kvfree(trees); > + nr_zswap_trees[type] =3D 0; > zswap_trees[type] =3D NULL; > } > > > -- > b4 0.10.1