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 72976C4332F for ; Wed, 8 Nov 2023 20:20:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E09BF6B0201; Wed, 8 Nov 2023 15:20:07 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DBA446B0202; Wed, 8 Nov 2023 15:20:07 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CA8AB6B0204; Wed, 8 Nov 2023 15:20:07 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id BC93C6B0201 for ; Wed, 8 Nov 2023 15:20:07 -0500 (EST) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 8E31D40D1C for ; Wed, 8 Nov 2023 20:20:07 +0000 (UTC) X-FDA: 81435903654.18.772810C Received: from mail-ej1-f47.google.com (mail-ej1-f47.google.com [209.85.218.47]) by imf25.hostedemail.com (Postfix) with ESMTP id A72EEA001B for ; Wed, 8 Nov 2023 20:20:04 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=3GQaPZsE; spf=pass (imf25.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.47 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=1699474804; 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=FtLYKaLVdpiXryZk2aF3ie1bDJ0QyIbGRi7qd0RZPcs=; b=rUnunpFPeru7hG6c3kLgTroIKOS09FMTQsnA6i7ENYcc9vx1HWHQQS0labGl/5xcM8APuD pEsvWtHE1STKPGASlqxy2Nz5YXzWq5GyKxgvV0rKiKiRrT2ztCp05oExodD/m22ymnZoO9 yX8Fa6kI8HSYvNm4ghxb3muhZPY6Owo= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1699474804; a=rsa-sha256; cv=none; b=aLwxI3y6vXkK2keNFqOGfuQQ400OqDYmolTRdWbNuuf6h8s8s+pAx2stTKjCQv36sEtHot xJESarE27nThr8BmPVVGH06flwsxG1WDQtaA7IuUPi/b6NrGEEARsL9bea6aOKm9m1O3zL sId9BH0fd8S0QgkWCLqaO7Kjs0wbnWs= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=3GQaPZsE; spf=pass (imf25.hostedemail.com: domain of yosryahmed@google.com designates 209.85.218.47 as permitted sender) smtp.mailfrom=yosryahmed@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-ej1-f47.google.com with SMTP id a640c23a62f3a-9d267605ceeso20791666b.2 for ; Wed, 08 Nov 2023 12:20:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1699474803; x=1700079603; 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=FtLYKaLVdpiXryZk2aF3ie1bDJ0QyIbGRi7qd0RZPcs=; b=3GQaPZsEtF3nCWCCohkLkMl/a5h8jdsnTQxtFJmCvVzjWHQaNMnYrYkqC8HmyPWMJp 40mulQqV9Qokjjj+URldqoWIR1R53ZFp7YX7EFKuYIhVHU9QWjVQOoGNUVHB3UVsiZoq DIal/haV7aMDFYHZblFCffx9+TIhL9tU/TNpbCbptu4ntDFKeiBuyzrmuNgTCkxEVEdL kHMQ2BmNQYrcj2QeDOdmLcXeq3K62x2wO8OFDPqzlQwR7tcx/uSnHZfEQTumfyGbJNSD H9sujiEf+kJjoEt+HcgvDIE+4D+bUiGpMswJbZGK5HiJSzUS+e1WuykNeNrpJQ3F8mvF 5jdA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699474803; x=1700079603; 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=FtLYKaLVdpiXryZk2aF3ie1bDJ0QyIbGRi7qd0RZPcs=; b=MM1MzxMAqgHq5FpK7QY5OLUPjlFtHMTbtS8pnJGnHSWSc83VHLNHjJahruImu1PBWV pax/Ytf8plEa/Kru9Lo06/toPS16rRgw4W7zgaVGdiFS5PI95u0TMlnXhCC7Ej0X8l21 WIVH95fkp7Vh1zp69qTIwTyMD13LRRimzPC1Mf8EK9cq8/2D+OB8x0h+t2eYCAgO4C5+ ippVaxTUjzPS/geWx0SzS7alDnQnOlKPsDiSYVJfXo9/+sXkyAc/QpNwDzvkRqcd52u9 m4CEdQ6iCsdT3j/xY+R6vXhR+gULuYRgZyO0ZqZjwA+JLQ+vOpz8kmCfdxGRCr5LUd0P lpxw== X-Gm-Message-State: AOJu0YyY/vcq9iunx7zJreAAIRkbjnTfggQmP9aXzAGYdKMdZUeTf4o9 1WREufOLTs8eeApkalupeKNMDLXGHVdulUSCQ5478A== X-Google-Smtp-Source: AGHT+IHCdfIlDdpR1DaNWL/TEHJsCdP+QHVVPRQ+4hQG1IKuBMgUn8ycyu9oxzLbmezdCYYsrlfuSXUz/zrVlrKqFpc= X-Received: by 2002:a17:906:d553:b0:9e0:5dab:a0ec with SMTP id cr19-20020a170906d55300b009e05daba0ecmr2661945ejc.76.1699474803026; Wed, 08 Nov 2023 12:20:03 -0800 (PST) MIME-Version: 1.0 References: <20231108164920.3401565-1-jackmanb@google.com> In-Reply-To: <20231108164920.3401565-1-jackmanb@google.com> From: Yosry Ahmed Date: Wed, 8 Nov 2023 12:19:23 -0800 Message-ID: Subject: Re: [PATCH v2] mm/page_alloc: Dedupe some memcg uncharging logic To: Brendan Jackman , Roman Gushchin Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Johannes Weiner , Michal Hocko , Shakeel Butt , Muchun Song , Andrew Morton Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: A72EEA001B X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: prs1f1eibksfnri7bdhea8zpmwisctcf X-HE-Tag: 1699474804-536664 X-HE-Meta: U2FsdGVkX18/Bnv/QY35ijzp5kJ4dNdNGs6KQOdenAg9dZ5wJgSlVZmyCsZP+c+wBh0QuLfWByWA2u/vc+wec1Gx6MQsBUhamDXQEjCDxtwQB5hxAEvBsZ/9lU143n/zzGpjTCBswAs4rN51IUVqJBr4rMtURMJvUI0J5fdS1ClH02a5dmMGUJUTcztHP8QUEtK6Tiu1YHdIHpe7zjYZqxju/Hvpqm+7VYg2C0lnD7hTd/Afl6w1CFUNv50Z6MuooRWz+QsmV0MvTx263uscse16FO8HNTSuBqjyGwSTP0m4FWt0+WdrXeGwv3zPZQKJ+p1wqaHMfSbuEwRPGQfqUcw4nJZNXNxHt9ChKJB0dxbBTxEEjkfxxtFHuMz3wRugdQvpRFwntbBzeLVJ6oU6ZXTR2EQEKay/zkGElwLS7hxi5CluaVyuDWiWtll6M64mVKWKwodDAYStLL1Y/UJ99ZLHaTibN8ySI2itau4NLN5MFA+lcOBHfPzS9WblrzQt5wmIJZPpvUuStxXwI3x68i+NjBaRukFkhzsULe30ZH4D0gBVXROPQIL3+3yiB4MKd3P8I+XpoT4GkAWCZy9YnAKggbtZHhB1F3UkvZXQkAgyVBGU2mohKorTWqIIziyHqELkO+z7TJlvVs3q8q0HK78yievc6n0PTEgWO9zbba3u7HN9tkXQKf5m3iaJclCR+pu/6gh5IXoq5bBRqHdPT3/GWp8JYnP3DIsTzOOiNc6cnVnz601qa/6YjqwSsHZDo80ZjJjoUaOLxCoTWGXDtD2KFuJBNfXrgNQ+YTyN+6K1Z8WXkO9XwA2hZVT6xF6fyspo5cS/RV+bhIAHXogxs0Iw/+Lv39EDm2N1m9Qf4cXyyzb3W8cCOCoLd/qxGe0rEAhSRxofG2NohP+N81aATLgi760WHMIFTvAVuirqkriWwyMtH5d47CLyQRqMlfTDp8V+rG6bYLhZTzePMFj sQuiZD1K qh0ohsFMWs44hviCqSdCN28ttEKun08v89FEBGS/2QNfcmug1bMMBaBhX4cQ6PC2LYsVj3gSUf9908D3ff0+fy6kyh/pw2mN8AkNcGUzAc4AdsGNUSs6XH27LfrwTqwrsg/O0tM0sUxfQczmbmMxorhAc/opimgsuaRDL7ISSl4tCG4gZ5uyCip/bt6K12dxnMPD9XoN264PppzP09De8UEEbwRaUo2MTNGyTUFcxTMpD0hoC2QjQjDB6fD1jjMwXi5kkacf0sS5hHDJCsbIdNJkN4OYu4QdxrUf1P11trP0hdQI= 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, Nov 8, 2023 at 8:49=E2=80=AFAM Brendan Jackman wrote: > > The duplication makes it seem like some work is required before > uncharging in the !PageHWPoison case. But it isn't, so we can simplify > the code a little. > > Note the PageMemcgKmem check is redundant, but I've left it in as it > avoids an unnecessary function call. > > Signed-off-by: Brendan Jackman > --- > mm/page_alloc.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 733732e7e0ba..dd5e8a759d27 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -1086,13 +1086,11 @@ static __always_inline bool free_pages_prepare(st= ruct page *page, > trace_mm_page_free(page, order); > kmsan_free_page(page, order); > > + if (memcg_kmem_online() && PageMemcgKmem(page)) > + __memcg_kmem_uncharge_page(page, order); > + > if (unlikely(PageHWPoison(page)) && !order) { > - /* > - * Do not let hwpoison pages hit pcplists/buddy > - * Untie memcg state and reset page's owner > - */ > - if (memcg_kmem_online() && PageMemcgKmem(page)) > - __memcg_kmem_uncharge_page(page, order); > + /* Do not let hwpoison pages hit pcplists/buddy */ > reset_page_owner(page, order); > page_table_check_free(page, order); > return false; > @@ -1123,8 +1121,6 @@ static __always_inline bool free_pages_prepare(stru= ct page *page, > } > if (PageMappingFlags(page)) > page->mapping =3D NULL; > - if (memcg_kmem_online() && PageMemcgKmem(page)) > - __memcg_kmem_uncharge_page(page, order); Nothing happening in the function before this point seems to affect the memcg uncharging. It only acts on the head page, and most of the code up until here is acting on tail pages. LGTM, but I'd be more comfortable if Roman took a look as well. Reviewed-by: Yosry Ahmed > if (is_check_pages_enabled()) { > if (free_page_is_bad(page)) > bad++; > -- > 2.42.0.869.gea05f2083d-goog > >