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 X-Spam-Level: X-Spam-Status: No, score=-18.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 70979C433E0 for ; Thu, 4 Mar 2021 17:13:18 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 0D71B64F5C for ; Thu, 4 Mar 2021 17:13:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0D71B64F5C Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 936536B0006; Thu, 4 Mar 2021 12:13:17 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 90DB56B0007; Thu, 4 Mar 2021 12:13:17 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7D72A6B000D; Thu, 4 Mar 2021 12:13:17 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0151.hostedemail.com [216.40.44.151]) by kanga.kvack.org (Postfix) with ESMTP id 605896B0006 for ; Thu, 4 Mar 2021 12:13:17 -0500 (EST) Received: from smtpin09.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 24AE41832B5AA for ; Thu, 4 Mar 2021 17:13:17 +0000 (UTC) X-FDA: 77882837634.09.0EE98A7 Received: from mail-lj1-f172.google.com (mail-lj1-f172.google.com [209.85.208.172]) by imf08.hostedemail.com (Postfix) with ESMTP id 86F7F80192E6 for ; Thu, 4 Mar 2021 17:13:12 +0000 (UTC) Received: by mail-lj1-f172.google.com with SMTP id h4so34108424ljl.0 for ; Thu, 04 Mar 2021 09:13:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=UFk60/R93j/Ir+3OsIDPm87BVki6b+umgOzaexWfu9Y=; b=ipdNXv2W0+m0TzGiGZv/k4LwVFCEWWmyFxoH/AYPwJo5e7snO6Lo6WhFIf4StYPudv GF9t8QFsvY8w/m8QsKaAe0JtnyI0ZJu5wRBt06IVJepz0mdCB+sbU1pTGeIhTSV3Uo3t vhbPl5N0Dsl+fvA8jy4KGdhnFFRyfHlAlW2C1NycxnBBiNxtGh0trXgrjheABWbzzsof 3fXEIY1DSd9tR61ancM308v6eKm7y1ZzOTwT5dxGaUcNRgiJ+JsA96KnOafSct7wzL/n C5xuzppV407zHw2hxKpg8PpdWFTC+RdKXnxOam+nmXN1eVa6X7ttbeaX6q8YdcGBMNos aotw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=UFk60/R93j/Ir+3OsIDPm87BVki6b+umgOzaexWfu9Y=; b=oeZVHPxL1Bp77Tk9fTXvQ1NOMDW92KaKUrVhA9SnkV4gpS1U2hehdD+kSU7ymSPBlN ucbUSq3yhaZlq8POwGCBhwdT1ZXHrbmMEzOsESeUPMl6tzsYam6cJjWxWGLZDGcTdao0 y5EK8CuMEgOyg6TpiSP9p5cfUuhPq239ABek+fR6hMB6u53sONbaVPGhGSPQJGs3F9rJ 91IaD2/cydTg4xo0mad22UBhisUk7C9wdrbN1XmvyLTo936J7gbgjQMu8Kcbt1DoeaMN nWwf668l76L4ab7RgSSBYOYkAiLgUGophXiU04kZd9nZ5XqsaHFGvAavA/nBtw3bm5pL h5fQ== X-Gm-Message-State: AOAM532CDQRBHFfNkbvo+zgJJFA9+gEcOBuqvY9LK0YunjCuMALlvnuU tNuXSTilqeHB8x26X9QAzCPa4GV9I0VecTcHOO+6Ww== X-Google-Smtp-Source: ABdhPJwd/9rGKR+1GgvpMgGk8rXue/ZLv08tWsIm+I/pEInOeNK57UG4fTTeUXVNpCEnlOZUqQgjLqVDgQntILSRdBk= X-Received: by 2002:a2e:9cc4:: with SMTP id g4mr2891774ljj.34.1614877988461; Thu, 04 Mar 2021 09:13:08 -0800 (PST) MIME-Version: 1.0 References: <20210304014229.521351-1-shakeelb@google.com> In-Reply-To: From: Shakeel Butt Date: Thu, 4 Mar 2021 09:12:57 -0800 Message-ID: Subject: Re: [PATCH v3] memcg: charge before adding to swapcache on swapin To: Johannes Weiner Cc: Hugh Dickins , Roman Gushchin , Michal Hocko , Andrew Morton , Cgroups , Linux MM , LKML Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 86F7F80192E6 X-Stat-Signature: 1kq7a7ggb1urfhucpuan7qufmbigc3f9 Received-SPF: none (google.com>: No applicable sender policy available) receiver=imf08; identity=mailfrom; envelope-from=""; helo=mail-lj1-f172.google.com; client-ip=209.85.208.172 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1614877992-569259 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 Thu, Mar 4, 2021 at 7:48 AM Johannes Weiner wrote: > > On Wed, Mar 03, 2021 at 05:42:29PM -0800, Shakeel Butt wrote: > > Currently the kernel adds the page, allocated for swapin, to the > > swapcache before charging the page. This is fine but now we want a > > per-memcg swapcache stat which is essential for folks who wants to > > transparently migrate from cgroup v1's memsw to cgroup v2's memory and > > swap counters. In addition charging a page before exposing it to other > > parts of the kernel is a step in the right direction. > > > > To correctly maintain the per-memcg swapcache stat, this patch has > > adopted to charge the page before adding it to swapcache. One > > challenge in this option is the failure case of add_to_swap_cache() on > > which we need to undo the mem_cgroup_charge(). Specifically undoing > > mem_cgroup_uncharge_swap() is not simple. > > > > To resolve the issue, this patch introduces transaction like interface > > to charge a page for swapin. The function mem_cgroup_charge_swapin_page() > > initiates the charging of the page and mem_cgroup_finish_swapin_page() > > completes the charging process. So, the kernel starts the charging > > process of the page for swapin with mem_cgroup_charge_swapin_page(), > > adds the page to the swapcache and on success completes the charging > > process with mem_cgroup_finish_swapin_page(). > > > > Signed-off-by: Shakeel Butt > > The patch looks good to me, I have just a minor documentation nit > below. But with that addressed, please add: > > Acked-by: Johannes Weiner Thanks. > [...] > > It's possible somebody later needs to change things around in the > swapin path and it's not immediately obvious when exactly these two > functions need to be called in the swapin sequence. > > Maybe add here and above that charge_swapin_page needs to be called > before we try adding the page to the swapcache, and finish_swapin_page > needs to be called when swapcache insertion has been successful? I will update the comments and send v4 after a day or so to see if someone else has any comments.