Re: [PATCH 4/5] mm/page_ext: support extra space allocation by page_ext user

[Vlastimil Babka]

On 08/10/2016 08:16 AM, js1304@xxxxxxxxx wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
>
> Until now, if some page_ext users want to use it's own field on page_ext,
> it should be defined in struct page_ext by hard-coding. It has a problem
> that wastes memory in following situation.
>
> struct page_ext {
>  #ifdef CONFIG_A
> 	int a;
>  #endif
>  #ifdef CONFIG_B
> 	int b;
>  #endif
> };
>
> Assume that kernel is built with both CONFIG_A and CONFIG_B.
> Even if we enable feature A and doesn't enable feature B at runtime,
> each entry of struct page_ext takes two int rather than one int.
> It's undesirable result so this patch tries to fix it.
>
> To solve above problem, this patch implements to support extra space
> allocation at runtime. When need() callback returns true, it's extra
> memory requirement is summed to entry size of page_ext. Also, offset
> for each user's extra memory space is returned. With this offset,
> user can use this extra space and there is no need to define needed
> field on page_ext by hard-coding.
>
> This patch only implements an infrastructure. Following patch will use it
> for page_owner which is only user having it's own fields on page_ext.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>

Fine, but...

>  static void __init invoke_init_callbacks(void)
> @@ -91,6 +102,16 @@ static void __init invoke_init_callbacks(void)
>  	}
>  }
>
> +static unsigned long get_entry_size(void)
> +{
> +	return sizeof(struct page_ext) + extra_mem;
> +}
> +
> +static inline struct page_ext *get_entry_base(void *base, unsigned long offset)
> +{
> +	return base + get_entry_size() * offset;
> +}

Why _base()? Why not just get_entry?
Also I find it confusing that the word offset here is different than the 
offset in page_ext_operations. Maybe use "index" instead?

Vlastimil