On 11/13/20 9:44 AM, Matthew Wilcox wrote:
On Fri, Nov 13, 2020 at 12:38:36PM +0000, Matthew Wilcox wrote:
So what if we had:
/* Cache memory */
struct cmem {
struct page pages[1];
};
OK, that's a terrible name. I went with 'folio' for this demonstration.
Other names suggested include album, sheaf and ream.
+1 for "folio", that's a good name! It stands out, as it should, given that
it's a different type. The others could work too, but this one is especially
nice because there are no pre-existing uses in the kernel, so no baggage.
And it's grep-able too.
Your demo diff looks good to me, and I think it noticeably improves things
and is worth doing. One tiny tweak first:
The struct member should be named .page, rather than .pages. That's because
pages in -mm usually refers to "struct page **", but here you've got a
struct page *. Notice how the various odd things get better in the patch
if you change it to .page:
...
+static inline atomic_t *folio_mapcount_ptr(struct folio *folio)
{
- return &page[1].compound_mapcount;
+ return &folio->pages[1].compound_mapcount;
See, this diff is changing from "page" to "pages", but we don't
especially desire that, *and* it's arguably even more readable like
this anyway:
return &folio->page[1].compound_mapcount;
and...
...
@@ -166,16 +167,16 @@ void __dump_page(struct page *page, const char *reason)
out_mapping:
BUILD_BUG_ON(ARRAY_SIZE(pageflag_names) != __NR_PAGEFLAGS + 1);
- pr_warn("%sflags: %#lx(%pGp)%s\n", type, head->flags, &head->flags,
- page_cma ? " CMA" : "");
+ pr_warn("%sflags: %#lx(%pGp)%s\n", type, folio->pages->flags,
+ &(folio->pages->flags), page_cma ? " CMA" : "");
hex_only:
print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32,
sizeof(unsigned long), page,
sizeof(struct page), false);
- if (head != page)
+ if (folio->pages != page)
This one in particular gets a lot better:
if (folio->page != page)
...is much more natural.
thanks,
--
John Hubbard
NVIDIA
