On Tue, Jan 04, 2022 at 05:25:07PM -0800, John Hubbard wrote:
> > + folio = page_folio(page);
> > + if (WARN_ON_ONCE(folio_ref_count(folio) < 0))
> > return NULL;
> > - if (unlikely(!page_cache_add_speculative(head, refs)))
> > + if (unlikely(!folio_ref_try_add_rcu(folio, refs)))
>
> I'm a little lost about the meaning and intended use of the _rcu aspects
> of folio_ref_try_add_rcu() here. For example, try_get_folio() does not
> require that callers are in an rcu read section, right? This is probably
> just a documentation question, sorry if it's obvious--I wasn't able to
> work it out on my own.
Normally this is called under gup_fast, so it is operating under the
quasi-rcu it uses. I have no idea about preempt kernels if this should
be marked with rcu or if the local_irq_save is good enough..
The exceptional cases are get_gate_page() which I suppose relies on
the mmap lock to ensure that the special gate page cannot be
concurrently freed.
Then there are the cases under slow GUP (eg follow_page_pte()) which
don't need "try" at all because they hold the PTLs so it is guarenteed
the refcount should be non-zero AFAIK..
It wouldn't be a bad idea to have a non-try version of this just for
clarity, could it peform a bit better?
/*
* try_grab_page() should always succeed here, because: a) we
* hold the pmd (ptl) lock, and b) we've just checked that the
* huge pmd (head) page is present in the page tables. The ptl
* prevents the head page and tail pages from being rearranged
* in any way. So this page must be available at this point,
* unless the page refcount overflowed:
*/
if (WARN_ON_ONCE(!try_grab_page(page, flags))) {
Jason
