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 BE011C61DA4 for ; Wed, 22 Feb 2023 17:38:44 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 55A566B0072; Wed, 22 Feb 2023 12:38:44 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 50A4C6B0073; Wed, 22 Feb 2023 12:38:44 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3D1E36B007D; Wed, 22 Feb 2023 12:38:44 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 2F6F16B0072 for ; Wed, 22 Feb 2023 12:38:44 -0500 (EST) Received: from smtpin16.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 04B9A120CDE for ; Wed, 22 Feb 2023 17:38:43 +0000 (UTC) X-FDA: 80495637768.16.465B5B5 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf30.hostedemail.com (Postfix) with ESMTP id EAEEA8000F for ; Wed, 22 Feb 2023 17:38:40 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=a3wp1FzW; spf=pass (imf30.hostedemail.com: domain of peterx@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1677087521; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=zyXKxJsS3f6jk1qGN9WiAvGHAqOKgKLJ6qX1KIG39lo=; b=459U6ytd/gCYmb92ZvCuUgwYv5ruO7b6esPNEqrrL/XimJn8TkjokL8v8SwX3tKXldnB3Y UnOOT3tuwsQTVhM9p85Ucn9Wcsph8hQ/mDs1i1foyw+KY/zQrjwgOxaUoNHclhFYyM1+YD 9vfIOTGOfTj0QsDuZzxg1kayV87oPUI= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=a3wp1FzW; spf=pass (imf30.hostedemail.com: domain of peterx@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1677087521; a=rsa-sha256; cv=none; b=PIdWhVjf1lOxaDPEVSSMU0ggCmTp++id7nIcha3NPx02VRZfPqIzQaB9eEOZFyfO2oXKU7 win5LaIwHPUKlwi5dxxnPyW3TwezsITQKQ2X48/8HLixUnizGIUPaOXZpuXMVHvTEqTGvI j6ybaKnz+gHdYYRRo9uqPKshQZJ3euI= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1677087520; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=zyXKxJsS3f6jk1qGN9WiAvGHAqOKgKLJ6qX1KIG39lo=; b=a3wp1FzWvQ02ztJF0xTbhbOAuFAuYAWVaWHOR0bjI73Rili2I2hzkkDy7LfG/GXesWRicj g2WWz8A23dB27XVaTNcBsAUiwqPswMB0znXoTJ1AETde8Kxwn7A4QGnQ61npHCHOsJ4R75 CpyuwSAOb8BaRT7vEnFXm42qc8QW0C0= Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-612-fqY_XoVsOGaniQuZLK81_Q-1; Wed, 22 Feb 2023 12:38:38 -0500 X-MC-Unique: fqY_XoVsOGaniQuZLK81_Q-1 Received: by mail-qk1-f200.google.com with SMTP id c9-20020ae9e209000000b0073b344eb74dso3899454qkc.10 for ; Wed, 22 Feb 2023 09:38:38 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677087518; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=xlZAfWTuJXkBs46DXI6mtkWkRzzW2ABpGzcojN9g6uo=; b=rnRAeB4oYconfFFmebK7RA8nn2sWPHocvk129eCj6Y2eZlos5ObHteoq7Fp6BiKUyf tzu6G/cZ8JIaJi1QEd/MPulFuyrnNoneM0A+7jexzcm8xn6zC20ECyN855N8LdoHSAKB 8wwoy7frLQByih5OzabhkmrfjqwDLk6chVISnZ6NGmWZNsLNC7OvZWpOv+hZNpH3Mr0T gLFUpJv8H/qe5kNbwN5SsugKXVxhaSfdZ986SnR0UU8s9zByMqyqyY5gAKd49nAdIa5a p9uflcrds6KY9OafaC6uJzGNrU/UgbA2Ybz87ymtUng2hfFPgw8LHuVwy1OY8t29fFUg ZIpQ== X-Gm-Message-State: AO0yUKUOKiz/uy/Z0c8QrYjSsNciEY+3PWHqUjqtynjYbPt2/RoM9yKJ eKESZ6xbfkv3tP5XhIh7R9NAmQWjq84xMXN01Pxyycwc4m1lKvzxoInUbh1r6qHJv02jc6I50GW 38OKwJBdM2iU= X-Received: by 2002:ac8:58d6:0:b0:3bd:1647:160b with SMTP id u22-20020ac858d6000000b003bd1647160bmr19096804qta.2.1677087518245; Wed, 22 Feb 2023 09:38:38 -0800 (PST) X-Google-Smtp-Source: AK7set8cvK6Lhac8CLf7MjNiS0kkrtU05xWHkW5wbZo6gEjGoD1r7QQMMDIClIyubuIcHlBBqiWx5w== X-Received: by 2002:ac8:58d6:0:b0:3bd:1647:160b with SMTP id u22-20020ac858d6000000b003bd1647160bmr19096769qta.2.1677087517965; Wed, 22 Feb 2023 09:38:37 -0800 (PST) Received: from x1n (bras-base-aurron9127w-grc-56-70-30-145-63.dsl.bell.ca. [70.30.145.63]) by smtp.gmail.com with ESMTPSA id a184-20020a3798c1000000b007423caef02fsm700845qke.122.2023.02.22.09.38.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Feb 2023 09:38:37 -0800 (PST) Date: Wed, 22 Feb 2023 12:38:36 -0500 From: Peter Xu To: Johannes Weiner Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Andrew Morton , Yang Shi , David Stevens , stable@vger.kernel.org Subject: Re: [PATCH] mm/khugepaged: alloc_charge_hpage() take care of mem charge errors Message-ID: References: <20230221214344.609226-1-peterx@redhat.com> MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/mixed; boundary="Vmaj7Dywv+8d4uMP" Content-Disposition: inline X-Rspam-User: X-Rspamd-Server: rspam03 X-Stat-Signature: rde5cnf4rq9uc8xijfugobp4w4q3p8k7 X-Rspamd-Queue-Id: EAEEA8000F X-HE-Tag: 1677087520-34335 X-HE-Meta: U2FsdGVkX18XUSw2wdSzxX8RmV1qqiFUYOzx9Hm+n/5wFWoAc4A3+89Ertl6fi8rnxVBInxVxY/Cq1D2AQILgn2IaB2pdKDWy1IxedWcydFndLETEppSnF8m1Rqvxz5Az1fVwPitldkIEw35bCT8adk+Dzqo8//jsgc+55+CCfYV3E8ZyeAI0A0r9O6n13KJvJ7DCMy+vHsS5HWIXhv/roBGxu420lWZGirJ3ijj/U6K+Ljt+XtruIvOXO3yT7PP9QSaTRYzOYqsEV78jn3HYFFsNvuxEOnJAb6254CW9CuPHLfLNcpG32oRO3PnYcUVcD11rdM6mdZ1fqdirFl9zLjqHRph9BN88eSrtesrG1JXEaxJr6dQtQqwh1LYX3ot6Wi9Gm0JKylRhijfGzLUC2czTu/Ypl0zFXSuv+bvXJoCOFx8SBsyiGVBXKCg8Gp272nYF7MxhpCt8QZXQA/vFPEgeVV6rVbYxurrgxl8O+Yq0lCe2XTcugifqBCYRxmaugTRDJyKHTzqLFJAHxhVB+cRVujiYCweqm3qtdq3w8nAY+BCjYIUpnDcXMsv9mSmtBBkuJw0n0tbFb1wpQhsnyYmmeyqNfJYtiVtfjLReE6BsDrtMxZNrs4mvU1JuMExp2mOpzjCNYtM2HaPsuE6QwcYNqkENfQm4IjvoZTbqbtzoa733QeGGbhfIg6lQWVYYfvCy+xtJNFXtT3Ffpdy8arrU9k0oRL4WcBlTPoj5rEfXs3qxPeq0SMyYCyC4JbhWSW39KA5Ts/MT2Isz5m60l/aKK3vXiECMd7Knx8nx1MgsJFpnvOS71AdKraMFaN5ywpbDEwdEXi/lEAqrvH51Gjhw00YQNE1Q1YZiI3A0/P2xaxlVwL3Vyelg+qpLSUlwM9FCMCJsVqHZIpE8M9MfXcQpRzNsgQtJkK3PYT9MohC26QXIQU/NUFaBY74RVpegRvu3YwT2/SmN260FpZ H7PH5m29 M/NmkZm7LrEvakeQDt1tajQjnr8QjwpEsw8GS2yg1yUJOYBbYULzQP0xycPkHji88C8Abc9/o9hzgGCwOBFNfbDXvG/rKyM+EJjHc1mBEG+awGwdqJE38nSwUTemC9Y9NZMo6thSplJyIlJpgMIO0Ju3OcF36lILeaMw/91gcVoMlkt1h5k5EeH+jlb92oCl3y0BcX4T3NFG8eQiN7MAd5vlHQ2sQrnBCByEXWCu7iL3pI2GNSNGqSRTwZRJFEjeMd9r4fYTt8hDAMhap2CkBVEN9WmiNGJX6jwcyt4UcSyRuWPIKJOODXjfBvsNEydfwr9EW2QgOIPouiBQnIfqMNVZlHR6ob22CXGQk47f0X47sP/sSTX5/JjUU1L2AMKfGdWFzXoCfqsRqu6iVBtptvJmrKLRVXFeYwXAM9a1vIr1+2yM2nVeFKb+3OjsSVBPMj2CydTx9mhUXmSZQmZlvveVJRDhUvshGpoPTJ0tguHUm5nL2LVCmMkEAhONjfrHeqHYng8MiAXxNAxJKyTb/DviaNALW14TkLuAKKlvMP8N8Sx7o2/n9stUneeLxPMit2LwGw2zxABZQlOjspE8JAY2b9FaVtXuzHxU9leFz33sZ+RkIrAdtoPhpGWCRPLrvpdo7FYlVM5iiPFTcBS5/+qLJz3ySIYLnqMpa7oCC5fMIy0LSap0ZfthD/Z4Azi8wR1h1HYJCUudISWsiTojVh+a+ghyhZfZcDcMK 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: --Vmaj7Dywv+8d4uMP Content-Type: text/plain; charset=utf-8 Content-Disposition: inline On Wed, Feb 22, 2023 at 12:06:20PM -0500, Johannes Weiner wrote: > Hello, > > On Tue, Feb 21, 2023 at 04:43:44PM -0500, Peter Xu wrote: > > If memory charge failed, the caller shouldn't call mem_cgroup_uncharge(). > > Let alloc_charge_hpage() handle the error itself and clear hpage properly > > if mem charge fails. > > I'm a bit confused by this patch. > > There isn't anything wrong with calling mem_cgroup_uncharge() on an > uncharged page, functionally. It checks and bails out. Indeed, I didn't really notice there's zero side effect of calling that, sorry. In that case both "Fixes" and "Cc: stable" do not apply. > > It's an unnecessary call of course, but since it's an error path it's > also not a cost issue, either. > > I could see an argument for improving the code, but this is actually > more code, and the caller still has the uncharge-and-put branch anyway > for when the collapse fails later on. > > So I'm not sure I understand the benefit of this change. Yes, the benefit is having a clear interface for alloc_charge_hpage() with no prone to leaking huge page. The patch comes from a review for David's other patch here: https://lore.kernel.org/all/Y%2FU9fBxVJdhxiZ1v@x1n/ I've attached a new version just to reword and remove the inproper tags. Do you think that's acceptable? Thanks, -- Peter Xu --Vmaj7Dywv+8d4uMP Content-Type: text/plain; charset=utf-8 Content-Disposition: attachment; filename="0001-mm-khugepaged-alloc_charge_hpage-take-care-of-mem-ch.patch" >From 0595acbd688b60ff7b2821a073c0fe857a4ae0ee Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Tue, 21 Feb 2023 16:43:44 -0500 Subject: [PATCH] mm/khugepaged: alloc_charge_hpage() take care of mem charge errors If memory charge failed, instead of returning the hpage but with an error, allow the function to cleanup the folio properly, which is normally what a function should do in this case - either return successfully, or return with no side effect of partial runs with an indicated error. This will also avoid the caller calling mem_cgroup_uncharge() unnecessarily with either anon or shmem path (even if it's safe to do so). Cc: Johannes Weiner Cc: Yang Shi Cc: David Stevens Signed-off-by: Peter Xu --- mm/khugepaged.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/mm/khugepaged.c b/mm/khugepaged.c index 8dbc39896811..941d1c7ea910 100644 --- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -1063,12 +1063,19 @@ static int alloc_charge_hpage(struct page **hpage, struct mm_struct *mm, gfp_t gfp = (cc->is_khugepaged ? alloc_hugepage_khugepaged_gfpmask() : GFP_TRANSHUGE); int node = hpage_collapse_find_target_node(cc); + struct folio *folio; if (!hpage_collapse_alloc_page(hpage, gfp, node, &cc->alloc_nmask)) return SCAN_ALLOC_HUGE_PAGE_FAIL; - if (unlikely(mem_cgroup_charge(page_folio(*hpage), mm, gfp))) + + folio = page_folio(*hpage); + if (unlikely(mem_cgroup_charge(folio, mm, gfp))) { + folio_put(folio); + *hpage = NULL; return SCAN_CGROUP_CHARGE_FAIL; + } count_memcg_page_event(*hpage, THP_COLLAPSE_ALLOC); + return SCAN_SUCCEED; } -- 2.39.1 --Vmaj7Dywv+8d4uMP--