Date:   Thu, 9 Mar 2023 08:25:29 +1100
From:   Dave Chinner <david@fromorbit.com>
To:     Yosry Ahmed <yosryahmed@google.com>
Cc:     Johannes Weiner <hannes@cmpxchg.org>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        "Darrick J. Wong" <djwong@kernel.org>,
        Christoph Lameter <cl@linux.com>,
        David Rientjes <rientjes@google.com>,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Roman Gushchin <roman.gushchin@linux.dev>,
        Hyeonggon Yoo <42.hyeyoo@gmail.com>,
        "Matthew Wilcox (Oracle)" <willy@infradead.org>,
        Miaohe Lin <linmiaohe@huawei.com>,
        David Hildenbrand <david@redhat.com>,
        Peter Xu <peterx@redhat.com>, NeilBrown <neilb@suse.de>,
        Shakeel Butt <shakeelb@google.com>,
        Michal Hocko <mhocko@kernel.org>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-xfs@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH v1 0/2] Ignore non-LRU-based reclaim in memcg reclaim
Message-ID: <20230308212529.GL360264@dread.disaster.area>
References: <20230228085002.2592473-1-yosryahmed@google.com>
 <20230308160056.GA414058@cmpxchg.org>
 <CAJD7tka=6b-U3m0FdMoP=9nC8sYuJ9thghb9muqN5hQ5ZMrDag@mail.gmail.com>
 <20230308201629.GB476158@cmpxchg.org>
 <CAJD7tkbDN2LUG_EZHV8VZd3M4-wtY9TCO5uS2c5qvqEWpoMvoA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAJD7tkbDN2LUG_EZHV8VZd3M4-wtY9TCO5uS2c5qvqEWpoMvoA@mail.gmail.com>
Precedence: bulk

On Wed, Mar 08, 2023 at 12:24:08PM -0800, Yosry Ahmed wrote:
> > I tried to come up with something better, but wasn't happy with any of
> > the options, either. So I defaulted to just leaving it alone :-)
> >
> > It's part of the shrinker API and the name hasn't changed since the
> > initial git import of the kernel tree. It should be fine, churn-wise.
> 
> Last attempt, just update_reclaim_state() (corresponding to
> flush_reclaim_state() below). It doesn't tell a story, but neither
> does incrementing a counter in current->reclaim_state. If that doesn't
> make you happy I'll give up now and leave it as-is :)

This is used in different subsystem shrinkers outside mm/, so the
name needs to be correctly namespaced. Please prefix it with the
subsystem the function belongs to, at minimum.

mm_account_reclaimed_pages() is what is actually being done here.
It is self describing  and leaves behind no ambiguity as to what is
being accounted and why, nor which subsystem the accounting belongs
to.

It doesn't matter what the internal mm/vmscan structures are called,
all we care about is telling the mm infrastructure how many extra
pages were freed by the shrinker....

-Dave.
-- 
Dave Chinner
david@fromorbit.com