Re: [PATCH 05/17] gup: Add try_get_folio()

[Jason Gunthorpe <jgg@ziepe.ca>]

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