On Tue, Sep 22, 2020 at 12:44:51PM +0100, Matthew Wilcox wrote:
> I've just kicked off a test run using this:
>
> +/*
> + * Have to be careful when freeing a non-compound allocation in case somebody
> + * else takes a temporary reference on the first page and then calls put_page()
> + */
> void __free_pages(struct page *page, unsigned int order)
> {
> - if (put_page_testzero(page))
> - free_the_page(page, order);
> + if (likely(page_ref_freeze(page, 1)))
> + goto free;
> + if (likely(order == 0 || PageHead(page))) {
> + if (put_page_testzero(page))
> + goto free;
> + return;
> + }
> +
> + prep_compound_page(page, order);
> + put_page(page);
> + return;
> +free:
> + free_the_page(page, order);
> }
> EXPORT_SYMBOL(__free_pages);
>
> > > Better ideas?
> >
> > IMHO, alloc_pages() with order > 0 and without __GFP_COMP is a weird beast. In
> > that case it would be probably best to refcount each base page separately. I
> > don't know how many assumptions this would break :/
>
> You sound like a man who's never dealt with device drivers. Multiorder,
> non-compound allocations are the norm in that world.
This seems easier to reason about:
+/*
+ * If we free a non-compound allocation, another thread may have a
+ * transient reference to the first page. It has no way of knowing
+ * about the rest of the allocation, so we have to free all but the
+ * first page here.
+ */
void __free_pages(struct page *page, unsigned int order)
{
if (put_page_testzero(page))
free_the_page(page, order);
+ else
+ while (order-- > 0)
+ free_the_page(page + 1 << order, order);
}
EXPORT_SYMBOL(__free_pages);
