[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGsJ_4yzq35ng77LjOi0jj0A6-9o+s5jKgP=S2Cx9fSsJtyzeA@mail.gmail.com>
Date: Tue, 5 Nov 2024 22:15:40 +1300
From: Barry Song <21cnbao@...il.com>
To: David Hildenbrand <david@...hat.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, Usama Arif <usamaarif642@...il.com>,
linux-mm@...ck.org, linux-kernel@...r.kernel.org,
Barry Song <v-songbaohua@...o.com>, Chengming Zhou <chengming.zhou@...ux.dev>,
Yosry Ahmed <yosryahmed@...gle.com>, Nhat Pham <nphamcs@...il.com>,
Johannes Weiner <hannes@...xchg.org>, Hugh Dickins <hughd@...gle.com>,
Matthew Wilcox <willy@...radead.org>, Shakeel Butt <shakeel.butt@...ux.dev>,
Andi Kleen <ak@...ux.intel.com>, Baolin Wang <baolin.wang@...ux.alibaba.com>,
Chris Li <chrisl@...nel.org>, "Huang, Ying" <ying.huang@...el.com>,
Kairui Song <kasong@...cent.com>, Ryan Roberts <ryan.roberts@....com>
Subject: Re: [PATCH v2] mm: count zeromap read and set for swapout and swapin
On Tue, Nov 5, 2024 at 9:23 PM David Hildenbrand <david@...hat.com> wrote:
>
> On 05.11.24 04:40, Andrew Morton wrote:
> > On Mon, 4 Nov 2024 13:32:55 +0100 David Hildenbrand <david@...hat.com> wrote:
> >
> >>> As mentioned above, this isn't about fixing a bug; it's simply to ensure
> >>> that swap-related metrics don't disappear.
> >>
> >> Documentation/process/submitting-patches.rst:
> >>
> >> "A Fixes: tag indicates that the patch fixes an issue in a previous
> >> commit. It is used to make it easy to determine where a bug originated,
> >> which can help review a bug fix."
> >>
> >> If there is no BUG, I'm afraid you are abusing that tag.
> >
> > I think the abuse is reasonable. We have no Should-be-included-with:.
>
> A "Belongs-to:" might make sense, for this kind of stuff that is still
> only in an RFC. Or we update the doc to explicitly spell out this
> special case of using "Fixes" to sort out something into the RC.
>
> Because if this would be already in a released kernel, it would get a
> bit trickier: stable rules explicitly spell out "fix a real bug".
>
> >
> > 0ca0c24e3211 is only in 6.12-rcX so this is the time to make
> > userspace-visible tweaks, so the 6.12 interfaces are the same as the
> > 6.13+ interfaces (which is what I think is happening here?)
> > > And including the Fixes in this patch might be useful to someone who is
> > backporting 0ca0c24e3211 into some earlier kernel for their own
> > purposes.
>
> Just to be clear: adding new counters would hardly be fixing existing
> tools that perform calculations based on existing counters. So we are
> already changing the "userspace-visible" portion in some way, and I have
> no idea what in vmstat we consider "stable".
>
> But I still don't think it's all that big of a deal except in some
> handcrafted scenarios hardly anybody cares about; the cover letter is
> also pretty clear on that.
I may have been mistaken in the cover letter. According to the zswap data Usama
provided for servers, zero-filled pages accounted for about 1%. So
really doesn't
matter too much, but I just checked with Hailong from our team—he has data
on same-page-filled usage in Android apps, which on average show 3-4%
same-page-filled, with over 85% being zero-filled. Some apps even reach
6-7% zero-filled pages. We previously used these counters to profile
optimizations, but with zeromap now serving as the frontend for swap files,
those counters will disappear entirely from both zRAM and pswpin/pswpout
metrics, as folios are filtered earlier.
Hailong essentially has a table that looks like the below which could be
collected from the existing counters:
com.xxx.app 5% same-page-filled. 88% zero
com.yyy.app 6% same-page-filled. 85% zero
com.zzz.map 6.7 same-page-filled. 88% zero
....
Anyone on 6.12 will be unable to track zero-filled pages unless they
backport this patch from a newer kernel version if it doesn’t make it
into 6.12.
Whether it's marked as 'Belongs-to:' or 'Fixes:', I'd prefer we aim to
land it in
6.12 :-)
>
> So I'll shut up now and let people figure out the naming first, and if a
> new counter is required at all :)
>
> --
> Cheers,
>
> David / dhildenb
>
Thanks
Barry
Powered by blists - more mailing lists