linux-kernel - Re: [PATCH] use total_highpages when calculating lowmem-only allocation sizes (core)

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: <20101207151054.32542836.akpm@linux-foundation.org>
Date:	Tue, 7 Dec 2010 15:10:54 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	"Jan Beulich" <JBeulich@...ell.com>
Cc:	<linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>
Subject: Re: [PATCH] use total_highpages when calculating lowmem-only
 allocation sizes (core)

On Mon, 06 Dec 2010 16:41:11 +0000
"Jan Beulich" <JBeulich@...ell.com> wrote:

> For those (large) table allocations that come only from lowmem, the
> total amount of memory shouldn't really matter.
> 
> For vfs_caches_init(), in the same spirit also replace the use of
> nr_free_pages() by nr_free_buffer_pages().
> 
> Signed-off-by: Jan Beulich <jbeulich@...ell.com>
> 
> ---
>  fs/dcache.c                       |    4 ++--
>  init/main.c                       |    5 +++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> --- linux-2.6.37-rc4/fs/dcache.c
> +++ 2.6.37-rc4-use-totalhigh_pages/fs/dcache.c
> @@ -2474,10 +2474,10 @@ void __init vfs_caches_init(unsigned lon
>  {
>  	unsigned long reserve;
>  
> -	/* Base hash sizes on available memory, with a reserve equal to
> +	/* Base hash sizes on available lowmem memory, with a reserve equal to
>             150% of current kernel size */
>  
> -	reserve = min((mempages - nr_free_pages()) * 3/2, mempages - 1);
> +	reserve = min((mempages - nr_free_buffer_pages()) * 3/2, mempages - 1);
>  	mempages -= reserve;
>  
>  	names_cachep = kmem_cache_create("names_cache", PATH_MAX, 0,
> --- linux-2.6.37-rc4/init/main.c
> +++ 2.6.37-rc4-use-totalhigh_pages/init/main.c
> @@ -22,6 +22,7 @@
>  #include <linux/init.h>
>  #include <linux/initrd.h>
>  #include <linux/bootmem.h>
> +#include <linux/highmem.h>
>  #include <linux/acpi.h>
>  #include <linux/tty.h>
>  #include <linux/percpu.h>
> @@ -673,13 +674,13 @@ asmlinkage void __init start_kernel(void
>  #endif
>  	thread_info_cache_init();
>  	cred_init();
> -	fork_init(totalram_pages);
> +	fork_init(totalram_pages - totalhigh_pages);
>  	proc_caches_init();
>  	buffer_init();
>  	key_init();
>  	security_init();
>  	dbg_late_init();
> -	vfs_caches_init(totalram_pages);
> +	vfs_caches_init(totalram_pages - totalhigh_pages);
>  	signals_init();
>  	/* rootfs populating might need page-writeback */
>  	page_writeback_init();

Dunno.  The code is really quite confused, unobvious and not obviously
correct.

Mainly because it has callers who read some global state and then pass
that into callees who take that arg and then combine it with other
global state.  The code would be much more confidence-inspiring if it
were cleaned up, so that all callees just read the global state when
they need it.

And is there any significant difference between (totalram_pages -
totalhigh_pages) and nr_free_buffer_pages()?  They're both kind-of
evaluating the same thing?

And after this patch, vfs_caches_init() is evaluating

	totalram_pages - totalhigh_pages - nr_free_buffer_pages()

which will be pretty close to zero, won't it?  Maybe negative?  Does
the code actually work??

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/