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 E6AEDC282EC for ; Fri, 7 Mar 2025 02:55:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 13319280005; Thu, 6 Mar 2025 21:55:12 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 0E4D1280001; Thu, 6 Mar 2025 21:55:12 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EC785280005; Thu, 6 Mar 2025 21:55:11 -0500 (EST) 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 CC42B280001 for ; Thu, 6 Mar 2025 21:55:11 -0500 (EST) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id AD06F1C7487 for ; Fri, 7 Mar 2025 02:55:13 +0000 (UTC) X-FDA: 83193238506.03.45BC29E Received: from mail-lf1-f41.google.com (mail-lf1-f41.google.com [209.85.167.41]) by imf02.hostedemail.com (Postfix) with ESMTP id B568880005 for ; Fri, 7 Mar 2025 02:55:11 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=YfB6I4UT; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf02.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.167.41 as permitted sender) smtp.mailfrom=ryncsn@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1741316111; a=rsa-sha256; cv=none; b=pfitrFIv3rmBbiVxSJcsNlmv9X85uOgw23N9KAsUxVDxcN/oXHIEjW7SPxM2a3y9OUXMzy znz2I/yzdMLhJDTmpjeDh+1pRsMQuy748wY5x28CcNa/ehmDqKlidkVey7XdibpW9R1pfh ni0ihW6Rt/FeFM8gqXUIf8Wfwin8xrU= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=YfB6I4UT; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf02.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.167.41 as permitted sender) smtp.mailfrom=ryncsn@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1741316111; 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=VN5pGxv27rMK93LrFT9jrycN+OHSc/LcBOR7eKQ5OKA=; b=shf0tB/gbF1iaEUgK5KrPGeCuqsxPCN8pzbaJUXFa2rkkVH/VXWWhQ0A76iNcIu2X9XCIt 0lB/awHz7dStrhIGTK+qL7ufcrcPpTvbWnjZwpBzjL6RQOTh+E1SXtNo89HL51NfH77O46 covw5TyhhuAIer/kCpMevYZgdksp248= Received: by mail-lf1-f41.google.com with SMTP id 2adb3069b0e04-549901474c3so558994e87.2 for ; Thu, 06 Mar 2025 18:55:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1741316109; x=1741920909; 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=VN5pGxv27rMK93LrFT9jrycN+OHSc/LcBOR7eKQ5OKA=; b=YfB6I4UTESwXz5o98cfTUmXH4x3FKrQbrBcmXas0BTWv4WFaS1HrmssR/6LAQlSUaT lCpvv4NbIdSd1ke0odamsjb40kXvY3aVIQbc3jEa+JcI+fXnQ/5b3upYIANonCZxFrEF xQlc4FoY0mrf9fKxLK3zCDXeuY8yJtVrV4hgf5qs8OR903zz42AqX/NtoQIVdubdwKaf AoKqXwiKM9IQeZj8oMR71+ov9PULbWZ4vTVXFY+7nBv2f5c7V2Z3wzX0bLP8oTJ894XM ewfP6RjsMgrnHdVB1rAPWpU7u0VMXL/U3Q3vQunt+gq6+mE5w7WRt9ZM58g/b/6SYLjU MRvw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741316109; x=1741920909; 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=VN5pGxv27rMK93LrFT9jrycN+OHSc/LcBOR7eKQ5OKA=; b=gstMhFwUd99NIYBnWHyTRFVEO4o+UcEeKlirIoMKIY89mJVXXS5JgaRaN1b4+/5cDq Z47QvQ1SyhpJ7DoyhffQSIs6NmfZDRYVCej0fAq0VzUK+7Cg6dcxIA0OSTTpOXqzrdpX qtMlRBrSCoc61fasmtOnGhl8Qdqj6t6pE/pPB2ITEdXOQ0EhjnKhoRx34J/IVDMZyrXn Tdx/ftSrHRdO35AFMdCPl6Zxptdup1krmcHa61joaHXWuFh2Z9wuUn3mXWafZW6LgTdZ vp6kZOqUIr1Pr8NYSvIHAzWxQwm+fx9WbRz+Uj75x9Eveb4/T3lm3BLGJIPKzNUFVy82 S03A== X-Gm-Message-State: AOJu0YwuznlxbsicpSfsVGeWAQ017bhiWV9HZSBvTzC0YLMIE2SHnkIN czOcxYS4sz0U5/+FEr6Sf25rPmIQyt0HzuCe5eaCxGxErkcG4Vzrss3JDyjDWMAy8OJAK4X3hJ+ EUtnbh/oU4unmsuQDj5rBwp/D/A5UctVK X-Gm-Gg: ASbGncuzE1Zq73UdfU1evKq7mCxcCbLaKZ8THpnPMxBIcVbdOsUEi0x2up+0ikmJymA NxwzbXHFpV5i3uMHQiuOqyCI5wdQn4A24SMHw3XJNG+9b1xIUEFbFHXPADApykVUhhGjIU9a/RK xCoAtmFkOflFz7fWaSXEayq/MOZw== X-Google-Smtp-Source: AGHT+IG1Quu/CAIbmiDVApj6Ycq7SILX5FBBMwib1CJAPLOffS+9Ob9igHMO8fRt+EiDfihBeXa8pKJe2Fs0CRTOnz4= X-Received: by 2002:a05:6512:281b:b0:545:2c40:ec1d with SMTP id 2adb3069b0e04-549910d721dmr515928e87.44.1741316109039; Thu, 06 Mar 2025 18:55:09 -0800 (PST) MIME-Version: 1.0 References: <20250224180212.22802-1-ryncsn@gmail.com> <20250224180212.22802-6-ryncsn@gmail.com> In-Reply-To: <20250224180212.22802-6-ryncsn@gmail.com> From: Kairui Song Date: Fri, 7 Mar 2025 10:54:52 +0800 X-Gm-Features: AQ5f1JpfoeQtaOACR7LCx-R8WU-mOtOoshrIjrK21jHGDTWrf10WFIb8Xaj0XTo Message-ID: Subject: Re: [PATCH v2 5/7] mm, swap: use percpu cluster as allocation fast path To: linux-mm@kvack.org Cc: Andrew Morton , Chris Li , Barry Song , Hugh Dickins , Yosry Ahmed , "Huang, Ying" , Baoquan He , Nhat Pham , Johannes Weiner , Baolin Wang , Kalesh Singh , Matthew Wilcox , linux-kernel@vger.kernel.org, Kairui Song Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: B568880005 X-Stat-Signature: y73gkq1gxwtmrmb85abtc6h6ehxhus7i X-Rspam-User: X-HE-Tag: 1741316111-511871 X-HE-Meta: U2FsdGVkX18SFolReMOCm672k7D+sBYStluLw9qpPSCuOaBz9WR6y0ZMpVXYfm/OMjPPif/zmSo8MZiX744gstHjw0s3cQJNlJZOfhqoP7wjWUa1l3xN4Rehp7QVYY/gJrCpBI+91r8DZrZYQ37ywRlYzZ3D/9eQvLGS35RVkigGfoanJjGlJw1kdSFkhW4c9Bn2rmjd6zm3yoZZy81ZgMNmQ2J3UZv+nBLTb0nsiNzFUW0l5L235+IRdGJ1z8vGvSWkBCW3/NDQw70LzApPJ6jtKkjbBpOSDxArswAwemg+oiydVPTGBi76KvkHseSGE8hfr70Jqmowd7B3R59//o2A8zvsiVW1cX5HRY7G5IE8ky8METPaogZAUDTq1qkYxpjIiITeGew4PkZF6vebRFn9vqS66bOM+Z4+tZlxVx8Gt/Irn1Nq293XNm4eVerpLgQ4bixi2mmKckmjdog4veaxJjCOWDO8DAnZYXR7PBzm52ni5NQbYsFjOqAJCGGBgEyPIkolklTDxMQ7dy8fQDrdK2seozRyNq+B/PXe+rsOYhXM3eszypGKIUVMGyGBtUdUlLsxQFbfG75oceKc3yAEUUj7KqhnKAOExkZ5vVoWo/0Xv2uIbsmQ9q3/d8qsxm7OE0MaJx4HmB6MSb5emjBOSUTNmzJ7/Ce69DdPpL+EpQsZd/+Ocan7bw8FR4p/WqkEQ7KUgOpVLMxQZhB82awBtjMwRF7/z3VJt8Vf6AW+vE+ufNElGwIJkyOEYWI38OOQhOLeilTOPwFPjlxcYL3DqsC6+ZiNyqtgTFFLfj1GbO8IZyqjxOoXhlWY5LCiUnatChnHIBclVQB+ZVYNuXyENIfJ+94iMmGFcNn6+6aZykq43Y9S5Ws1omjnWYlNdcZoKKsWY8Uxz+vLO3Rs931WP42esvpDvigVsuoSep3DntxhmYy5sJPaT17RDZ0tHV2g3h3P1OicmBhyKgk rDxd0lAR FvgwC5Mx3oaVW8w039cpWNzNOScNbWAho3MoQ6Tc22x5zvBHXBybK2U0MRpuzDpqYNRXagGpNc7yUZApY44eQoG7h7HjrRqm81pjbaeTCNtkL8URipuLz4tVtlzw0vgrqDULizOQYRXw6lWblBYeYLHKNid/8m61K0/OS5PJBB+K+WtqfJf9nuhTSt7mj/moHx1r5s3XhskePub1QNUCSHmiARS3cbh2rnEWcsEINWaqIVTZBreLraWSnqy4X69kTz4+89LEpFFywnFh8XmhGiD96BLQnE7PsX6B2UBWYBrXamo3ABZ9RRH85RvfQASsplaq701Kxtmf14M3+5ueooPguKgJGBOV3ooDYnsa0fkxHQCr3cG5GP7aalw== 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 Tue, Feb 25, 2025 at 2:03=E2=80=AFAM Kairui Song wrot= e: > > From: Kairui Song > > Current allocation workflow first traverses the plist with a global lock > held, after choosing a device, it uses the percpu cluster on that swap > device. This commit moves the percpu cluster variable out of being tied > to individual swap devices, making it a global percpu variable, and will > be used directly for allocation as a fast path. > > The global percpu cluster variable will never point to a HDD device, and > allocations on a HDD device are still globally serialized. > > This improves the allocator performance and prepares for removal of the > slot cache in later commits. There shouldn't be much observable behavior > change, except one thing: this changes how swap device allocation > rotation works. > > Currently, each allocation will rotate the plist, and because of the > existence of slot cache (one order 0 allocation usually returns 64 > entries), swap devices of the same priority are rotated for every 64 > order 0 entries consumed. High order allocations are different, they > will bypass the slot cache, and so swap device is rotated for every > 16K, 32K, or up to 2M allocation. > > The rotation rule was never clearly defined or documented, it was changed > several times without mentioning. > > After this commit, and once slot cache is gone in later commits, swap > device rotation will happen for every consumed cluster. Ideally non-HDD > devices will be rotated if 2M space has been consumed for each order. > Fragmented clusters will rotate the device faster, which seems OK. > HDD devices is rotated for every allocation regardless of the allocation > order, which should be OK too and trivial. > > This commit also slightly changes allocation behaviour for slot cache. > The new added cluster allocation fast path may allocate entries from > different device to the slot cache, this is not observable from user > space, only impact performance very slightly, and slot cache will be > just gone in next commit, so this can be ignored. > > Signed-off-by: Kairui Song > --- > include/linux/swap.h | 11 ++-- > mm/swapfile.c | 136 +++++++++++++++++++++++++++++-------------- > 2 files changed, 95 insertions(+), 52 deletions(-) > > diff --git a/include/linux/swap.h b/include/linux/swap.h > index 2fe91c293636..374bffc87427 100644 > --- a/include/linux/swap.h > +++ b/include/linux/swap.h > @@ -284,12 +284,10 @@ enum swap_cluster_flags { > #endif > > /* > - * We assign a cluster to each CPU, so each CPU can allocate swap entry = from > - * its own cluster and swapout sequentially. The purpose is to optimize = swapout > - * throughput. > + * We keep using same cluster for rotational device so IO will be sequen= tial. > + * The purpose is to optimize SWAP throughput on these device. > */ > -struct percpu_cluster { > - local_lock_t lock; /* Protect the percpu_cluster above */ > +struct swap_sequential_cluster { > unsigned int next[SWAP_NR_ORDERS]; /* Likely next allocation offs= et */ > }; > > @@ -315,8 +313,7 @@ struct swap_info_struct { > atomic_long_t frag_cluster_nr[SWAP_NR_ORDERS]; > unsigned int pages; /* total of usable pages of swap = */ > atomic_long_t inuse_pages; /* number of those currently in u= se */ > - struct percpu_cluster __percpu *percpu_cluster; /* per cpu's swap= location */ > - struct percpu_cluster *global_cluster; /* Use one global cluster = for rotating device */ > + struct swap_sequential_cluster *global_cluster; /* Use one global= cluster for rotating device */ > spinlock_t global_cluster_lock; /* Serialize usage of global clus= ter */ > struct rb_root swap_extent_root;/* root of the swap extent rbtree= */ > struct block_device *bdev; /* swap device or bdev of swap fi= le */ > diff --git a/mm/swapfile.c b/mm/swapfile.c > index db836670c334..7caaaea95408 100644 > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -116,6 +116,18 @@ static atomic_t proc_poll_event =3D ATOMIC_INIT(0); > > atomic_t nr_rotate_swap =3D ATOMIC_INIT(0); > > +struct percpu_swap_cluster { > + struct swap_info_struct *si[SWAP_NR_ORDERS]; > + unsigned long offset[SWAP_NR_ORDERS]; > + local_lock_t lock; > +}; > + > +static DEFINE_PER_CPU(struct percpu_swap_cluster, percpu_swap_cluster) = =3D { > + .si =3D { NULL }, > + .offset =3D { SWAP_ENTRY_INVALID }, > + .lock =3D INIT_LOCAL_LOCK(), > +}; > + > static struct swap_info_struct *swap_type_to_swap_info(int type) > { > if (type >=3D MAX_SWAPFILES) > @@ -539,7 +551,7 @@ static bool swap_do_scheduled_discard(struct swap_inf= o_struct *si) > ci =3D list_first_entry(&si->discard_clusters, struct swa= p_cluster_info, list); > /* > * Delete the cluster from list to prepare for discard, b= ut keep > - * the CLUSTER_FLAG_DISCARD flag, there could be percpu_c= luster > + * the CLUSTER_FLAG_DISCARD flag, percpu_swap_cluster cou= ld be > * pointing to it, or ran into by relocate_cluster. > */ > list_del(&ci->list); > @@ -805,10 +817,12 @@ static unsigned int alloc_swap_scan_cluster(struct = swap_info_struct *si, > out: > relocate_cluster(si, ci); > unlock_cluster(ci); > - if (si->flags & SWP_SOLIDSTATE) > - __this_cpu_write(si->percpu_cluster->next[order], next); > - else > + if (si->flags & SWP_SOLIDSTATE) { > + __this_cpu_write(percpu_swap_cluster.si[order], si); > + __this_cpu_write(percpu_swap_cluster.offset[order], next)= ; > + } else { > si->global_cluster->next[order] =3D next; > + } > return found; > } > > @@ -862,9 +876,8 @@ static void swap_reclaim_work(struct work_struct *wor= k) > } > > /* > - * Try to get swap entries with specified order from current cpu's swap = entry > - * pool (a cluster). This might involve allocating a new cluster for cur= rent CPU > - * too. > + * Try to allocate swap entries with specified order and try set a new > + * cluster for current CPU too. > */ > static unsigned long cluster_alloc_swap_entry(struct swap_info_struct *s= i, int order, > unsigned char usage) > @@ -872,18 +885,12 @@ static unsigned long cluster_alloc_swap_entry(struc= t swap_info_struct *si, int o > struct swap_cluster_info *ci; > unsigned int offset, found =3D 0; > > - if (si->flags & SWP_SOLIDSTATE) { > - /* Fast path using per CPU cluster */ > - local_lock(&si->percpu_cluster->lock); > - offset =3D __this_cpu_read(si->percpu_cluster->next[order= ]); > - } else { > + if (!(si->flags & SWP_SOLIDSTATE)) { > /* Serialize HDD SWAP allocation for each device. */ > spin_lock(&si->global_cluster_lock); > offset =3D si->global_cluster->next[order]; > - } > - > - if (offset) { > ci =3D lock_cluster(si, offset); > + > /* Cluster could have been used by another order */ > if (cluster_is_usable(ci, order)) { > if (cluster_is_empty(ci)) > @@ -973,9 +980,7 @@ static unsigned long cluster_alloc_swap_entry(struct = swap_info_struct *si, int o > } > } > done: > - if (si->flags & SWP_SOLIDSTATE) > - local_unlock(&si->percpu_cluster->lock); > - else > + if (!(si->flags & SWP_SOLIDSTATE)) > spin_unlock(&si->global_cluster_lock); > return found; > } > @@ -1196,6 +1201,49 @@ static bool get_swap_device_info(struct swap_info_= struct *si) > return true; > } > > +/* > + * Fast path try to get swap entries with specified order from current > + * CPU's swap entry pool (a cluster). > + */ > +static int swap_alloc_fast(swp_entry_t entries[], > + unsigned char usage, > + int order, int n_goal) > +{ > + struct swap_cluster_info *ci; > + struct swap_info_struct *si; > + unsigned int offset, found; > + int n_ret =3D 0; > + > + n_goal =3D min(n_goal, SWAP_BATCH); > + > + /* > + * Once allocated, swap_info_struct will never be completely free= d, > + * so checking it's liveness by get_swap_device_info is enough. > + */ > + si =3D __this_cpu_read(percpu_swap_cluster.si[order]); > + offset =3D __this_cpu_read(percpu_swap_cluster.offset[order]); > + if (!si || !offset || !get_swap_device_info(si)) > + return 0; Found one issue with this intermediate patch, "si" will be reused upon swapoff & swapon again. So after the reuse, get_swap_device_info() returns true and `offset` is not 0, but `offset` is invalid, it could point to a value larger than si->max if the previous device is larger. To fix it, need to squash this onto it: (also include a code fix for cluster_is_usable check, this code error is fixed by 7/7 but shouldn't be here in the first place) (replacing __this_cpu_xx with this_cpu_xx because the swapoff flush will access other CPU's variable now) diff --git a/mm/swapfile.c b/mm/swapfile.c index 7caaaea95408..68b40e74be93 100644 --- a/mm/swapfile.c +++ b/mm/swapfile.c @@ -818,8 +818,8 @@ static unsigned int alloc_swap_scan_cluster(struct swap_info_struct *si, relocate_cluster(si, ci); unlock_cluster(ci); if (si->flags & SWP_SOLIDSTATE) { - __this_cpu_write(percpu_swap_cluster.si[order], si); - __this_cpu_write(percpu_swap_cluster.offset[order], next); + this_cpu_write(percpu_swap_cluster.si[order], si); + this_cpu_write(percpu_swap_cluster.offset[order], next); } else { si->global_cluster->next[order] =3D next; } @@ -1220,15 +1220,17 @@ static int swap_alloc_fast(swp_entry_t entries[], * Once allocated, swap_info_struct will never be completely freed, * so checking it's liveness by get_swap_device_info is enough. */ - si =3D __this_cpu_read(percpu_swap_cluster.si[order]); - offset =3D __this_cpu_read(percpu_swap_cluster.offset[order]); + si =3D this_cpu_read(percpu_swap_cluster.si[order]); + offset =3D this_cpu_read(percpu_swap_cluster.offset[order]); if (!si || !offset || !get_swap_device_info(si)) return 0; while (offset) { ci =3D lock_cluster(si, offset); - if (!cluster_is_usable(ci, order)) + if (!cluster_is_usable(ci, order)) { + unlock_cluster(ci); break; + } if (cluster_is_empty(ci)) offset =3D cluster_offset(si, ci); found =3D alloc_swap_scan_cluster(si, ci, offset, order, us= age); @@ -1237,7 +1239,7 @@ static int swap_alloc_fast(swp_entry_t entries[], entries[n_ret++] =3D swp_entry(si->type, found); if (n_ret =3D=3D n_goal) break; - offset =3D __this_cpu_read(percpu_swap_cluster.offset[order= ]); + offset =3D this_cpu_read(percpu_swap_cluster.offset[order])= ; } put_swap_device(si); @@ -2660,6 +2662,27 @@ static void wait_for_allocation(struct swap_info_struct *si) } } +/* + * Called after swap devices reference count is dead, so + * neither scan or allocation will go into it. + */ +static void flush_percpu_swap_cluster(struct swap_info_struct *si) +{ + int cpu; + struct swap_info_struct **pcp_si; + + for_each_possible_cpu(cpu) { + pcp_si =3D per_cpu_ptr(percpu_swap_cluster.si, cpu); + /* + * Invalidate the percpu swap cluster, si->users + * is dead, so no new users will point to it, flush any + * existing cache is enough. + */ + cmpxchg(pcp_si, si, NULL); + } +} + + SYSCALL_DEFINE1(swapoff, const char __user *, specialfile) { struct swap_info_struct *p =3D NULL; @@ -2761,6 +2784,7 @@ SYSCALL_DEFINE1(swapoff, const char __user *, special= file) flush_work(&p->discard_work); flush_work(&p->reclaim_work); + flush_percpu_swap_cluster(p); destroy_swap_extents(p); if (p->flags & SWP_CONTINUED) --- There will be minor conflict in the next commit after this squash, I will send a V3 shortly to resolve this if there is no objection on this.