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 A31B4E77170 for ; Wed, 4 Dec 2024 18:57:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1EE096B0082; Wed, 4 Dec 2024 13:57:41 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 175FC6B0083; Wed, 4 Dec 2024 13:57:41 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F31FD6B0085; Wed, 4 Dec 2024 13:57:40 -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 CE7146B0082 for ; Wed, 4 Dec 2024 13:57:40 -0500 (EST) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 735561A1209 for ; Wed, 4 Dec 2024 18:57:40 +0000 (UTC) X-FDA: 82858184430.17.858DAA0 Received: from mail-qk1-f182.google.com (mail-qk1-f182.google.com [209.85.222.182]) by imf04.hostedemail.com (Postfix) with ESMTP id 9DDED40002 for ; Wed, 4 Dec 2024 18:57:22 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ZCRAzAqw; spf=pass (imf04.hostedemail.com: domain of yosryahmed@google.com designates 209.85.222.182 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1733338642; 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=PShsGKOP5A/4kSqaxEWpemov0e3wuEnAMDVyESQ3RB4=; b=6LKOuQ72NimUw7N8practR55lMdGA5oDdQahbeS2yHytskX0f5Va6iPy+G9JCNy68mKGSp qTj40EEKqpnPTvIJMzEDuQ8O/1kihHvCP1A8ZOf+MjWmKCg6SUj0VeWf8hvYI/zA1qnDgl e155ICDC9L0EuBfSAbG59kUYX0RtlRo= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ZCRAzAqw; spf=pass (imf04.hostedemail.com: domain of yosryahmed@google.com designates 209.85.222.182 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733338642; a=rsa-sha256; cv=none; b=OKUkW81c86XmKj764gQO4NNamQLvfygbfRJ4zGJqQVbBXhq8jspIaw3chPmzAtcMIew6zu 5a8gyXZ6F/PTdWtyrk/1aBd9ak6AJLTzH1ExQOAuI2X8Cyr2dhmnUYGH4+orhxcYM/J0T5 561Rh6KcxPtaf1qpkls5b0WM4DepeG8= Received: by mail-qk1-f182.google.com with SMTP id af79cd13be357-7b667e13949so2816485a.3 for ; Wed, 04 Dec 2024 10:57:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1733338658; x=1733943458; 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=PShsGKOP5A/4kSqaxEWpemov0e3wuEnAMDVyESQ3RB4=; b=ZCRAzAqwe9qLSJyBcDNkc/T7dcpIt5hnvWcLUatKEks/U318Y+AjOCba7Wax+IWw1L 35TnTIQtBat9aaYujd4pTKEISX8nWr+0igSTEUvBBIZovlGlvFrD/VGw91Gu9EKj6Wd8 Eo1Zdv4iZlssee8+c9vehqFvHPpjlJosS5BlNmLwPMqKwJrmDjZQ/OxhF3a4K2TnKQqH LQ81bCo7veH3TCxuutaxhaDjHYwsGuHcKEVTjqlAHGRwmy4MrmO79yCE00nN2ZWNLP0y 23bpTuyQLCO6g04xDEqDlwRbI8qjHKASMpXoMZ5S0cKt9RO20+lOg1Gj3ki/dveM32yA xCAg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733338658; x=1733943458; 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=PShsGKOP5A/4kSqaxEWpemov0e3wuEnAMDVyESQ3RB4=; b=NhGVMVq9wCf8P1/eULqV/3e59E/jNoBwIn30ybfZHvoPaP8uDAKhCRRBQTcxV3Se/P ZetQYtL/dXdRT1QJOB12l6lP06E0dmzMgRaT4h7RVvxBc5lsf1d7aQpfKg8op22JsOGw f3qpe0MrF9aXycNBpgtYId4YO5w+8WGk3P7Cgq4xDhrAJMIG1kFM8dE29Ncf6ZBLN2pj +tQkB7RDasA2ZYQ7qpK/nXJ7Fdrp0i3D58hhzYtWFZjZTWz8frVT2umvS4ZUXWdET3eg hKHy98jUUqUP7+xeqEhZUXTeW2fJfLThQ185kjbO8PczKXOMh6Tvvw3XgerwAXLBwSzt Ldnw== X-Forwarded-Encrypted: i=1; AJvYcCXtY6dXVCdcUEPnm+w1A7RmqdEu2wegYclisXHK9JRXpWkqtvQhzPagBp/1RINBjZfCtQueyELb/Q==@kvack.org X-Gm-Message-State: AOJu0Yz9Ijy1ibrF6r8WPbkNLKFGy9++USU4dXzFbYdy7/1CId45kg4w yOR5MOH12aYPuY8vmViT3iEOfYuiBtg/uQRAPtCvuEHjuGItekWfiSFKIBcFY9ER9aEjMFWjFAo IX8pHH2oQxOPjznd0hS/SqWzzxxhUmnmvX5Qu X-Gm-Gg: ASbGnctaTRIR3OFdtuf5T1oZw56p6pTT786M8O4TG+o9AMDodIcSIndNtylI5wLktk1 bi5RI4rZbJXnoTJAB9M9YgP+W5im3 X-Google-Smtp-Source: AGHT+IE0nEyhaK3PznGX7U3n2gojmNdZ8qVyGVv+V0HdgDaiSIR5GA7Vt+Cs1aj0M9chqIf5AMk4w5sj3jZT2vKC7ao= X-Received: by 2002:ad4:5aee:0:b0:6d4:2806:1765 with SMTP id 6a1803df08f44-6d8b740e273mr101125466d6.34.1733338657345; Wed, 04 Dec 2024 10:57:37 -0800 (PST) MIME-Version: 1.0 References: <20241202184154.19321-1-ryncsn@gmail.com> <20241202184154.19321-5-ryncsn@gmail.com> In-Reply-To: From: Yosry Ahmed Date: Wed, 4 Dec 2024 10:57:00 -0800 Message-ID: Subject: Re: [PATCH 4/4] mm, swap_cgroup: remove global swap cgroup lock To: Kairui Song Cc: "Paul E. McKenney" , linux-mm@kvack.org, Andrew Morton , Chris Li , Hugh Dickins , "Huang, Ying" , Roman Gushchin , Shakeel Butt , Johannes Weiner , Barry Song , Michal Hocko , linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 9DDED40002 X-Rspam-User: X-Stat-Signature: r15ayjcfukfrc6az169n5tariscggjtw X-HE-Tag: 1733338642-498919 X-HE-Meta: U2FsdGVkX18jfz2Nu2qniqFoBniW+wfZ/5T6lxaL3g5JsRTKER7plCRinQ943yhh0N5sWSDrlGgxEAuXYCK43TVLdCO08oM81ToTf/ee3y7eG/DFoH43OAxvAurT20tZC2E2Dt7alpcZEjtnkd9URoADE+R4ntAkhRTstjf8Hl8+XlEjTdjRF442ldAK5bJz89qDoA/p9Tdfb1k49/qRrRR2U1IwHLY88du3UsLoFL/o2vZL33TsGAelg1P2AbWE072U2ladGQk19anpezIO1HmzohmQBQj57C496g+aB2noCYjPkeTqp7lRI4D19NNvcw9wrVOoNLjg5bFQHzs8fUWGCMar321ZiKzubozZb5WvFKw+JYnCn+LIUaF9O3aM7PVJ4n315AgagPWJ5rPy4eq0QcnXln1L4zV0mxeJJrCs/bokn1OH46SOqFWqvu4BNq0iitEy+6HKJjyRTkHOV639pdEZQySxnNkL5HOyNfOZL8ndG/w/CrzEP3XLL36M47DrhIGhJDVPkjY29zT/yf30lQ2f4UfLqmC4v+y40XH365z/qYiOLR97Zzk/Mespn45mS6oTfjFtC2Hqagvq9GvLFU1myMRMyHQ61Zc1KozTQMeEbv+aoTv0im/hJdEaDLzB/mrqbJuelhSwvvYdRCE0C5sL/J3gYynngOg9I2lcki6aFIND0+BnXJDyFTZE4rV6IzDGbWI9ISmKIjqFvqeQRW5aCZvnMmobLWXKXAPkXt5HR4B+GfWe5G+r28T1wdjmrdgDaFPg5iM/LLJAN5lqV25UY5Q4KtozUB73lpl9JpfYqLnJc6OqtrzkGJ7Lw9BoFmrb2wc9INSgQKOwAJCyHSAYlUeorQQvdyQeo7CfuP3CiDfdtU3iLTxujU6QmmzQgahLONMqrDxIZHn7HdkeyT+X/6zMIVutkTv4o50egcuZ+UGEvaqUWpnPp3wt/5laPvEuqLgI9KQshJC 9x583BbE 3Q2bEYnEIXLfB5Qsxnb6bBcKYnRAKdzYCMySGI1S5e5x6mbb1Vg3YwQOmFHbo8MfTw1nXC61xkAQC3eUdROU+a3zaazx60PudIKqkjE6lZPkicyoCZN01nvYpcTlJ4TlKKvxftfYWZz9rGw76TOYr/r4ZZNx7OiHUtgSZ1lK8Clv/QCtnY06i8qiLRdfxgiqkfU9SfZ5Ha88O7WMHCNUhyyyUJHE+EUZQPFDNkn7cnKsSbdAnzyBK08mQfHbqXqj1E8w73migwOBo5O8il4+AuLT/PKx+DyK5WuP8onEHJSjo6tAj85dPb3A9tA== 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, Dec 4, 2024 at 9:58=E2=80=AFAM Kairui Song wrote= : > > On Wed, Dec 4, 2024 at 3:18=E2=80=AFAM Yosry Ahmed wrote: > > > > On Tue, Dec 3, 2024 at 10:20=E2=80=AFAM Kairui Song = wrote: > > > > > > On Tue, Dec 3, 2024 at 4:36=E2=80=AFAM Yosry Ahmed wrote: > > > > > > > > On Mon, Dec 2, 2024 at 11:28=E2=80=AFAM Yosry Ahmed wrote: > > > > > > > > > > On Mon, Dec 2, 2024 at 10:42=E2=80=AFAM Kairui Song wrote: > > > > > > > > > > > > From: Kairui Song > > > > > > > > > > > > commit e9e58a4ec3b1 ("memcg: avoid use cmpxchg in swap cgroup m= aintainance") > > > > > > replaced the cmpxchg/xchg with a global irq spinlock because so= me archs > > > > > > doesn't support 2 bytes cmpxchg/xchg. Clearly this won't scale = well. > > > > > > > > > > > > And as commented in swap_cgroup.c, this lock is not needed for = map > > > > > > synchronization. > > > > > > > > > > > > Emulation of 2 bytes cmpxchg/xchg with atomic isn't hard, so im= plement > > > > > > it to get rid of this lock. > > > > > > > > > > > > Testing using 64G brd and build with build kernel with make -j9= 6 in 1.5G > > > > > > memory cgroup using 4k folios showed below improvement (10 test= run): > > > > > > > > > > > > Before this series: > > > > > > Sys time: 10730.08 (stdev 49.030728) > > > > > > Real time: 171.03 (stdev 0.850355) > > > > > > > > > > > > After this commit: > > > > > > Sys time: 9612.24 (stdev 66.310789), -10.42% > > > > > > Real time: 159.78 (stdev 0.577193), -6.57% > > > > > > > > > > > > With 64k folios and 2G memcg: > > > > > > Before this series: > > > > > > Sys time: 7626.77 (stdev 43.545517) > > > > > > Real time: 136.22 (stdev 1.265544) > > > > > > > > > > > > After this commit: > > > > > > Sys time: 6936.03 (stdev 39.996280), -9.06% > > > > > > Real time: 129.65 (stdev 0.880039), -4.82% > > > > > > > > > > > > Sequential swapout of 8G 4k zero folios (24 test run): > > > > > > Before this series: > > > > > > 5461409.12 us (stdev 183957.827084) > > > > > > > > > > > > After this commit: > > > > > > 5420447.26 us (stdev 196419.240317) > > > > > > > > > > > > Sequential swapin of 8G 4k zero folios (24 test run): > > > > > > Before this series: > > > > > > 19736958.916667 us (stdev 189027.246676) > > > > > > > > > > > > After this commit: > > > > > > 19662182.629630 us (stdev 172717.640614) > > > > > > > > > > > > Performance is better or at least not worse for all tests above= . > > > > > > > > > > > > Signed-off-by: Kairui Song > > > > > > --- > > > > > > mm/swap_cgroup.c | 56 +++++++++++++++++++++++++++++++++++-----= -------- > > > > > > 1 file changed, 41 insertions(+), 15 deletions(-) > > > > > > > > > > > > diff --git a/mm/swap_cgroup.c b/mm/swap_cgroup.c > > > > > > index a76afdc3666a..028f5e6be3f0 100644 > > > > > > --- a/mm/swap_cgroup.c > > > > > > +++ b/mm/swap_cgroup.c > > > > > > @@ -5,6 +5,15 @@ > > > > > > > > > > > > #include /* depends on mm.h include */ > > > > > > > > > > > > +#define ID_PER_UNIT (sizeof(atomic_t) / sizeof(unsigned short)= ) > > > > > > +struct swap_cgroup_unit { > > > > > > + union { > > > > > > + int raw; > > > > > > + atomic_t val; > > > > > > + unsigned short __id[ID_PER_UNIT]; > > > > > > + }; > > > > > > +}; > > > > > > > > > > This doubles the size of the per-entry data, right? > > > > > > > > Oh we don't, we just store 2 ids in an int instead of storing each = id > > > > individually. But the question below still stands, can't we just us= e > > > > cmpxchg() directly on the id? > > > > > > Hi Yosry, > > > > > > Last time I checked the xchg status some archs still don't support > > > xchg for 2 bytes, I just found things may have changed slightly but i= t > > > seems at least parisc still doesn't support that. And looking at the > > > code some arches still don't support cmpxchg of 2 bytes today (And I > > > just dropped cmpxchg helper for swap_cgroup so that should be OK). RC= U > > > just dropped one-byte cmpxchg emulation 2 months ago in d4e287d7caff > > > so that area is changing. Lacking such support is exactly the reason > > > why there was a global lock previously, so I think the safe move is > > > just to emulate the operation manually for now? > > > > +Paul E. McKenney > > > > If there's already work to support 2-byte cmpxchg() I'd rather wait > > for that. Alternatively, if it's not too difficult, we should > > generalize this emulation to something like cmpxchg_emu_u8() and add > > the missing arch support. It doesn't feel right to have our own custom > > 2-byte cmpxchg() emulation here. > > Actually here we need 2-byte xchg, not cmpxchg. I'm not exactly sure > if any arch still has anything missing for that support, or is there a > plan to support it for all archs? Not sure to be honest. Taking a step back, with swap_cgroup_cmpxchg() do we still need the synchronization to begin with? It seems like swap_cgroup_record() is the only modifier now, could multiple callers be racing for the same swap slot?