Re: Folio discussion recap
From: Matthew Wilcox <willy@infradead.org>
Date: 2021-09-23 03:50:08
Also in:
linux-fsdevel, lkml
On Wed, Sep 22, 2021 at 05:45:15PM -0700, Ira Weiny wrote:
On Tue, Sep 21, 2021 at 11:18:52PM +0100, Matthew Wilcox wrote:quoted
+/** + * page_slab - Converts from page to slab. + * @p: The page. + * + * This function cannot be called on a NULL pointer. It can be called + * on a non-slab page; the caller should check is_slab() to be sure + * that the slab really is a slab. + * + * Return: The slab which contains this page. + */ +#define page_slab(p) (_Generic((p), \ + const struct page *: (const struct slab *)_compound_head(p), \ + struct page *: (struct slab *)_compound_head(p))) + +static inline bool is_slab(struct slab *slab) +{ + return test_bit(PG_slab, &slab->flags); +} +I'm sorry, I don't have a dog in this fight and conceptually I think folios are a good idea... But for this work, having a call which returns if a 'struct slab' really is a 'struct slab' seems odd and well, IMHO, wrong. Why can't page_slab() return NULL if there is no slab containing that page?
No, this is a good question.
The way slub works right now is that if you ask for a "large" allocation,
it does:
flags |= __GFP_COMP;
page = alloc_pages_node(node, flags, order);
and returns page_address(page) (eventually; the code is more complex)
So when you call kfree(), it uses the PageSlab flag to determine if the
allocation was "large" or not:
page = virt_to_head_page(x);
if (unlikely(!PageSlab(page))) {
free_nonslab_page(page, object);
return;
}
slab_free(page->slab_cache, page, object, NULL, 1, _RET_IP_);
Now, you could say that this is a bad way to handle things, and every
allocation from slab should have PageSlab set, and it should use one of
the many other bits in page->flags to indicate whether it's a large
allocation or not. I may have feelings in that direction myself.
But I don't think I should be changing that in this patch.
Maybe calling this function is_slab() is the confusing thing.
Perhaps it should be called SlabIsLargeAllocation(). Not sure.