Dave Young wrote:
On 6/29/07, Jeremy Fitzhardinge <[email protected]> wrote:
Dave Young wrote:
> Hi,
> The second parameter of change_page_attr in iounmap is wrong, it
should be (p->size - 1) >> PAGE_SHIFT
>
Why's that? Isn't p->size always going to be a pagesize multiple; in
which case, why would you want to change_page_attr on n-1 pages?
Are you seeing a problem that this patch fixes?
J
Hi,
Please read the ioremap_nocache function, the page number is
calculated by:
last_addr = phys_addr + size - 1;
npages = (last_addr - phys_addr) >> PAGE_SHIFT;
but the pages number in iounmap is p->size >> PAGE_SHIFT, the result
is not consistent.
If there's no netsc520 device then the netsc520 mtd driver
initializing will cause oops.
I debugged it with some printk messages, find that the ioremap_nocache
call change_page_attr 256 times, but the iounmap call change_page_attr
more than 256 times, so kernel oops. please finid the oops message:
OK, so the problem is that get_vm_area allocates the vm_area with an
extra page added as a guard page. iounmap uses p->size directly,
without taking the guard page into account.
I don't see why this doesn't cause more problems. I guess uncached
iomappings are not used that much?
Anyway, I think this is the right fix:
Subject: fix iounmap's use of vm_struct's size field
get_vm_area always returns an area with an adjacent guard page. That
guard page is included in vm_struct.size. iounmap uses vm_struct.size
to determine how much address space needs to have change_page_attr
applied to it, which will BUG if applied to the guard page.
This patch adds a helper function - get_vm_area_size() in
linux/vmalloc.h - to return the actual size of a vm area, and uses it
to make iounmap do the right thing. There are probably other places
which should be using get_vm_area_size().
Thanks to Dave Young <[email protected]> for debugging the
problem.
Signed-off-by: Jeremy Fitzhardinge <[email protected]>
Cc: Dave Young <[email protected]>
Cc: Chuck Ebbert <[email protected]>
Cc: Andi Kleen <[email protected]>
---
arch/i386/mm/ioremap.c | 2 +-
include/linux/vmalloc.h | 6 ++++++
2 files changed, 7 insertions(+), 1 deletion(-)
===================================================================
--- a/arch/i386/mm/ioremap.c
+++ b/arch/i386/mm/ioremap.c
@@ -193,7 +193,7 @@ void iounmap(volatile void __iomem *addr
/* Reset the direct mapping. Can block */
if ((p->flags >> 20) && p->phys_addr < virt_to_phys(high_memory) - 1) {
change_page_attr(virt_to_page(__va(p->phys_addr)),
- p->size >> PAGE_SHIFT,
+ get_vm_area_size(p) >> PAGE_SHIFT,
PAGE_KERNEL);
global_flush_tlb();
}
===================================================================
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -70,6 +70,12 @@ extern int map_vm_area(struct vm_struct
struct page ***pages);
extern void unmap_kernel_range(unsigned long addr, unsigned long size);
+static inline size_t get_vm_area_size(const struct vm_struct *area)
+{
+ /* return actual size without guard page */
+ return area->size - PAGE_SIZE;
+}
+
/* Allocate/destroy a 'vmalloc' VM area. */
extern struct vm_struct *alloc_vm_area(size_t size);
extern void free_vm_area(struct vm_struct *area);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
