Re: [PATCH mm-unstable v2 1/6] mm: Add folio_estimated_mapcount()

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

 



On 25.01.23 23:09, Vishal Moola wrote:
On Wed, Jan 25, 2023 at 1:29 PM David Hildenbrand <david@xxxxxxxxxx> wrote:

On 25.01.23 11:24, David Hildenbrand wrote:
On 25.01.23 11:20, David Hildenbrand wrote:
On 24.01.23 02:22, Vishal Moola (Oracle) wrote:
folio_estimated_mapcount() takes in a folio and calls page_mapcount() on
the first page of that folio.

This is necessary for folio conversions where we only care about either the
entire_mapcount of a large folio, or the mapcount of a not large folio.

This is in contrast to folio_mapcount() which calculates the total
number of the times a folio and its subpages are mapped.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@xxxxxxxxx>
---
     include/linux/mm.h | 5 +++++
     1 file changed, 5 insertions(+)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index c9db257f09b3..543c360f7ecc 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -875,6 +875,11 @@ static inline int page_mapcount(struct page *page)
             return mapcount;
     }

+static inline int folio_estimated_mapcount(struct folio *folio)
+{
+   return page_mapcount(folio_page(folio, 0));
+}
+
     int folio_total_mapcount(struct folio *folio);

     /**

I'm sorry, but "estimated" as absolutely unclear semantics. You could
have a THP mapped into 9999 processes using THP and the estimation would
be "0".

... or would it be 9999 ? What about a PMD-mapped THP? What about a
partially unmapped THP?

What are we estimating?

Thinking about mapcounts again, might not have been my smartest moment.

What we return here is the precise number of times the first subpage is
mapped (via the large folio and directly). That's supposed to be an
estimate for the number of times any subpage part of the folio is mapped.

I really don't know a better name, but folio_estimated_mapcount() does
not feel completely right to me and triggere dmy confusion in the first
place ... hm ...

I can understand the confusion, but I can't think of a better name
either myself. I'll go ahead and add a comment to make the purpose
of this function more clear. Looks like I'll have to move it to get rid
of the build warnings/errors anyway.

The issue is that we're not estimating the mapcount of the folio, so the name is very misleading ... I think you really want to avoid the term mapcount completely in that context. We're just using the #mappings of the first subpage to determine something differently.

Thinking about it, I guess "folio_estimated_sharers()" might be what we actually want to name it. Then you can comment how we estimate sharers by looking at into how many page tables the first subpage is currently mapped, and assume the same holds true for the other subpages.

It's unreliable because other subpages might behave differently, we might not be holding the pagelock to stabilize, and we're not looking at indirect mappings via the swapcache. But it's a comapratively good estimate for most scenarios I guess.

--
Thanks,

David / dhildenb





[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