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 F1D37D0BB5B for ; Thu, 24 Oct 2024 03:54:56 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 89F6F6B008A; Wed, 23 Oct 2024 23:54:56 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 84F006B0093; Wed, 23 Oct 2024 23:54:56 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 716AD6B0095; Wed, 23 Oct 2024 23:54:56 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 53E1C6B008A for ; Wed, 23 Oct 2024 23:54:56 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 8E4DFC104B for ; Thu, 24 Oct 2024 03:54:36 +0000 (UTC) X-FDA: 82707129246.24.9C0B15A Received: from mail-lj1-f171.google.com (mail-lj1-f171.google.com [209.85.208.171]) by imf28.hostedemail.com (Postfix) with ESMTP id 14981C0013 for ; Thu, 24 Oct 2024 03:54:35 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=MkBHm3R4; spf=pass (imf28.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.171 as permitted sender) smtp.mailfrom=ryncsn@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729742042; 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=07ds5cLkeguJ+l/Qap22eMCzcE6U5yKDcVjgsBgc4+Q=; b=I33y0oAYGukTpnWH1Cpp9eWOwVZ7MkGWm7TI5i/pA/8++/xUysvdeqIs/UeFS9mPQaJmr1 CP6rlZaboiNTstW1K0yj8CEo87vr+VCgGK85k/fUxdm+nCXUeZJhS7ZKDQiPIcz2lyvQq4 E3akkJwjOmQYXh94H6742IOH9zOyajw= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=MkBHm3R4; spf=pass (imf28.hostedemail.com: domain of ryncsn@gmail.com designates 209.85.208.171 as permitted sender) smtp.mailfrom=ryncsn@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729742042; a=rsa-sha256; cv=none; b=nfCsEoa6odpuL8PR8/rnSqmOEcjiA0Wb8qIMKU2DWJMpZh7SsYWP+Pjz1kCSmPg9rsi53m hsgPxrPD1Vi13PE5GFcUZbcdKR4gqntCS7ClAnyvaLOeQyx0wTbS32IAV1c8V9WGEmXENx HUB/E4kSX4xuC2Oy5NPhcKjcrTiLXIs= Received: by mail-lj1-f171.google.com with SMTP id 38308e7fff4ca-2fb584a8f81so3456861fa.3 for ; Wed, 23 Oct 2024 20:54:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729742092; x=1730346892; 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=07ds5cLkeguJ+l/Qap22eMCzcE6U5yKDcVjgsBgc4+Q=; b=MkBHm3R4BAJrSIfb4q3KKomIA5vls/15RboWlXBFEFeSKU1SC66Vy+zlRy5LnzUfUE JAm3NFAUmk8ci1htX12B+F1QasxMTEyLOWRpwZCpye9gjfl+xX8jwVI6SHlBTh/Mkc1g casck2dewuRXaQZn8eIVKoPuqfyzxov+FVGkW4oG0uwkghD4yeh3A1FA2KpApzEXOSoB YOkAcc5PiIwE3MeNlPR7qQyE3LPKu1ca1G4QDuOKkudvR2F03N7feo3IHK/ti8sIdgwr vlEz9XsCxbCtvWbstJsjNof8jjCfGvXTE3/vcqJlnMMbTPryzbOQMI3Sh8LwFVctEn78 IKWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729742092; x=1730346892; 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=07ds5cLkeguJ+l/Qap22eMCzcE6U5yKDcVjgsBgc4+Q=; b=DZZZqcLdZTcftoaHPOfXhpHeQPMgAvlPmGSnpgJBxx+j8nXo1JdM06La4PuLEMA8WI jzLhtDpqpfmqc2n3qBcSQgXl9iPCj1khbOYaN/xORh05IFr3rDkmI3tgnlTTqDtMbBq3 fckWzQHppKwF/cvPMGvNQGUo45bt2g2DVJD/6SFHphd9DDaURijBuk4qxV9LqwV6c60N x/ZjZNJORvSm61EmdKJaxDx0tYESwQ+f68V/mNS2hEs2FK1Q7epcrR4FktNgTbAuTZCP IbC4DxwNGA+DFGuX2YrtiRSyGPdE9KwPn1Ahsi8q6XTtqoh/7h3A6CArEhm4J0H/RlTU S81Q== X-Gm-Message-State: AOJu0YwevOwW+M2h79ki9PNw/6cQLRt+jQqK/itsS5s7EIqEhhbXY1fS Qc6obl92/Bw8FFrvspAq/YuX0b0zoxjo+WyFKga0+UAzLcjBCHEazRaFqGfHqRc9eTaCoJRG/vg WSwrpENIjdkWgyroJtgXCM0m2Hoo6qSI3 X-Google-Smtp-Source: AGHT+IEDuksgHw6m5F/v7Yj194PpG1p8f235x3nUcWMwIJoB9EqfL7mFDsBjLLq4vNjTDkyOXukL6dXtI8AgkxB2W5U= X-Received: by 2002:a2e:be10:0:b0:2fa:c18c:faba with SMTP id 38308e7fff4ca-2fca825b933mr2734891fa.30.1729742092289; Wed, 23 Oct 2024 20:54:52 -0700 (PDT) MIME-Version: 1.0 References: <871q0641xd.fsf@yhuang6-desk2.ccr.corp.intel.com> In-Reply-To: From: Kairui Song Date: Thu, 24 Oct 2024 11:54:36 +0800 Message-ID: Subject: Re: cluster_alloc_swap_entry() may change per-cpu cluster of another CPU randomly To: "Huang, Ying" Cc: linux-mm Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: 5j4cgcd4ij73coxu81medrnzdo1p1f63 X-Rspamd-Queue-Id: 14981C0013 X-Rspamd-Server: rspam11 X-HE-Tag: 1729742075-467932 X-HE-Meta: U2FsdGVkX1/4KY9fxMykKZFhCmUS6UhQrNMoyssOHsQC+ONZMrGvEEniSV/bdJgx5YGuPrz3b7xpJ1O7hrk5DIeywv+10e2Nu0aHWSJAMwfqF2gc3E32/03hAeeqS5Bsyx7k00Wnr+fSrVmQtQvyE2dKSdnPHo387dikTXxJi7YJU2g9T3cu/kgB1vCcYbP0azseh++kCoWEu+tHibkZT5lZxFi+0qXJdv/9vjVOegauSsfcl+lGfdU+RyR9lZxxUTwNVmIa6fkINH71kqk+mdOTdu32pJD4KFG3nP7d8xX166GntDlOUOKn/nqHme14vZEmadL6D3JL1OP0+12xEDMv10ie9FapbgboSyrItqfHE9j2n7ei/AlfyMsAp60qgjrLHuAsgYvaeGAtO1fOXk5cnF/xfueX62YhJhrM7eIp5tQ+PENQsdOfQBxuq1qXLUFKqyqNUGo3ZG0DFRJjTtyn6Y4ifitx0o+4yfYuqD0zclQqQ7ch1rNw5UMhwJwKNM1kaD8JN3ZBRv9J4SGUbhgLRxBT4VhFyV2VFvIDAOUR1g3xhETc76Z5/Xlvv37ZnW4yn/hSRy4TSYnP2wH9uxRdhgtzOCVHRe72AaArVRcl9XX3FCWjiSy8craGbAuuTQYP8H/xwEka11sYRsPzyfbghN/WsfUKN/6AeVD3TJ1cLfPEEInpCeGGkMfyXVMZqvoefVpKelh8Lmm4ob8Av3vVd69AcQ6Xc/m0vulUU2ZUwANGI9p9iawZjOu95eVZRk+SN9cgwfvr3zjwyECiNJogePg97sjRpvp3D8Z2LgXNInVEWQF+/eqZPSdhIJq8hmlVqmi/LTjVrrQ5QnMNUBi+V+EYcv8vCymdonLhjOObkKVtqxp7K9+EvBkd3/biaEOaPVwjuz8UkdMnTCpli3ubJXmzwlmJuovyZeA2oae7CIAqa+K0dLyua5LSnp9SJID2ToQICA5WQEVtJBd 6v9pwiE4 Q8wwVP2PrdM3C3YkQ/n5d5+F7ex0aoXw9kHYZQGZduzlM47bYs10giKurpkmESEJvHceTOZNFjlGyKu82IlZzda1tw+PFMF7uheSscBoITG7HoJ8IGwgFHu087o7u47kIsYflHu4pGus3nFxT7e02M/MrHMkCpAzJtXHyXf64Re86DDsgBK3axacBe7+ER8KwJlQgtpRutRwy4a96jst61TF8owkkgRgrcPQmEgVXfvNe8obyeXZ29/qtSQcrYfq/v3EdTFn/mXPMacXKJmNNgpFNeDnmyiM3jcr32VYhxxhO0zLFZ1sZcx8UxJKYFdMYM6z6MvcenI76HEEY8anzEmf7A8e42PcQCyE3K8e4MZ9L/+d1tCzNxhiFrejqGhCasFzcPkY5NTza1xNlv5phB8c/ibrSBoM1Kz4T X-Bogosity: Ham, tests=bogofilter, spamicity=0.013895, 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, Oct 24, 2024 at 11:50=E2=80=AFAM Kairui Song wro= te: > > On Thu, Oct 24, 2024 at 11:29=E2=80=AFAM Huang, Ying wrote: > > > > Hi, Kairui, > > > > When reading new swap cluster allocation code. Sorry, I didn't find > > time to review your patch at the first place. I found that in > > cluster_alloc_swap_entry(), the following code path is possible, > > > > cluster =3D this_cpu_ptr(si->percpu_cluster); > > offset =3D alloc_swap_scan_cluster(); > > ... > > spin_unlock(&ci->lock); > > spin_unlock(&si->lock); > > /* migrate to another cpu */ > > spin_lock(&ci->lock); > > spin_lock(&si->lock); > > cluster->next[order] =3D offset; > > > > That is, the per cpu cluster of a CPU may be changed on another CPU. I > > guess that this will not cause some functionality issue. However, this > > makes code harder to be reasoned. Is it possible to avoid unlock befor= e > > changing per cpu cluster? Or, give up updating per cpu cluster if we > > need to unlock. > > Hi Ying > > Yes, I've noticed this when working on the new si->lock series. It may > cause fragmentation, only if the CPU the current allocation migrates > from requested another swap allocation, in such case it may waste one > per cpu cluster. > > No functionality or leak issue, but make things harder to reason > indeed, as you mentioned. > > In the new series I used a local lock to prevent migration, so the cpu > is stable during the whole process. However that is not doable unless > si->lock is removed from allocation path or there are lock dependency > issue. > > Do we need to fix this separately in the current tree? The > fragmentation is overall better with these patches, so I think that is Typo: I mean I think these patches are not causing new issues. > causing any issues. Before these patches per CPU cluster can also be > stolen by other CPUs so these were not guaranteed to be accurate. > > One easy way we can do is set the cluster->next[order] to 0 before > scanning for a new cluster to use, and check if it is 0 before setting > next[order] again. > > That may still cause fragmentation as migration will cause extra > clusters to be scanned and used, which is hard to avoid without > changing too much code (like the things being done in the new si->lock > series).