linux-kernel - Re: [PATCH 4/5] mm/page_ext: support extra space allocation by page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <43c12773-6ee6-b7c3-6357-67180e5bfd9b@suse.cz>
Date:	Thu, 11 Aug 2016 14:53:52 +0200
From:	Vlastimil Babka <vbabka@...e.cz>
To:	js1304@...il.com, Andrew Morton <akpm@...ux-foundation.org>
Cc:	Minchan Kim <minchan@...nel.org>, Michal Hocko <mhocko@...nel.org>,
	Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Joonsoo Kim <iamjoonsoo.kim@....com>
Subject: Re: [PATCH 4/5] mm/page_ext: support extra space allocation by
 page_ext user

On 08/10/2016 08:16 AM, js1304@...il.com wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@....com>
>
> 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@....com>

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