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 4A7F4C433E0 for ; Wed, 10 Feb 2021 22:59:47 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id C0A6A64EBB for ; Wed, 10 Feb 2021 22:59:46 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C0A6A64EBB 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 1D6F06B0006; Wed, 10 Feb 2021 17:59:46 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 187AC6B006C; Wed, 10 Feb 2021 17:59:46 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 074986B006E; Wed, 10 Feb 2021 17:59:46 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id E361F6B0006 for ; Wed, 10 Feb 2021 17:59:45 -0500 (EST) Received: from smtpin01.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id B44D8DE18 for ; Wed, 10 Feb 2021 22:59:45 +0000 (UTC) X-FDA: 77803877130.01.hose60_1304df227613 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin01.hostedemail.com (Postfix) with ESMTP id 91AA61005CC37 for ; Wed, 10 Feb 2021 22:59:45 +0000 (UTC) X-HE-Tag: hose60_1304df227613 X-Filterd-Recvd-Size: 6473 Received: from mail-lf1-f50.google.com (mail-lf1-f50.google.com [209.85.167.50]) by imf21.hostedemail.com (Postfix) with ESMTP for ; Wed, 10 Feb 2021 22:59:45 +0000 (UTC) Received: by mail-lf1-f50.google.com with SMTP id f1so5430554lfu.3 for ; Wed, 10 Feb 2021 14:59:44 -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=xrTlzAySlcJTUXRnlEJkf0lrI9Iuz7zvICLmzyym/xM=; b=eMT+HrSpUk67s7D9Fb/wYqRRhLhygRRnmrKjid5+dGjRRdcWnl+Coe1Hmivi013BcD vM7qrpbntHPGYw3ZR2mT73sG3gTPW+Ae1qRtuLiiI+dqqOKuimFiDxcTK8abjY31E3rf Fmq+1ScDwhhI6jHG7o9mVfPDf1sBmPRQsQLW79d/E3O3s4IGItRepiKnHYMt1D30bRmR MM+t2zmasxETxauJwuE6V8yQF8vBKu/UNkNA0RFX4nCUyjeyCijXg0fMI7T9wwjnhnNQ 0u4Vr+WykIzPW6B+1Kw9DctVhRNP42iVTrRSwiizbnXzW/vUetxSQ8bVn1+17zIlc7Ni ft3w== 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=xrTlzAySlcJTUXRnlEJkf0lrI9Iuz7zvICLmzyym/xM=; b=U1uy+ez67pvs2glwqOhDPpJMcAwh1OBcWDeDAdGA34kqOpIpPq9YeR3SaYAEGA49D4 rq94t3pJyOrSuY5/3j0z18uP4VIlitg33r5DR+HX2jAib+dJ54hw1S1ortod7yGF7E2n hyL3lYGgvR1o0IQT/vE5Ns+mQZazpA4tr3i862sGMnfQS5LffHHg9GCAT4LlMsbTjZvP J1DT0nkPB6NNVzZEUHlQh95995wFViDiOQcOuMGiGjQDG8Su0Ic6gZJmoYjeKIcV52Cr IFTiVn0lkPNczgOMelC7gy+of2X2CJ2Sop4RqQvxHmyHGGPOFZMYEMopnd4eZO87JhVA CHFQ== X-Gm-Message-State: AOAM532nH2WErJGcagUkWwsxYmtfwpu4qqZYWClSq/QRQZ5qQjAJqyxK aKWOYNVO+01vR7GQeT0YUkWW4pCEoDQzSWMQ3ngukg== X-Google-Smtp-Source: ABdhPJw6wefq5+DqM++nrhDWfNPnH6l/1z2hdn+8UkJNXwUTzgKLrXbPfqsQWUSSzXJb8EYLGptouvYaVvCRqnVy9jc= X-Received: by 2002:a19:4cc2:: with SMTP id z185mr2657923lfa.83.1612997983406; Wed, 10 Feb 2021 14:59:43 -0800 (PST) MIME-Version: 1.0 References: <20210209214543.112655-1-hannes@cmpxchg.org> In-Reply-To: From: Shakeel Butt Date: Wed, 10 Feb 2021 14:59:32 -0800 Message-ID: Subject: Re: [PATCH v2] mm: page-writeback: simplify memcg handling in test_clear_page_writeback() To: Johannes Weiner Cc: Hugh Dickins , Andrew Morton , Michal Hocko , Roman Gushchin , Linux MM , Cgroups , LKML , Kernel Team , Arjun Roy Content-Type: text/plain; charset="UTF-8" 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 10, 2021 at 9:44 AM Johannes Weiner wrote: > > On Wed, Feb 10, 2021 at 08:22:00AM -0800, Hugh Dickins wrote: > > On Tue, 9 Feb 2021, Hugh Dickins wrote: > > > On Tue, 9 Feb 2021, Johannes Weiner wrote: > > > > > > > Page writeback doesn't hold a page reference, which allows truncate to > > > > free a page the second PageWriteback is cleared. This used to require > > > > special attention in test_clear_page_writeback(), where we had to be > > > > careful not to rely on the unstable page->memcg binding and look up > > > > all the necessary information before clearing the writeback flag. > > > > > > > > Since commit 073861ed77b6 ("mm: fix VM_BUG_ON(PageTail) and > > > > BUG_ON(PageWriteback)") test_clear_page_writeback() is called with an > > > > explicit reference on the page, and this dance is no longer needed. > > > > > > > > Use unlock_page_memcg() and dec_lruvec_page_stat() directly. > > > > > > s/stat()/state()/ > > > > > > This is a nice cleanup: I hadn't seen that connection at all. > > > > > > But I think you should take it further: > > > __unlock_page_memcg() can then be static in mm/memcontrol.c, > > > and its declarations deleted from include/linux/memcontrol.h? > > > > And further: void lock_page_memcg(page), not returning memcg. > > You're right on all counts! > > > > And further: delete __dec_lruvec_state() and dec_lruvec_state() > > > from include/linux/vmstat.h - unless you feel that every "inc" > > > ought to be matched by a "dec", even when unused. > > Hey look, there isn't a user for the __inc, either :) There is one for > inc, but I don't insist on having symmetry there. > > > > > Signed-off-by: Johannes Weiner > > > > > > Acked-by: Hugh Dickins > > Thanks for the review and good feedback. > > How about this v2? > > --- > > From 5bcc0f468460aa2670c40318bb657e8b08ef96d5 Mon Sep 17 00:00:00 2001 > From: Johannes Weiner > Date: Tue, 9 Feb 2021 16:22:42 -0500 > Subject: [PATCH] mm: page-writeback: simplify memcg handling in > test_clear_page_writeback() > > Page writeback doesn't hold a page reference, which allows truncate to > free a page the second PageWriteback is cleared. This used to require > special attention in test_clear_page_writeback(), where we had to be > careful not to rely on the unstable page->memcg binding and look up > all the necessary information before clearing the writeback flag. > > Since commit 073861ed77b6 ("mm: fix VM_BUG_ON(PageTail) and > BUG_ON(PageWriteback)") test_clear_page_writeback() is called with an > explicit reference on the page, and this dance is no longer needed. > > Use unlock_page_memcg() and dec_lruvec_page_state() directly. > > This removes the last user of the lock_page_memcg() return value, > change it to void. Touch up the comments in there as well. This also > removes the last extern user of __unlock_page_memcg(), make it > static. Further, it removes the last user of dec_lruvec_state(), > delete it, along with a few other unused helpers. > > Signed-off-by: Johannes Weiner > Acked-by: Hugh Dickins > Reviewed-by: Shakeel Butt The patch looks fine. I don't want to spoil the fun but just wanted to call out that I might bring back __unlock_page_memcg() for the memcg accounting of zero copy TCP memory work where we are uncharging the page in page_remove_rmap().