Re: Are THPs the right model for the pagecache?

[John Hubbard <jhubbard@nvidia.com>]

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