Re: [RFC PATCH 5/5] mm: shmem: add anonymous share mTHP counters

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2024/4/24 16:15, Ryan Roberts wrote:
On 24/04/2024 08:11, David Hildenbrand wrote:
On 24.04.24 08:10, Baolin Wang wrote:


On 2024/4/23 19:37, David Hildenbrand wrote:
On 23.04.24 03:17, Barry Song wrote:
On Mon, Apr 22, 2024 at 3:03 PM Baolin Wang
<baolin.wang@xxxxxxxxxxxxxxxxx> wrote:

Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
---
    include/linux/huge_mm.h | 2 ++
    mm/huge_memory.c        | 4 ++++
    mm/shmem.c              | 5 ++++-
    3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 26b6fa98d8ac..67b9c1acad31 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -270,6 +270,8 @@ enum mthp_stat_item {
           MTHP_STAT_ANON_SWPOUT,
           MTHP_STAT_ANON_SWPOUT_FALLBACK,
           MTHP_STAT_ANON_SWPIN_REFAULT,
+       MTHP_STAT_SHMEM_ANON_ALLOC,
+       MTHP_STAT_SHMEM_ANON_ALLOC_FALLBACK,

not quite sure about this. for 2MB pmd-mapped THP shmem, we count them
as FILE_THP.
here we are counting as SHMEM_ANON. To me, SHMEM_ANON is more correct but
it doesn't align with pmd-mapped THP. David, Ryan, what do you think?

The term "anonymous share" in the patch subject is weird to begin with
;) Easy to confuse with anonymous cow-shared memory. Let's just call it
"anonymous shmem", which it is under the hood.

Sure.

... regarding the question: if we add FILE_ALLOC and friends, at least
initially, we wouldn't account other large pagecache folios.

... likely we should add that then as well so the counter matches the
actual name?

If we later realize that we need separate FILE vs. SHMEM vs. WHATEVER
counters, we can always add more fine-grained counters later. Doing it
consistently w.r.t. traditional THPs first sounds reasonable.

Um, once we expose it to userspace through the sysfs interface, the
sysfs interface should be explicit as much as possible and avoid
confusing users, otherwise it will be difficult to change this kind of
interface in the future. Personally, I prefer to Ryan's suggestion.

Inconsistency is confusing. As long as you avoid that, I don't particularly care.

This is a good point. We have been careful to make sure the 2M ANON mTHP stats
match the existing PMD-size stats. So we should definitely make sure that any
future 2M FILE mTHP stats match too, which I guess means counting both SHMEM and
FILE events.

So perhaps it makes more sense to add FILE counters to start with. If we need
the SHMEM-specific counters, we could add them later?

I'm happy to go with the crowd on this...

(Seems I'm the only one who prefers the term 'SHMEM_' now.) Fine, I have no strong preference, and let's keep consistency first. Thanks guys.




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux