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 6D91EC0218D for ; Wed, 29 Jan 2025 06:40:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BC9E1280020; Wed, 29 Jan 2025 01:40:21 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B79B228001A; Wed, 29 Jan 2025 01:40:21 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A199E280020; Wed, 29 Jan 2025 01:40:21 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 84D0528001A for ; Wed, 29 Jan 2025 01:40:21 -0500 (EST) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id E311BB0670 for ; Wed, 29 Jan 2025 06:40:20 +0000 (UTC) X-FDA: 83059540200.25.C1FAF92 Received: from mail-lf1-f43.google.com (mail-lf1-f43.google.com [209.85.167.43]) by imf27.hostedemail.com (Postfix) with ESMTP id E4A6240004 for ; Wed, 29 Jan 2025 06:40:18 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=kLP+DIls; spf=pass (imf27.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.167.43 as permitted sender) smtp.mailfrom=42.hyeyoo@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=1738132819; 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=6isReLAdoS6aWuC2/rSil8l7f86NbJOTiZ62Ip7Wor4=; b=KyBg2t9p415jaHnWI3LtACTxsii1ujl5PIOrWh0HWAMKty14yle+ibx4YPBSq0kRSB69RD r2VGgDbRInk7FQCZ1urKhYl+rUxeb/Vt8zj9k+n9p4fRZoWg3trdtMR9mLUe5a9ml/zOvv HG7/RIDehUZ0o50h3QlJg8sb3Tmrikc= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=kLP+DIls; spf=pass (imf27.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.167.43 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738132819; a=rsa-sha256; cv=none; b=V2BOdDGU4oXoTIMOpooXOPbghpqmTJ1YCwIc2GwajqWbC3llyfc9E1ujpVh1QTfVgGvqYu qBNmn60ejRpt8zRZ8/Y6Du54d7ENnluXYxQpNcwwEq0+q2wQb0Bw9bvujIknlj8klbPEcI +XDcvNjcZSx7sL1vn6niRbSOTGKmjlE= Received: by mail-lf1-f43.google.com with SMTP id 2adb3069b0e04-53f22fd6887so7043907e87.2 for ; Tue, 28 Jan 2025 22:40:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1738132817; x=1738737617; 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=6isReLAdoS6aWuC2/rSil8l7f86NbJOTiZ62Ip7Wor4=; b=kLP+DIlsl/ktbRKP28DehxQWxaUswEsYHpRp2Xkuefy4k1DxynwTIo+rYwW+6Po57g heQ8LFe6u9fiDX9yZClaGEQsot04TWXPZ6ih2A+NEf//M8jVAYAogODpBH16lfi08/s2 Nchtirt9d9WE+EPGRRjJ5YTMJA81VRq59yiLeOglmyf+2l5fjR1Ti57UA5CqVaeH12Yl 8a89dU34gvgt1bVqVGRLUZjR6Rg8hGty4b1wMC1yRYJN2cX5KgyMf0gwAv8OQq4At5sx X9Ca8jwBbLMfrst+tqMdtJxTNBPJHMM0CUFKHpGSOlwALDpEyzx+ao0BtwBsDRvg0cO3 mS3Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738132817; x=1738737617; 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=6isReLAdoS6aWuC2/rSil8l7f86NbJOTiZ62Ip7Wor4=; b=YvD31+01/LFyzvzkN058QjdRzdFAvyyeLtUBQXmUWJbOcwmoAlIa9764Ovbn2J/oL2 HQfyh5gQy8yvjxD5JnFEgDtQAZ7J6gWDOO1/PlNyasxHcm6U0Dxt8Edm9e/htw7uuBp+ 0Pq7P2/VvrVoqxsgE2mwEdzxJYvimZmB/HXy9j6RRNhM/xFLZpGFwIB8uuPDIJxMIkvv afmKHNDMFr3KPcFcTJFwVJaCaaDc3pSjX+NoU/4qqktPhHpDfYzAKxIMN8PDdxOG4u5L K5j98RQYqPXWw3MirYn8p3GZmgAimHtf3YeGW+Y1hZX6WnrKLvFYD/QeHyE3dJcVSFJl qwbw== X-Forwarded-Encrypted: i=1; AJvYcCVm6Cz7MIBErM9FuR4iM27wMRqVylT10poPydfjwR7gIzHBlhfdLvccLqBcpTnf3T1rb/jQPGEAMg==@kvack.org X-Gm-Message-State: AOJu0YxPeIWV5Mcewkr91pGW4A0PqRD/71yObiof+TVfD+2GeX8c7rZC pJbPijCkA41jCFT62tdQrRB8L8GOiNxVBv52u+66k4912yPVISlfqFPZfAulgQlCqLlZmw5Xqcy /dQ4ZhrNUEaPmYJ5OrZflEIMTguQ= X-Gm-Gg: ASbGncvGF7Os6DjczanShIY2qTDnFR4jey+WqUYGlulGKFFtFTocyASkeG0OrX8p/Bm IV+eD/fwhgWcsOPZntLUXlZWZol2c4lsq+s1Ma5+5gMoRHWs8FVXxxPwCpbHuBF2LRly+jReEuu fmaVwhozlswQ== X-Google-Smtp-Source: AGHT+IHUv39tyL1eOqadyRt1GHW006Tn6NmbmkMSTBUsczjrXgo01QsD/3dSO97xOFc+k9ROweae1TwHCuFzPuZgfTc= X-Received: by 2002:a05:6512:3d92:b0:540:2022:e3b7 with SMTP id 2adb3069b0e04-543e4c3fdc4mr386879e87.53.1738132816871; Tue, 28 Jan 2025 22:40:16 -0800 (PST) MIME-Version: 1.0 References: <20250128185507.2176-1-42.hyeyoo@gmail.com> In-Reply-To: From: Hyeonggon Yoo <42.hyeyoo@gmail.com> Date: Wed, 29 Jan 2025 15:40:04 +0900 X-Gm-Features: AWEUYZnFcyk0wJcOv9GOVR5zQlNeyY4r21oAmDUugcXoq4-ygFcFJ_ateoC4o4I Message-ID: Subject: Re: [PATCH v2 mm-hotfixes] mm/zswap: fix inconsistent charging when zswap_store_page() fails To: "Sridhar, Kanchana P" Cc: Johannes Weiner , Yosry Ahmed , Nhat Pham , Chengming Zhou , Andrew Morton , "linux-mm@kvack.org" , "stable@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: E4A6240004 X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: kyzi7ezkg96c3kmbypy5doroh6aoicwj X-HE-Tag: 1738132818-319804 X-HE-Meta: U2FsdGVkX1+8me/PYDxjoL8k5E4QfLCH60AoDkgrq6g9WhVz9GhkxF9U4k+VURgivmQbB+RTSh6APiZIFix8+vsTACUp3gIGpQAuC7PvNQtpMRfDXZQZIPxVmbpuJE5/tDmXetTdXUeS1jKs9KgKP63VNdpqYpLzddaf+F07FEA4d91FMAIOHflt8ZnLzmEOADy8LWhujp0kupllw6twZxW5bUiYVdSkbNNv9EUwHMOyKDyG4jk6/YB0VBFE99Uxjaq70w60OBrdvhfOopn7dksydO4KdJB1MB2kW8+SjfTO83FYKoClqDgH/+dQWuV/YJqk2Xjoz5JnoGAYWUREmX9RMhRDqvUL6R4xnUTdhhOS44ZDQ9H//opEHCIpo0gubM6sFY/BcJy8Opbk+3uHzlwXhpc2xKYOxFlShNqSliYTCWK+9lf0BAjRMmSpZRjaqA0Q/APRSWoV7ZK4MJaFK0ROx19MWpNUi/ty4M0wDo9Iuf/YAJMLxsbJtxQUOmSJ6cEFm74KNRGicZut03zoff+8kAJb5+wNatlC3fET1HbE8wCccp8ca2MLPuRwNhJGBTnH87qXaTkHtXfJAkxJeuqFNcds2MgOJwRHwQuuGVb5zYmhQuzVX3yK4Nk1TzVcqZ5tQX9lp1jmMhjs54ZXlua2rhCnpzF4XUG+niTpr7CQgwtWI6hGLt3CokeZsEsdIzMi1VtZS796XmpdZnallm/DcdizOTxKtgvFWN6rLTA9sXe0yRykp/WuG6NQg2+cq0YJBp3ne7sMENEZ7wz8diVxxkpFHIrMHv8JFIlKFVomiMLzHN3+S9vX3O+9jl62/stFfBX5j9HnDL6Z5BqNLoGtZlHexnkTpuR34SDUWXPTrIg66jBADYO+/wdFciEXbqnDZ4BJupva0CGE6lbMRSiG9GcGR1DxqyLmGVptJcSsvExBZxArfPtRFQb5Bgggyggxu3jZ36G+kmqZV89 iSYp5Rn2 Vdmam7+FKIOP1HdVkfHssjDw2nY0qFyy23AmHdOx4Q1QfG8vnECc8OpNdD9qdzGu9BuqtloKDbxEta7xtlmYyP7geahJkvu7Rq/bKvwLghEKECSJ/P1P3+PzNYf3lryzFDQ4t3l9EPFjk+b9xG3kLXo0xazeJIGkRw6EVOwTMiFRA9o/+0lgqdLE++ja5MHOVH1ENDwzL3eWevDrdvQ+qHUzA2dFNvk3womwxWvlSLvHIkZyUmQ6pPH/cHX35hsYbhWZKJ17gYu2nVu+5d/O/f8A+wiGycUtRwzjJyVsGGlme1RMjAhWmX3wamsWDEayMKrhoBlCHbaJa3+cAA/4zaRE7jIxzVz1lu8Tgygr4kwUsw75cPsfW51pehESHC/C6xmzjnrcCyCyi3fbATaTRfhQKeO9lEEjjo74dkxxwHjRkR9Zoba/2lybk9eb8spSL+lhTdeOJqyKFH/ZHP/pnJqI1U5qTuekY70g2zCWHH5RIUWNShI9b0UV8QUS/e5TDz3V1kxwOpJcVg0ZORGz6o7UkRroQho+Z8VdJkGumwJhhOFbeyezr0cC0F/zC4tsG70h/I1I/irvxIuCCyGPb9mqkyNIvzDEjGJXK1ibw/icN7VT7d3V9WInpvLm6Ui13ly47p/mUQ28eDz86Cadl9JJcncA4XI2fXtwTeAyVmNfuGyUYcJ5mZqjQolinKrb7NOQTMwRbzXZZFFM= 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 29, 2025 at 4:09=E2=80=AFAM Sridhar, Kanchana P wrote: > > Hi Hyeonggon, > > > -----Original Message----- > > From: Hyeonggon Yoo <42.hyeyoo@gmail.com> > > Sent: Tuesday, January 28, 2025 10:55 AM > > To: Sridhar, Kanchana P ; Johannes Weiner > > ; Yosry Ahmed ; Nhat > > Pham ; Chengming Zhou > > ; Andrew Morton > foundation.org> > > Cc: linux-mm@kvack.org; Hyeonggon Yoo <42.hyeyoo@gmail.com>; > > stable@vger.kernel.org > > Subject: [PATCH v2 mm-hotfixes] mm/zswap: fix inconsistent charging whe= n > > zswap_store_page() fails > > > > Commit b7c0ccdfbafd ("mm: zswap: support large folios in zswap_store()"= ) > > skips charging any zswapped base pages when it failed to zswap the enti= re > > folio. > > > > However, when some base pages are zswapped but it failed to zswap > > the entire folio, the zswap operation is rolled back. > > When freeing zswap entries for those pages, zswap_entry_free() uncharge= s > > the pages that were not previously charged, causing zswap charging to > > become inconsistent. > > > > This inconsistency triggers two warnings with following steps: > > # On a machine with 64GiB of RAM and 36GiB of zswap > > $ stress-ng --bigheap 2 # wait until the OOM-killer kills stress-ng > > $ sudo reboot > > > > Two warnings are: > > in mm/memcontrol.c:163, function obj_cgroup_release(): > > WARN_ON_ONCE(nr_bytes & (PAGE_SIZE - 1)); > > > > in mm/page_counter.c:60, function page_counter_cancel(): > > if (WARN_ONCE(new < 0, "page_counter underflow: %ld > > nr_pages=3D%lu\n", > > new, nr_pages)) > > > > While objcg events should only be accounted for when the entire folio i= s > > zswapped, objcg charging should be performed regardlessly. > > Fix accordingly. > > > > After resolving the inconsistency, these warnings disappear. > > > > Fixes: b7c0ccdfbafd ("mm: zswap: support large folios in zswap_store()"= ) > > Cc: stable@vger.kernel.org > > Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> > > --- > > > > v1->v2: > > > > Fixed objcg events being accounted for on zswap failure. > > > > Fixed the incorrect description. I misunderstood that the base pages a= re > > going to be stored in zswap, but their zswap entries are freed immedia= tely. > > > > Added a comment on why it charges pages that are going to be removed > > from zswap. > > > > mm/zswap.c | 14 ++++++++++---- > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/mm/zswap.c b/mm/zswap.c > > index 6504174fbc6a..10b30ac46deb 100644 > > --- a/mm/zswap.c > > +++ b/mm/zswap.c > > @@ -1568,20 +1568,26 @@ bool zswap_store(struct folio *folio) > > > > bytes =3D zswap_store_page(page, objcg, pool); > > if (bytes < 0) > > - goto put_pool; > > + goto charge_zswap; > > compressed_bytes +=3D bytes; > > } > > > > - if (objcg) { > > - obj_cgroup_charge_zswap(objcg, compressed_bytes); > > + if (objcg) > > count_objcg_events(objcg, ZSWPOUT, nr_pages); > > - } > > > > atomic_long_add(nr_pages, &zswap_stored_pages); > > count_vm_events(ZSWPOUT, nr_pages); > > > > ret =3D true; > > > > +charge_zswap: > > + /* > > + * Charge zswapped pages even when it failed to zswap the entire > > folio, > > + * because zswap_entry_free() will uncharge them anyway. > > + * Otherwise zswap charging will become inconsistent. > > + */ > > + if (objcg) > > + obj_cgroup_charge_zswap(objcg, compressed_bytes); > > Thanks for finding this bug! I am thinking it might make sense to charge > and increment the zswap_stored_pages counter in zswap_store_page(). > Something like: > > diff --git a/mm/zswap.c b/mm/zswap.c > index b84c20d889b1..fd2a72598a8a 100644 > --- a/mm/zswap.c > +++ b/mm/zswap.c > @@ -1504,11 +1504,14 @@ static ssize_t zswap_store_page(struct page *page= , > entry->pool =3D pool; > entry->swpentry =3D page_swpentry; > entry->objcg =3D objcg; > + if (objcg) > + obj_cgroup_charge_zswap(objcg, entry->length); > entry->referenced =3D true; > if (entry->length) { > INIT_LIST_HEAD(&entry->lru); > zswap_lru_add(&zswap_list_lru, entry); > } > + atomic_long_inc(&zswap_stored_pages); > > return entry->length; > > @@ -1526,7 +1529,6 @@ bool zswap_store(struct folio *folio) > struct obj_cgroup *objcg =3D NULL; > struct mem_cgroup *memcg =3D NULL; > struct zswap_pool *pool; > - size_t compressed_bytes =3D 0; > bool ret =3D false; > long index; > > @@ -1569,15 +1571,11 @@ bool zswap_store(struct folio *folio) > bytes =3D zswap_store_page(page, objcg, pool); > if (bytes < 0) > goto put_pool; > - compressed_bytes +=3D bytes; > } > > - if (objcg) { > - obj_cgroup_charge_zswap(objcg, compressed_bytes); > + if (objcg) > count_objcg_events(objcg, ZSWPOUT, nr_pages); > - } > > - atomic_long_add(nr_pages, &zswap_stored_pages); > count_vm_events(ZSWPOUT, nr_pages); > > ret =3D true; Hi Sridhar, It looks much clearer! And we can optimize if it turns out to be worth the complexity. May I ask your permission to add your Signed-off-by: and Co-developed-by: ? I'm afraid to use this without your confirmation due to the Developer's Certificate of Origin. Best, Hyeonggon > > put_pool: > > zswap_pool_put(pool); > > put_objcg: > > -- > > 2.47.1 >