On Mon, Jan 10, 2022 at 08:32:20PM -0800, John Hubbard wrote:
> On 1/9/22 20:23, Matthew Wilcox (Oracle) wrote:
> ...
> > diff --git a/mm/gup.c b/mm/gup.c
> > index dbb1b54d0def..3ed9907f3c8d 100644
> > --- a/mm/gup.c
> > +++ b/mm/gup.c
> > @@ -29,12 +29,23 @@ struct follow_page_context {
> > unsigned int page_mask;
> > };
> > -static void hpage_pincount_add(struct page *page, int refs)
> > +/*
> > + * When pinning a compound page of order > 1 (which is what
> > + * hpage_pincount_available() checks for), use an exact count to track
> > + * it, via page_pincount_add/_sub().
> > + *
> > + * However, be sure to *also* increment the normal page refcount field
> > + * at least once, so that the page really is pinned. That's why the
> > + * refcount from the earlier try_get_compound_head() is left intact.
> > + */
>
> I just realized, after looking at this again in a later patch, that the
> last paragraph, above, is now misplaced. It refers to the behavior of the
> caller, not to this routine. So it needs to be airlifted back to the
> caller.
I really do think it fits better here. The thing is there's just one
caller, so it's a little hard to decide what "all callers" need when
there's only one. Maybe I can wordsmith this a bit to read better.
> > +static void page_pincount_add(struct page *page, int refs)
> > {
> > - VM_BUG_ON_PAGE(!hpage_pincount_available(page), page);
> > VM_BUG_ON_PAGE(page != compound_head(page), page);
> > - atomic_add(refs, compound_pincount_ptr(page));
> > + if (hpage_pincount_available(page))
> > + atomic_add(refs, compound_pincount_ptr(page));
> > + else
> > + page_ref_add(page, refs * (GUP_PIN_COUNTING_BIAS - 1));
> > }
> > static void hpage_pincount_sub(struct page *page, int refs)
> > @@ -150,21 +161,7 @@ struct page *try_grab_compound_head(struct page *page,
> > if (!page)
> > return NULL;
> > - /*
> > - * When pinning a compound page of order > 1 (which is what
> > - * hpage_pincount_available() checks for), use an exact count to
> > - * track it, via hpage_pincount_add/_sub().
> > - *
> > - * However, be sure to *also* increment the normal page refcount
> > - * field at least once, so that the page really is pinned.
> > - * That's why the refcount from the earlier
> > - * try_get_compound_head() is left intact.
> > - */
>
> ...here.
>
> > - if (hpage_pincount_available(page))
> > - hpage_pincount_add(page, refs);
> > - else
> > - page_ref_add(page, refs * (GUP_PIN_COUNTING_BIAS - 1));
> > -
> > + page_pincount_add(page, refs);
> > mod_node_page_state(page_pgdat(page), NR_FOLL_PIN_ACQUIRED,
> > refs);
>
> thanks,
> --
> John Hubbard
> NVIDIA
