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 4A4ADC64EC7 for ; Wed, 22 Feb 2023 19:22:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 81CF26B0071; Wed, 22 Feb 2023 14:22:32 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7CC0F6B0072; Wed, 22 Feb 2023 14:22:32 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6BA9D6B0073; Wed, 22 Feb 2023 14:22:32 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 5BEB26B0071 for ; Wed, 22 Feb 2023 14:22:32 -0500 (EST) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 234921A0AD0 for ; Wed, 22 Feb 2023 19:22:32 +0000 (UTC) X-FDA: 80495899344.30.B0A2FC2 Received: from mail-qv1-f41.google.com (mail-qv1-f41.google.com [209.85.219.41]) by imf27.hostedemail.com (Postfix) with ESMTP id 4463D40007 for ; Wed, 22 Feb 2023 19:22:29 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=cmpxchg-org.20210112.gappssmtp.com header.s=20210112 header.b=sqJ32Rif; spf=pass (imf27.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.41 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1677093749; a=rsa-sha256; cv=none; b=ZPXfmhxH8Rt5KzQsSrqvtEvQB8CTFGpZleJbBfWY//GBQaSztHy6HLdyt+RlvsjQ/B/YBx nsv3wLvQsZ9dJJ3nA+KPaJyXZKi6MNIrxyqYasO5baNL0k0k2DXvYTKvqTUXJzxqAZpL1S 6eIY3kksknu5S2jIM+xYHBDkPRnCHZ4= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=cmpxchg-org.20210112.gappssmtp.com header.s=20210112 header.b=sqJ32Rif; spf=pass (imf27.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.41 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1677093749; 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=NJWYEAAsYNz+CjPq0GlL9zUnfQnB8A7P/QP84KCSNwY=; b=yROHstuiAsD+rbcAJkIcIH5eeIG/Ec0yeDKK6tb3Fq2H4x5UzsCa3XqzJ4moLIw8nG5b7I 8cr/uv3G/3YB+fPXNrNi3Hr/su1htZrNq837JQOz1ms5m7vDDAIcwp5sEOgRO7aq0QwQUh tZ6xuZNdU+Ti6YlpWu/4G6MFxlJTDR8= Received: by mail-qv1-f41.google.com with SMTP id nv15so9532502qvb.7 for ; Wed, 22 Feb 2023 11:22:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20210112.gappssmtp.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=NJWYEAAsYNz+CjPq0GlL9zUnfQnB8A7P/QP84KCSNwY=; b=sqJ32RifrlWeoIMvFm2CCDN5Ubwhfnij9NOlKQtXmSuwFgR4ek1qWC6VC0HpIBzPiW 9QhLxHISdA9CtLJ3vBwIevz760Xv1kRjTI5fDVeMgFI9y+LUfp0QmJZfS80SFpsjAgdM yYQreO2aksWfZFS2Y2hWoK4NBRD36gK1FNuF6kiRaxh4qBQkwtG4V0UQrczA8rlcx7N1 9BvJyalK7yYljMBU2U0MVgqEbEaMycTtcBujW4J93VqPHHIk2USVPWTQJanvJ8DmcA8i Hj1Ej4vNAXwTuRmAhidvbOHLK10U18MQfMJ9q3wPF1zm9hnz6GT/7IaqINKvg34kHYIp hIpA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=NJWYEAAsYNz+CjPq0GlL9zUnfQnB8A7P/QP84KCSNwY=; b=IVb8gcUDbw9omf1rXjm1DqE5a0poAUUM2ARjWmToEalPgTbAGB7q6aULzZUjzWSl1G CdGsYHnBUBz5aWRQoud3FkSt9CHknJiXFkI5eTTSwWmN2XhmJB0n5JzOkeXu2to2YQWI WeatXIHJQ2SUBxNQIDoOG/q9F/Rg+WtlprkxN5WfjG3Pxhn9kNnpvRA+jFq9XTcMdJAG 6q6DJRCTuao/+o9052TkVY9Px3hPktNkhmdkE6NCnF5OMf5oL7spdsuJcg48ZGQiYEeY VO5O9t45vvwuRiba5nZbPkx+cKnGDh3/D/tq7WCIdNYsDydpDQAAkYJVgrwTgus2mAdm ECTA== X-Gm-Message-State: AO0yUKUxqeNP7qA1HVcP+Oj8UbZhRgq5glSzRdhIhX6KTu/yWaSRWHmU oJnb5QenfRXhj+YTIDNPaIM87A== X-Google-Smtp-Source: AK7set9cYkEvkHFxOBz6FdfILUmIQjk+AAn5rEGd6MznXpR1z8qxvIBtI3T8ya5Tdlkd9UzITmOdVQ== X-Received: by 2002:a05:6214:4118:b0:56f:52ba:ccea with SMTP id kc24-20020a056214411800b0056f52bacceamr17150614qvb.20.1677093748368; Wed, 22 Feb 2023 11:22:28 -0800 (PST) Received: from localhost ([2620:10d:c091:480::1:5e17]) by smtp.gmail.com with ESMTPSA id 11-20020a37060b000000b00741921f3f60sm3748694qkg.42.2023.02.22.11.22.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 22 Feb 2023 11:22:28 -0800 (PST) Date: Wed, 22 Feb 2023 14:22:27 -0500 From: Johannes Weiner To: Peter Xu 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 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Queue-Id: 4463D40007 X-Rspamd-Server: rspam01 X-Stat-Signature: 1h3j98k5ruajint7thrdh358pjkyqacx X-HE-Tag: 1677093749-578127 X-HE-Meta: U2FsdGVkX19q/GAObyhgAJ3A1DjREyJXPdN5uwluLRObH+xtAT6SHmllt/+AZPEqWiBzatthmx09lqkLl1nE0m0YgHUcGBCun+5eXVJHNF6RQo/hA/lBUzubnk/yfI5MnCe78w9UB+sKJWiZaG0RMgy1JKs5jE5pJix5OLqQX9vEiUR9ENP9LM+buwflfQ+7We+bFvLdoChXMGzQESy4B5iEtweF+e0CQeVyeZ1Mmuq7oIrjJNv7WVbghi+s2YUCV3/9bH4iCVMvLFp/Ogw2vNo/Y7l9He7P5ya0Dpolvw412K6eCO/d7ETtoZwet4oMrcyTr74Cy2riFqB0R3lZmFtKaV+TTjGcq6KuLCLyW0huGF6t9JJmrKsHUeH4IJJcvd9TGylTOLpWekXHUnEHDWh/W5xP/XXllndD3AIRx7n6ostTdmaY2zWSnF7q4avAaWWk043ooxWB6cvVUd9QDGG6kxKX6z8144sMssT2UdWOuHkWmkM5xfO7+nw7XTp20R8fBHbVyDDTewtTgchtG0zgRyy5O2cAMRK/F8/tEYcmhbcLO5ITfXVg76gzBnDhGGnObR2qs/yCYF22s8Qr6yamk7FNE/BxmuPhs2DYOzqwNvXqOxUgrEZRzY9m5jdaTsRQivBYeyckvFTZojjs7PEgoRKELpbZYgNftMIIsYVEtj2ntnDifDhyu0AuwYullw7ONG3sqH74mRYxFntZKx9iVM2odqdLrk5hIjJO988/PTYOOJyrvuHD22gYrZqY19WC/CRo8fM09j+HFAm6sa8+M6zS1AXHYJTIhhlHlvjW/daNWzWuzRydev+m6Wji3JdNcOBOJr0BgYdSfZW7T3i5EuDu4iY1fBRLFg6Ovzteuj02NyV8dLVn+Uc928AiL9v/gBXLD6fU6+bI5AGVhhMQRnLI/JS+IYjFq2LrnrF1/wIKAR6+qLKXR/q7sCANxksA44V/YLmt93YpzO+ 0/XaTFiS 8AOMrwlJUPCqGUwmbDb0c57yvEjwK5e9CjOZwktlAM7bYdEICC/3x7s7xheFuXJo6HJOBe7j6BckmzxqecKa6wKbM3n/nEzBjQGrNEyuDQpqarpizJ5XuiaBdkV4HKfOWZQHTKyTEH218AOg3FyWflA78Y2cZV8aT7T10Hyy8vs4EVID2h0VbeAJh9+llMrVf06Lse1oNbaLH24lJFVHQ0NxlZ1xcZq3KmUIsWja1AoXCL7iwBuiGALl6A40wm3cRcIfcnYD+yHwg6qMnkTAPNDB8rsc6UjcNq3x7Pq5QTMOS7DGgWxGe9xdUEM1sZoe8udkOTBgVlkYrBP83ie5E/u8Jqo7tR2gDHD1x6qtGmaqnxlP2UqFUSyieryr85w+L3UuIAKUgL0NhsUcTbfv4Y8JbjlrwVj7JuqrTo6IKOoJ3AXcG07VAxcX1J8PAgdvE2XNErL0VDkKgLzup/OzZGt5LUOzEQ5kLW7bUXaJj/F1KyJ6gwzxX/gy/dKyQenODUmmzYnl8zBw6s5A/NKAdsIRLleBPoHOmb/fn/o5eMmDuGvh2tRB3rUgIlVcTgMUr0J/TxSpn7veiAGcOV4+Wug3ElhvlTg0RIFaoVNH10XxIBHLEOO5RNP3F7brsFv+i1/Q4V6VJhqhmO9uOqBqceu3QQEHh09/4qmCJnEJbJucrHhQpWe4v6fmZQw== 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: On Wed, Feb 22, 2023 at 12:38:36PM -0500, Peter Xu wrote: > 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/ Ah, that makes sense. Thanks for the context. > 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 Acked-by: Johannes Weiner