linux-kernel - Re: [PATCH] ARM: Fix page counting in mem_init and show

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: <20121129130844.GI19440@n2100.arm.linux.org.uk>
Date:	Thu, 29 Nov 2012 13:08:44 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Michael Spang <spang@...omium.org>
Cc:	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ARM: Fix page counting in mem_init and show_mem

On Mon, Oct 22, 2012 at 09:34:51PM -0400, Michael Spang wrote:
>  	for_each_bank (i, mi) {
>  		struct membank *bank = &mi->bank[i];
> -		unsigned int pfn1, pfn2;
> -		struct page *page, *end;
> +		unsigned int start, end, pfn;
>  
> -		pfn1 = bank_pfn_start(bank);
> -		pfn2 = bank_pfn_end(bank);
> +		start = bank_pfn_start(bank);
> +		end = bank_pfn_end(bank);
>  
> -		page = pfn_to_page(pfn1);
> -		end  = pfn_to_page(pfn2 - 1) + 1;
> +		for (pfn = start; pfn < end; pfn++) {
> +			struct page *page;
> +
> +			if (!pfn_valid(pfn))
> +				continue;

This is not a very good fix; what this means is that we end up calling
pfn_valid() for each and every page in the system, and as pfn_valid()
may not be a simple test (but a search) we should avoid that when we're
iterating over all pages in the system.

Firstly, the mem blank information is assumed from the very beginning
to be aligned with the sparsemem split-up.  This comes from the previous
discontiguous implementation where this was an absolute requirement.  We
continue to require that.

Secondly, if you're worred about the stolen memory, then we need to be
iterating over the memblock information instead of the membank information.
This is slightly more complex because memblock will merge neighbouring
regions into one contiguous entry - and this needs to be split up here.
This is why I persisted with the membank stuff here as that _should_
already be appropriately split.

In the long run though, moving to memblock and dealing better with the
split memory maps (rather than looking up each and every page using
pfn_to_page()) is the right way to go.
--
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/