On Tue, May 11, 2021 at 10:47:10PM +0100, Matthew Wilcox (Oracle) wrote:
> -static inline int page_ref_add_unless(struct page *page, int nr, int u)
> +static inline bool page_ref_add_unless(struct page *page, int nr, int u)
> {
> - int ret = atomic_add_unless(&page->_refcount, nr, u);
> + bool ret = atomic_add_unless(&page->_refcount, nr, u);
>
> if (page_ref_tracepoint_active(page_ref_mod_unless))
> __page_ref_mod_unless(page, nr, ret);
> return ret;
> }
Unrelated but neat cleanup.
>
> -static inline int folio_ref_add_unless(struct folio *folio, int nr, int u)
> +static inline bool folio_ref_add_unless(struct folio *folio, int nr, int u)
> {
> return page_ref_add_unless(&folio->page, nr, u);
> }
This should probably go into the patch adding folio_ref_add_unless.
> +static inline bool folio_ref_try_add_rcu(struct folio *folio, int count)
Should this have a __ prefix and/or a don't use direct comment?
> +{
> +#ifdef CONFIG_TINY_RCU
> + /*
> + * The caller guarantees the folio will not be freed from interrupt
> + * context, so (on !SMP) we only need preemption to be disabled
> + * and TINY_RCU does that for us.
> + */
> +# ifdef CONFIG_PREEMPT_COUNT
> + VM_BUG_ON(!in_atomic() && !irqs_disabled());
> +# endif
VM_BUG_ON(IS_ENABLED(CONFIG_PREEMPT_COUNT) &&
!in_atomic() && !irqs_disabled());
?
> + VM_BUG_ON_FOLIO(folio_ref_count(folio) == 0, folio);
> + folio_ref_add(folio, count);
> +#else
> + if (unlikely(!folio_ref_add_unless(folio, count, 0))) {
> + /* Either the folio has been freed, or will be freed. */
> + return false;
> + }
> +#endif
> + return true;
but is this tiny rcu optimization really worth it? I guess we're just
preserving it from the existing code and don't rock the boat..
> @@ -1746,6 +1746,26 @@ pgoff_t page_cache_prev_miss(struct address_space *mapping,
> }
> EXPORT_SYMBOL(page_cache_prev_miss);
>
> +/*
> + * Lockless page cache protocol:
> + * On the lookup side:
> + * 1. Load the folio from i_pages
> + * 2. Increment the refcount if it's not zero
> + * 3. If the folio is not found by xas_reload(), put the refcount and retry
> + *
> + * On the removal side:
> + * A. Freeze the page (by zeroing the refcount if nobody else has a reference)
> + * B. Remove the page from i_pages
> + * C. Return the page to the page allocator
> + *
> + * This means that any page may have its reference count temporarily
> + * increased by a speculative page cache (or fast GUP) lookup as it can
> + * be allocated by another user before the RCU grace period expires.
> + * Because the refcount temporarily acquired here may end up being the
> + * last refcount on the page, any page allocation must be freeable by
> + * put_folio().
> + */
> +
> /*
> * mapping_get_entry - Get a page cache entry.
> * @mapping: the address_space to search
Is this really a good place for the comment? I'd expect it either near
a relevant function or at the top of a file.
