MIME-Version: 1.0
References: <20230124012210.13963-1-vishal.moola@gmail.com>
 <20230124012210.13963-2-vishal.moola@gmail.com> <477bbc1e-a60a-cfce-d31e-0b4bdd5f6429@redhat.com>
 <526e5156-6c4b-9f2c-1637-2f83c1dfea1e@redhat.com> <53d86764-d155-8d97-1a4d-7e701848c950@redhat.com>
In-Reply-To: <53d86764-d155-8d97-1a4d-7e701848c950@redhat.com>
From:   Vishal Moola <vishal.moola@gmail.com>
Date:   Wed, 25 Jan 2023 14:09:40 -0800
Message-ID: <CAOzc2pyV9-wq04NRKVE1vRj7PnRF7g+jSFFj-oKYuZk-t9smBA@mail.gmail.com>
Subject: Re: [PATCH mm-unstable v2 1/6] mm: Add folio_estimated_mapcount()
To:     David Hildenbrand <david@redhat.com>
Cc:     linux-mm@kvack.org, akpm@linux-foundation.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Jan 25, 2023 at 1:29 PM David Hildenbrand <david@redhat.com> 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@gmail.com>
> >>> ---
> >>>     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.