Messages in this thread Patch in this message | | | Date | Mon, 02 Jul 2007 12:08:18 -0700 | From | Jeremy Fitzhardinge <> | Subject | Re: [PATCH] ioremap: fix iounmap numpages |
| |
Dave Young wrote: >> On 6/29/07, Jeremy Fitzhardinge <jeremy@goop.org> 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 <hidave.darkstar@gmail.com> for debugging the problem.
Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com> Cc: Dave Young <hidave.darkstar@gmail.com> Cc: Chuck Ebbert <cebbert@redhat.com> Cc: Andi Kleen <ak@suse.de>
--- 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |