Why the region area don't decrease 1 in function sanity_check_meminfo?

Hi Russell,

On 10/08/12 16:37, Russell King - ARM Linux wrote:

quoted

With !HIGHMEM, sanity_check_meminfo checks for banks that completely or
partially overlap the vmalloc region. The check for partial overlap checks
__va(bank->start + bank->size) > vmalloc_min, but the last address of the
bank is (bank->start + bank->size -1).

Erm.

Let's say you have a bank at 0x80000000, which maps to 0xc0000000 virtual.
This is 512MB in size (so it's last byte address is 0x9fffffff).  That
places it at at 0xdfffffff.  Now, let's say vmalloc_min is 0xe0000000.

"bank->start + bank->size" would be 0xa0000000, right ?

So, "__va(bank->start + bank->size)" would be 0xe0000000.

Yep, except in our (hypothetical?) case below...

And "0xe0000000 > vmalloc_min" would be false.

Yup... My commit message is wrong. Sorry. The linear case is fine...

quoted

However, theoretically, if using using SPARSEMEM in a situation where the
physical to virtual address conversion is not monotonic increasing, the
incorrect test could result in a bank not being truncated when it should be.

Right, so what you're actually talking about is a non-linear translation
by __va() and friends.  In that case, what you actually need is:

	(__va(bank->start + bank->size - 1) + 1) > vmalloc_min
or
	__va(bank->start + bank->size - 1) >= vmalloc_min

and not

	__va(bank->start + bank->size - 1) > vmalloc_min

Agreed. I prefer the second form, and there seems to be a precedent for
">=" further up in sanity_check_meminfo, too.

If you think it is worth fixing for this case, I'll make the fixes,
ensure the commit message is !wrong and submit this to the patch-system.

Jonny