In this function, have these codes:
/*
* Check whether this memory bank would partially overlap
* the vmalloc area.
*/
if (__va(bank->start + bank->size) > vmalloc_min ||
__va(bank->start + bank->size) < __va(bank->start)) {
unsigned long newsize = vmalloc_min - __va(bank->start);
printk(KERN_NOTICE "Truncating RAM at %.8llx-%.8llx "
"to -%.8llx (vmalloc region overlap).\n",
(unsigned long long)bank->start,
(unsigned long long)bank->start + bank->size - 1,
(unsigned long long)bank->start + newsize - 1);
bank->size = newsize;
}
Dear all,
I don't known why the memory region have not decrease 1, i think it
should be decreased(like this , (__va(bank->start + bank->size
-1))because in sparse memory system, this address bank->start +
bank->size is belong to another bank, so this method is not correctly.
Please indicate whether my understanding is correct.
Thanks.
Steve
Dear all,
If system has defined CONFIG_SPARSEMEM, that va() and pa() have been
redefiend, like this,
"
#define __phys_to_virt(phys) \
((phys) >= 0x80000000 ? (phys) - 0x80000000 + PAGE_OFFSET2 : \
(phys) >= 0x20000000 ? (phys) - 0x20000000 + PAGE_OFFSET1 : \
(phys) + PAGE_OFFSET)
#define __virt_to_phys(virt) \
((virt) >= PAGE_OFFSET2 ? (virt) - PAGE_OFFSET2 + 0x80000000 : \
(virt) >= PAGE_OFFSET1 ? (virt) - PAGE_OFFSET1 + 0x20000000 : \
(virt) - PAGE_OFFSET)
"
But this line "__va(bank->start + bank->size) < __va(bank->start)"
always assume that the next bank's vaddr is bigger than it.
Yes, we can defined these two MACROS carefully to avoid this problem.
How do you think it. Can you shed some light on what the correct
solution might be?
steve
quoted
Date: Tue, 24 Jul 2012 18:54:30 +0800
From: ??? <redacted>
To: will.deacon at arm.com, catalin.marinas at arm.com
Cc: linux-arm-kernel at lists.infradead.org
Subject: Why the region area don't decrease 1 in function
sanity_check_meminfo?
Message-ID:
[off-list ref]
Content-Type: text/plain; charset=ISO-8859-1
In this function, have these codes:
/*
* Check whether this memory bank would partially overlap
* the vmalloc area.
*/
if (__va(bank->start + bank->size) > vmalloc_min ||
__va(bank->start + bank->size) < __va(bank->start)) {
unsigned long newsize = vmalloc_min - __va(bank->start);
printk(KERN_NOTICE "Truncating RAM at %.8llx-%.8llx "
"to -%.8llx (vmalloc region overlap).\n",
(unsigned long long)bank->start,
(unsigned long long)bank->start + bank->size - 1,
(unsigned long long)bank->start + newsize - 1);
bank->size = newsize;
}
Dear all,
I don't known why the memory region have not decrease 1, i think it
should be decreased(like this , (__va(bank->start + bank->size
-1))because in sparse memory system, this address bank->start +
bank->size is belong to another bank, so this method is not correctly.
Please indicate whether my understanding is correct.
Thanks.
Steve
------------------------------
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
End of linux-arm-kernel Digest, Vol 36, Issue 687
*************************************************
From: Jonathan Austin <hidden> Date: 2012-08-10 15:14:05
On 24/07/12 11:54, ??? wrote:
I don't known why the memory region have not decrease 1, i think it
should be decreased(like this , (__va(bank->start + bank->size
-1))because in sparse memory system, this address bank->start +
bank->size is belong to another bank, so this method is not correctly.
I think you're right - we should have the '- 1' in there, although I
don't think there currently exist any situations where this will cause
us grief. Here's a patch for correctness anyway...
Please indicate whether my understanding is correct.
As far as I can see in all existing situations, the worst effect of this
will be truncating by a single byte a bank that doesn't actually need
truncating.
As you said in your following email, we would get trouble with this if
using SPARSEMEM in a situation where the physical to virtual address
conversion is not monotonic increasing.
Jonny
---8<---
From: Jonathan Austin <redacted>
Date: Thu, 9 Aug 2012 15:59:16 +0100
Subject: [PATCH] arm: mm: Fix vmalloc overlap check for !HIGHMEM
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).
This doesn't cause serious issues for existing platforms, except to
truncate by a single byte a bank that doesn't actually need truncating if
it happens to finish at vmalloc_min -1.
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.
Reported-by: ??? (Steve) <redacted>
Signed-off-by: Jonathan Austin <redacted>
---
arch/arm/mm/mmu.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
From: Russell King - ARM Linux <hidden> Date: 2012-08-10 15:37:05
On Fri, Aug 10, 2012 at 04:14:05PM +0100, Jonathan Austin wrote:
From: Jonathan Austin <redacted>
Date: Thu, 9 Aug 2012 15:59:16 +0100
Subject: [PATCH] arm: mm: Fix vmalloc overlap check for !HIGHMEM
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.
And "0xe0000000 > vmalloc_min" would be false.
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
From: Jonathan Austin <hidden> Date: 2012-08-10 17:43:56
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