On Tue, Feb 7, 2023 at 3:08 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> On Tue, Feb 07, 2023 at 02:56:52PM -0800, James Houghton wrote:
> > Hi Matthew,
> >
> > On Mon, Feb 6, 2023 at 12:34 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> > > I now think the mapcount has to be defined as "How many VMAs have
> > > one-or-more pages of this folio mapped".
> > >
> > > That means that our future folio_add_file_rmap_range() looks a bit
> > > like this:
> > >
> > > {
> > > bool add_mapcount = true;
> > >
> > > if (nr < folio_nr_pages(folio))
> > > add_mapcount = !folio_has_ptes(folio, vma);
> >
> > Can you elaborate on how folio_has_ptes() might be implemented?
>
> ... oh. First I called it folio_has_ptes() and then I went looking
> for inspiration on how to implement it, and I found page_mapped_in_vma()
> and so I thought folio_mapped_in_vma() would be a better name, but
> I forgot to change it here. Sorry.
Oh I see. Thanks. :)
So page_vma_mapped_walk() might have to walk up to HPAGE_PMD_NR-ish
PTEs (if we find a bunch of pte_none() PTEs). Just curious, could that
be any slower than what we currently do (like, incrementing up to
HPAGE_PMD_NR-ish subpage mapcounts)? Or is it not a concern?
>
> > It seems like for a naturally aligned THP, we can see if the PMD is
> > pmd_present(), otherwise, do we need to check (potentially) all the
> > PTEs? Does this introduce an ordering requirement, where we have to
> > update the page table always before/after we call
> > folio_add_file_rmap_range()?
>
> Actually, you _mustn't_ update the page table first, or it will see
> the PTEs and say "Ah, the folio is already mapped, I should not
> increment mapcount". So we keep the calling order as it is now;
> folio_add_X_rmap() followed by a call to set_ptes().
Ok, thanks for clarifying.
- James
