ioremap()/iounmap() problem
Hi,
Running with latest linux-omap kernel on OMAP3 SDP board, I have problem with iounmap(). It looks like iounmap() does not properly free large areas. Below is a test which fails for me in 6-7 loops.
OMAP spesific ioremap code doesn't do much, so I think it's somewhere in generic ARM code. I looked at the ioremap code and for larger areas the code uses area sections, and I believe the bug is somewhere there.
Does this work on other processors?
Tomi
#include <linux/module.h>
#include <linux/io.h>
static int test_init(void)
{
const unsigned long paddr = 0x70000000;
const unsigned long size = 2048 * 2048 * 4;
void *vaddr;
int i;
for (i = 0; i < 200; ++i) {
vaddr = ioremap(paddr, size);
if (!vaddr) {
printk("couldn't ioremap\n");
break;
}
printk("ioremapped to %p\n", vaddr);
iounmap(vaddr);
}
return 0;
}static void test_exit(void) { }
module_init(test_init);module_exit(test_exit);
Quoted 9 lines · Tomi Valkeinen
Hi,
Running with latest linux-omap kernel on OMAP3 SDP board, I have problem with
iounmap(). It looks likeiounmap()does not properly free large areas. Below is a test which fails for me in 6-7 loops.OMAP spesific ioremap code doesn't do much, so I think it's somewhere in generic ARM code. I looked at the ioremap code and for larger areas the code uses area sections, and I believe the bug is somewhere there.
In unmap_area_sections(), try changing:
unsigned long addr = virt, end = virt + (size & ~SZ_1M);
to
unsigned long addr = virt, end = virt + (size & ~(SZ_1M - 1));
Hi,
Quoted 11 lines · ext Russell King - ARM Linux
Quoted 9 lines · Tomi Valkeinen
Hi,
Running with latest linux-omap kernel on OMAP3 SDP board, I have problem with
iounmap(). It looks likeiounmap()does not properly free large areas. Below is a test which fails for me in 6-7 loops.OMAP spesific ioremap code doesn't do much, so I think it's somewhere in generic ARM code. I looked at the ioremap code and for larger areas the code uses area sections, and I believe the bug is somewhere there.
Quoted 7 lines
In
unmap_area_sections(), try changing:unsigned long addr = virt, end = virt + (size & ~SZ_1M);to
unsigned long addr = virt, end = virt + (size & ~(SZ_1M - 1));
This didn't help. I Added two debug prints, not sure if it means anything but the size on the second unmap is not the same as in the previous ones.
remap_area_sections(virt=d2000000, size=1000000, end=d3000000, pfn=70000) unmap_area_sections(virt=d2000000, size=1000000, end=d3000000) ioremapped to d2000000unmap_area_sections(virt=d2000000, size=1001000, end=d3000000)
Tomi
Quoted 16 lines · Tomi Valkeinen
Quoted 15 lines · ext Russell King - ARM Linux
Quoted 9 lines · Tomi Valkeinen
Hi,
Running with latest linux-omap kernel on OMAP3 SDP board, I have problem with
iounmap(). It looks likeiounmap()does not properly free large areas. Below is a test which fails for me in 6-7 loops.OMAP spesific ioremap code doesn't do much, so I think it's somewhere in generic ARM code. I looked at the ioremap code and for larger areas the code uses area sections, and I believe the bug is somewhere there.
In
unmap_area_sections(), try changing:unsigned long addr = virt, end = virt + (size & ~SZ_1M);
Quoted 7 lines
Quoted 3 lines
to
unsigned long addr = virt, end = virt + (size & ~(SZ_1M - 1));This didn't help. I Added two debug prints, not sure if it means anything but the size on the second unmap is not the same as in the previous ones.
If you read the comments on unmap_area_sections, you'd understand why this is.
Quoted 4 lines
remap_area_sections(virt=d2000000, size=1000000, end=d3000000, pfn=70000)unmap_area_sections(virt=d2000000, size=1000000, end=d3000000)ioremapped tod2000000unmap_area_sections(virt=d2000000, size=1001000, end=d3000000)
Yes, so changing that line results in 'end' being the correct 0x1000000 bytes greater than 'virt', as can be seen from the above. Before this change, end would've been 0xd3001000 which is incorrect.
Now, if there's something else wrong, please be more expansive about it. Please given an exact description of the failure you're seeing. (I'm not going to be able to try your test program for several days.)
Quoted 33 lines · ext Russell King - ARM Linux
Quoted 16 lines · Tomi Valkeinen
Quoted 15 lines · ext Russell King - ARM Linux
Quoted 9 lines · Tomi Valkeinen
Hi,
Running with latest linux-omap kernel on OMAP3 SDP board, I have problem with
iounmap(). It looks likeiounmap()does not properly free large areas. Below is a test which fails for me in 6-7 loops.OMAP spesific ioremap code doesn't do much, so I think it's somewhere in generic ARM code. I looked at the ioremap code and for larger areas the code uses area sections, and I believe the bug is somewhere there.
In
unmap_area_sections(), try changing:unsigned long addr = virt, end = virt + (size & ~SZ_1M);Quoted 7 lines
Quoted 3 lines
to
unsigned long addr = virt, end = virt + (size & ~(SZ_1M - 1));This didn't help. I Added two debug prints, not sure if it means anything but the size on the second unmap is not the same as in the previous ones.
If you read the comments on
unmap_area_sections, you'd understand why this is.Quoted 4 lines
remap_area_sections(virt=d2000000, size=1000000, end=d3000000, pfn=70000)unmap_area_sections(virt=d2000000, size=1000000, end=d3000000)ioremapped tod2000000unmap_area_sections(virt=d2000000, size=1001000, end=d3000000)
Quoted 7 lines
Yes, so changing that line results in 'end' being the correct
0x1000000bytes greater than 'virt', as can be seen from the above. Before this change, end would've been0xd3001000which is incorrect.Now, if there's something else wrong, please be more expansive about it. Please given an exact description of the failure you're seeing. (I'm not going to be able to try your test program for several days.)
The original problem remains, it looks to me that iounmap() doesn't free the memory, and thus ioremap will fail after couple of loops.
I get the following error on console:
vmap allocation for size 16781312 failed: use vmalloc=<size> to increase size.
I don't know too much about memory management, but I'll try to dig deeper in to this.
What I don't understand is that in __arm_ioremap_pfn() there's get_vm_area(), and later in the function, in case of error, that area is vunmap()ed. But there's no vunmap() done anywhere else in the code for area sections, so who does free it?
Tomi
Quoted 8 lines · Tomi Valkeinen
The original problem remains, it looks to me that
iounmap()doesn't free the memory, and thus ioremap will fail after couple of loops.I get the following error on console:
vmap allocation for size 16781312 failed: use vmalloc=<size> to increase size.
Quoted 7 lines
I don't know too much about memory management, but I'll try to dig deeper in to this.
What I don't understand is that in
__arm_ioremap_pfn()there'sget_vm_area(), and later in the function, in case of error, that area is vunmap()ed. But there's novunmap()done anywhere else in the code for area sections, so who does free it?
Yes, the error I pointed out is not the only issue with this code.
You might consider trying my patch which was sent in this thread to Matt Gerassimoff. And if you see anyone whinging that it won't work for whatever reason, please ignore them and just test the patch. You will not be disappointed.
Hi,
Quoted 17 lines · ext Russell King - ARM Linux
Quoted 8 lines · Tomi Valkeinen
The original problem remains, it looks to me that
iounmap()doesn't free the memory, and thus ioremap will fail after couple of loops.I get the following error on console:
vmap allocation for size 16781312 failed: use vmalloc=<size> to increase size.
Quoted 7 lines
I don't know too much about memory management, but I'll try to dig deeper in to this.
What I don't understand is that in
__arm_ioremap_pfn()there'sget_vm_area(), and later in the function, in case of error, that area is vunmap()ed. But there's novunmap()done anywhere else in the code for area sections, so who does free it?
Quoted 6 lines
Yes, the error I pointed out is not the only issue with this code.
You might consider trying my patch which was sent in this thread to Matt Gerassimoff. And if you see anyone whinging that it won't work for whatever reason, please ignore them and just test the patch. You will not be disappointed.
Sorry it took so long to test. The patch indeed works fine, thanks! And the test loop works also fine even without any schedulings, contrary of what was speculated. (Not that it matters in real usage).
I'll do some more testing and report if I see any oddities.
Tomi
Quoted 9 lines · Tomi Valkeinen
Quoted 5 lines
You might consider trying my patch which was sent in this thread to Matt Gerassimoff. And if you see anyone whinging that it won't work for whatever reason, please ignore them and just test the patch. You will not be disappointed.
Sorry it took so long to test. The patch indeed works fine, thanks! And the test loop works also fine even without any schedulings, contrary of what was speculated. (Not that it matters in real usage).
I've tested it as well. I was on a 2.6.28-rc branch initially. I pulled the latest linus git sources and tested the patch. It works.
I stand corrected and now I'll stop whining.
Signature · 1 line
Matt
Quoted 9 lines
Running with latest linux-omap kernel on OMAP3 SDP board, I have problem with
iounmap(). It looks likeiounmap()does not properly free large areas. Below is a test which fails for me in 6-7 loops.OMAP spesific ioremap code doesn't do much, so I think it's somewhere in generic ARM code. I looked at the ioremap code and for larger areas the code uses area sections, and I believe the bug is somewhere there.
Does this work on other processors?
If you wait around a bit using some kind of schedule() + jiffy wait after free will it work longer?
Every now and then I've seen some deferred resource releases cause failures of such loops. Usually they are more obvious like if your really malloc/free'ing direct memory, but there are indirect allocations which might add up.
Anyway, it's a behavioral question to help isolate.
Regards,
Richard W.
Quoted 13 lines · Richard
Quoted 9 lines
Running with latest linux-omap kernel on OMAP3 SDP board, I have problem with
iounmap(). It looks likeiounmap()does not properly free large areas. Below is a test which fails for me in 6-7 loops.OMAP spesific ioremap code doesn't do much, so I think it's somewhere in generic ARM code. I looked at the ioremap code and for larger areas the code uses area sections, and I believe the bug is somewhere there.
Does this work on other processors?
If you wait around a bit using some kind of
schedule()+ jiffy wait after free will it work longer?
Quoted 6 lines
Every now and then I've seen some deferred resource releases cause failures of such loops. Usually they are more obvious like if your really malloc/free'ing direct memory, but there are indirect allocations which might add up.
Anyway, it's a behavioral question to help isolate.
There is no deferred resource releasing here, so such a test is a waste of time.
Quoted 20 lines · Richard
Quoted 18 lines
Quoted 9 lines
Running with latest linux-omap kernel on OMAP3 SDP board, I have problem with
iounmap(). It looks likeiounmap()does not properly free large areas. Below is a test which fails for me in 6-7 loops.OMAP spesific ioremap code doesn't do much, so I think it's somewhere in generic ARM code. I looked at the ioremap code and for larger areas the code uses area sections, and I believe the bug is somewhere there.
Does this work on other processors?
If you wait around a bit using some kind of
schedule()+ jiffy wait after free will it work longer?Every now and then I've seen some deferred resource releases cause failures of such loops. Usually they are more obvious like if your really malloc/free'ing direct memory, but there are indirect allocations which might add up.
Quoted 4 lines
Quoted 1 line
Anyway, it's a behavioral question to help isolate.
There is no deferred resource releasing here, so such a test is a waste of time.
None? Aren't at least TLB table structures allocated to cover memory but some other tables?
Or are those all synchronous allocate and release? I was under some impression that kmalloc's to get some of these came from slabs which don't reap right away.
Defiantly we have had issues in video memory before in these kinds of test loops.
Are you simplifying or am I just off.
Thanks,
Richard W.
Quoted 25 lines · Richard
Quoted 20 lines · Richard
Quoted 18 lines
Quoted 9 lines
Running with latest linux-omap kernel on OMAP3 SDP board, I have problem with
iounmap(). It looks likeiounmap()does not properly free large areas. Below is a test which fails for me in 6-7 loops.OMAP spesific ioremap code doesn't do much, so I think it's somewhere in generic ARM code. I looked at the ioremap code and for larger areas the code uses area sections, and I believe the bug is somewhere there.
Does this work on other processors?
If you wait around a bit using some kind of
schedule()+ jiffy wait after free will it work longer?Every now and then I've seen some deferred resource releases cause failures of such loops. Usually they are more obvious like if your really malloc/free'ing direct memory, but there are indirect allocations which might add up.
Quoted 4 lines
Quoted 1 line
Anyway, it's a behavioral question to help isolate.
There is no deferred resource releasing here, so such a test is a waste of time.
Quoted 6 lines
None? Aren't at least TLB table structures allocated to cover memory but some other tables?
Or are those all synchronous allocate and release? I was under some impression that kmalloc's to get some of these came from slabs which don't reap right away.
There are no L2 PTE tables for section and supersection mappings, so this is irrelevent. Section and supersection mappings live in the L1 page table which persists for the lifetime of the task.
As such, ioremap/iounmap are only allocating and freeing a small structure to account for the allocation - if things are so tight that 6-7 loops causes that allocation to fail, you're not going to be able to run any userspace programs.
This is clearly a bug report which is too vague - which talks about a "failure" but doesn't say what the failure is. Until we have more information from the reporter, uninformed speculation is rather wasteful.
Let's wait until the reporter tells us what the nature of this failure is, as I've already requested an hour ago.
Quoted 8 lines · Russell King - ARM Linux
There are no L2 PTE tables for section and supersection mappings, so this is irrelevent. Section and supersection mappings live in the L1 page table which persists for the lifetime of the task.
As such, ioremap/iounmap are only allocating and freeing a small structure to account for the allocation - if things are so tight that 6-7 loops causes that allocation to fail, you're not going to be able to run any userspace programs.
This issue is the reason I have subscribed to the mailing list. I have discovered the problem and had a quick patch to solve it. Basically when I call ioremap() and then iounmap() (loading and unloading a module) successively, I get a different virtual address each time. Eventually, it causes an invalid pointer and causes and OOPS. The solution is not optimal but it may point to the actual problem. The call to ioremap() is a macro that eventually gets to a routine __arm_ioremap_pfn which is implemented in arch/arm/mm/ioremap.c. This function calls get_vm_area() which calls __get_vm_area_node() (both within mm/vmalloc.c). Finally, there is a routine called alloc_vmap_area() which allocates and initialized a struct vmap_area. The __iounmap() function (also implemented in arch/arm/mm/ioremap.c) never takes the
struct vmap_area into account. Looking through the sources, it's not easy to get
a pointer to that structure from a struct vm_struct pointer. So I went back to __arm_ioremap_pfn() and found a line that is completely baffling. I #ifdef'd it out to test and it completely solved the problem. Here is a simple patch to offending area of code:
--- arch/arm/mm/ioremap.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
arch/arm/mm/ioremap.c +0 -0
@@ -297,9 +297,11 @@ __arm_ioremap_pfn(unsigned long pfn, unsigned long offset, !((__pfn_to_phys(pfn) | size | addr) & ~SUPERSECTION_MASK))
{
area->flags |= VM_ARM_SECTION_MAPPING;
err = remap_area_supersections(addr, pfn, size, type);
+#if 0
} else if (!((__pfn_to_phys(pfn) | size | addr) & ~PMD_MASK)) {
area->flags |= VM_ARM_SECTION_MAPPING;
err = remap_area_sections(addr, pfn, size, type);
+#endif
} else
#endif
err = remap_area_pages(addr, pfn, size, type);--
1.6.0.6
The statement:
} else if (!((__pfn_to_phys(pfn) | size | addr) & ~PMD_MASK)) {is the strange one. I'm not what is being checked here except the PMD_MASK. But without that code, everything works 100%. I'm not sure what all the remap_area_sections() code does, but the cleanup definitely does not work, as the kernel OOPS will testify. There may be a better solution, but as far as I can tell, it's not really needed. Maybe someone else will disagree.
Matt
Quoted 2 lines · Matt Gerassimoff
This issue is the reason I have subscribed to the mailing list. I have discovered the problem and had a quick patch to solve it.
It's not a solution, it's a work-around. You're not seeing the problem anymore because you've changed the code to avoid using section mappings.
That's fine if you want to eat lots of TLB entries for your mapping.
Quoted 4 lines
The call to
ioremap()is a macro that eventually gets to a routine__arm_ioremap_pfnwhich is implemented inarch/arm/mm/ioremap.c. This function callsget_vm_area()which calls__get_vm_area_node()(both withinmm/vmalloc.c).
Ok so far.
Quoted 2 lines
Finally, there is a routine called
alloc_vmap_area()which allocates and initialized a structvmap_area.
And this is why we now have the problem. When the ARM section based ioremap implementation was written, none of that additional code existed.
The reason we've ended up in this situation is because iounmap() is doing the _insane_ thing and implementing its own method of freeing independent of mm/vmalloc.c - this is called "fragile coding" and must be avoided.
The solution is to fix iounmap() to use proper interfaces into mm/vmalloc.c when removing the section and supersection mappings.
Quoted 6 lines
The statement:
} else if (!((__pfn_to_phys(pfn) | size | addr) & ~PMD_MASK)) {is the strange one. I'm not what is being checked here except the
PMD_MASK.
(a | b | c) & mask (a & mask) | (b & mask) | (c & mask) (a & mask) || (b & mask) || (c & mask)
and PMD_MASK is used to mask off the offset into a 2MB section. So, (a & ~PMD_MASK) gives the offset into the 2MB section.
So, the test is:
- is the physical address aligned to 2MB
and
- is the size aligned to 2MB
and
- is the virtual address aligned to 2MB
then
map using section mappings.
Quoted 7 lines
But without that code, everything works 100%. I'm not sure what all the
remap_area_sections()
code does, but the cleanup definitely does not work, as the kernel OOPS will testify.
There may be a better solution, but as far as I can tell, it's not really needed. Maybe
someone else will disagree.
We might as well rip this code out then. I'm all for simpler code, but I'm sure the folk who want to squeeze the best performance out of their machines will quickly squeel if I did that.
And what cleanup are you referring to?
Quoted 6 lines · Russell King - ARM Linux
Quoted 2 lines · Matt Gerassimoff
This issue is the reason I have subscribed to the mailing list. I have discovered the problem and had a quick patch to solve it.
It's not a solution, it's a work-around. You're not seeing the problem anymore because you've changed the code to avoid using section mappings.
That's what I said at the end of the first message.
Quoted 2 lines
The solution is to fix
iounmap()to use proper interfaces intomm/vmalloc.cwhen removing the section and supersection mappings.
After looking at how the the vmap_area's are managed, that's going to be a tough one.
Quoted 19 lines
Quoted 7 lines
The statement:
} else if (!((__pfn_to_phys(pfn) | size | addr) & ~PMD_MASK)) {is the strange one. I'm not what is being checked here except the
PMD_MASK.(a | b | c) & mask (a & mask) | (b & mask) | (c & mask) (a & mask) || (b & mask) || (c & mask)and
PMD_MASKis used to mask off the offset into a 2MB section. So, (a & ~PMD_MASK) gives the offset into the 2MB section.So, the test is:
Quoted 7 lines
- is the physical address aligned to 2MB
and
- is the size aligned to 2MB
and
- is the virtual address aligned to 2MB
then
map using section mappings.
Ok, that makes sense.
Quoted 8 lines
Quoted 7 lines
But without that code, everything works 100%. I'm not sure what all the
remap_area_sections()
code does, but the cleanup definitely does not work, as the kernel OOPS will testify.
There may be a better solution, but as far as I can tell, it's not really needed. Maybe
someone else will disagree.
Quoted 5 lines
We might as well rip this code out then. I'm all for simpler code, but I'm sure the folk who want to squeeze the best performance out of their machines will quickly squeel if I did that.
And what cleanup are you referring to?
I meant freeing, not cleanup. Sorry.
As I said it's a start. I'm all for performance but not at the expense of integrity. Right now the code doesn't work and needs to be fixed.
Signature · 1 line
Matt
Quoted 15 lines · Matt Gerassimoff
Quoted 15 lines
Quoted 8 lines
But without that code, everything works 100%. I'm not sure what all the
remap_area_sections()
code does, but the cleanup definitely does not work, as the kernel OOPS will testify.
There may be a better solution, but as far as I can tell, it's not really
needed. Maybe
someone else will disagree.We might as well rip this code out then. I'm all for simpler code, but I'm
sure the folk who want to squeeze the best performance out of their machines
will quickly squeel if I did that.
Quoted 3 lines
Quoted 1 line
And what cleanup are you referring to?
I meant freeing, not cleanup. Sorry.
Okay, I won't include that bug fix with the fix I'm going to be working on, even though it's clearly a bug in its own right. After all, it "definitely does not work" in your words.
(It may not fix the problem _you're_ seeing, but I assure you, that code as it stands is wrong.)
Quoted 2 lines
As I said it's a start. I'm all for performance but not at the expense of integrity. Right now the code doesn't work and needs to be fixed.
Yes, I think everyone realises that there is a bug here which needs fixing.
Unfortunately, vague "it fails" bug reports are utterly useless for finding bugs. Your report provided that extra bit of information which points to where the real problem lies.
Now, I could be looking at finishing off what I'm currently working on so I can move on to fixing it instead of writing this message in reply to an obvious statement.
Quoted 33 lines · Russell King - ARM Linux
Quoted 15 lines · Matt Gerassimoff
Quoted 15 lines
Quoted 8 lines
But without that code, everything works 100%. I'm not sure what all the
remap_area_sections()
code does, but the cleanup definitely does not work, as the kernel OOPS will testify.
There may be a better solution, but as far as I can tell, it's not really
needed. Maybe
someone else will disagree.We might as well rip this code out then. I'm all for simpler code, but I'm
sure the folk who want to squeeze the best performance out of their machines
will quickly squeel if I did that.Quoted 3 lines
Quoted 1 line
And what cleanup are you referring to?
I meant freeing, not cleanup. Sorry.
Okay, I won't include that bug fix with the fix I'm going to be working on, even though it's clearly a bug in its own right. After all, it "definitely does not work" in your words.
(It may not fix the problem _you're_ seeing, but I assure you, that code as it stands is wrong.)
Quoted 2 lines
As I said it's a start. I'm all for performance but not at the expense of integrity. Right now the code doesn't work and needs to be fixed.
Yes, I think everyone realises that there is a bug here which needs fixing.
Quoted 7 lines
Unfortunately, vague "it fails" bug reports are utterly useless for finding bugs. Your report provided that extra bit of information which points to where the real problem lies.
Now, I could be looking at finishing off what I'm currently working on so I can move on to fixing it instead of writing this message in reply to an obvious statement.
Right, ignoring everything about "that fix definitely does not work" here's an as yet untested patch which should fix the problem.
The other thing to point out is that vunmap() does its work lazily (as Richard tried to point out). However, it'll become unlazy when it has more than 32MB * NR_CPUS to deal with. So, make sure that your ioremap/vmalloc region is larger than 32MB otherwise you'll still see crashes.
(That being the space between top of RAM + 8MB, and VMALLOC_END as seen in Documentation/arm/memory.txt.)
arch/arm/mm/ioremap.c +2 -9
@@ -138,7 +138,7 @@ void __check_kvm_seq(struct mm_struct *mm)*/ static void unmap_area_sections (unsigned long virt,unsigned long size) { -unsigned long addr = virt, end = virt + (size & ~SZ_1M ); +unsigned long addr = virt, end = virt + (size & ~(SZ_1M -1 ));pgd_t *pgd;flush_cache_vunmap (addr, end);
@@ -337,10 +337,7 @@ void __iounmap(volatile void __iomem *io_addr)void *addr = (void *)(PAGE_MASK & (unsigned long )io_addr);#ifndef CONFIG_SMP struct vm_struct **p, *tmp; -#endif -unsigned int section_mapping =0 ; -#ifndef CONFIG_SMP /* * If this is a section based mapping we need to handle it * specially as the VM subsystem does not know how to handle
@@ -352,11 +349,8 @@ void __iounmap(volatile void __iomem *io_addr)for (p = &vmlist ; (tmp = *p) ; p = &tmp->next) {if ((tmp->flags &VM_IOREMAP ) && (tmp->addr == addr)) {if (tmp->flags &VM_ARM_SECTION_MAPPING ) { - *p = tmp->next;unmap_area_sections ((unsigned long )tmp->addr, tmp->size); -kfree (tmp); - section_mapping =1 ; }break ; }
@@ -364,7 +358,6 @@ void __iounmap(volatile void __iomem *io_addr)write_unlock (&vmlist_lock);#endif -if (!section_mapping) -vunmap (addr); +vunmap (addr); }EXPORT_SYMBOL (__iounmap);
Quoted 35 lines · Russell King - ARM Linux
arch/arm/mm/ioremap.c +1
-7@@ -138,7 +138,7 @@ void __check_kvm_seq(struct mm_struct *mm)*/ static void unmap_area_sections (unsigned long virt,unsigned long size) { -unsigned long addr = virt, end = virt + (size & ~SZ_1M ); +unsigned long addr = virt, end = virt + (size & ~(SZ_1M -1 ));pgd_t *pgd;flush_cache_vunmap (addr, end);@@ -337,10 +337,7 @@ void __iounmap(volatile void __iomem *io_addr)void *addr = (void *)(PAGE_MASK & (unsigned long )io_addr);#ifndef CONFIG_SMP struct vm_struct **p, *tmp; -#endif -unsigned int section_mapping =0 ; -#ifndef CONFIG_SMP /* * If this is a section based mapping we need to handle it * specially as the VM subsystem does not know how to handle @@ -352,11 +349,8 @@ void __iounmap(volatile void __iomem *io_addr)for (p = &vmlist ; (tmp = *p) ; p = &tmp->next) {if ((tmp->flags &VM_IOREMAP ) && (tmp->addr == addr)) {if (tmp->flags &VM_ARM_SECTION_MAPPING ) { - *p = tmp->next;unmap_area_sections ((unsigned long )tmp->addr, tmp->size); -kfree (tmp); - section_mapping =1 ; }break ; }
arch/arm/mm/ioremap.c void __iounmap(volatile void __iomem *io_addr) · line 362
Quoted 7 lines
arch/arm/mm/ioremap.c +1
-2@@ -364,7 +358,6 @@ void __iounmap(volatile void __iomem *io_addr)write_unlock (&vmlist_lock);#endif -if (!section_mapping) -vunmap (addr); +vunmap (addr); }EXPORT_SYMBOL (__iounmap);
Before this is even tested. I've went down this path. You will receive a ton of Bad PMD errors. unmap_area_sections() and vunmap() don't play nice together. This type of solution will require changes to unmap_area_sections().
Signature · 1 line
Matt
Quoted 43 lines · Matt Gerassimoff
Quoted 40 lines · Russell King - ARM Linux
arch/arm/mm/ioremap.c +2
-9@@ -138,7 +138,7 @@ void __check_kvm_seq(struct mm_struct *mm)*/ static void unmap_area_sections (unsigned long virt,unsigned long size) { -unsigned long addr = virt, end = virt + (size & ~SZ_1M ); +unsigned long addr = virt, end = virt + (size & ~(SZ_1M -1 ));pgd_t *pgd;flush_cache_vunmap (addr, end);@@ -337,10 +337,7 @@ void __iounmap(volatile void __iomem *io_addr)void *addr = (void *)(PAGE_MASK & (unsigned long )io_addr);#ifndef CONFIG_SMP struct vm_struct **p, *tmp; -#endif -unsigned int section_mapping =0 ; -#ifndef CONFIG_SMP /* * If this is a section based mapping we need to handle it * specially as the VM subsystem does not know how to handle @@ -352,11 +349,8 @@ void __iounmap(volatile void __iomem *io_addr)for (p = &vmlist ; (tmp = *p) ; p = &tmp->next) {if ((tmp->flags &VM_IOREMAP ) && (tmp->addr == addr)) {if (tmp->flags &VM_ARM_SECTION_MAPPING ) { - *p = tmp->next;unmap_area_sections ((unsigned long )tmp->addr, tmp->size); -kfree (tmp); - section_mapping =1 ; }break ; }@@ -364,7 +358,6 @@ void __iounmap(volatile void __iomem *io_addr)write_unlock (&vmlist_lock);#endif -if (!section_mapping) -vunmap (addr); +vunmap (addr);
Quoted 8 lines
arch/arm/mm/ioremap.c void __iounmap(volatile void __iomem *io_addr) · line 363
Quoted 2 lines
arch/arm/mm/ioremap.c +0
-0}EXPORT_SYMBOL (__iounmap);Before this is even tested. I've went down this path. You will receive a ton
of Bad PMD errors.unmap_area_sections()andvunmap()don't play nice together.
This type of solution will require changes tounmap_area_sections().
Okay, I've just lost interest in persuing this bug any further.
Quoted 54 lines · Russell King - ARM Linux
Quoted 44 lines · Matt Gerassimoff
Quoted 7 lines · Russell King - ARM Linux
arch/arm/mm/ioremap.c +0
-0@@ -138,7 +138,7 @@ void __check_kvm_seq(struct mm_struct *mm)*/ static void unmap_area_sections(unsigned long virt, unsigned long
size)
Quoted 33 lines
{
arch/arm/mm/ioremap.c +1
-1-unsigned long addr = virt, end = virt + (size & ~SZ_1M ); +unsigned long addr = virt, end = virt + (size & ~(SZ_1M -1 ));pgd_t *pgd; flush_cache_vunmap(addr, end);arch/arm/mm/ioremap.c +1
-8@@ -337,10 +337,7 @@ void __iounmap(volatile void __iomem *io_addr)void *addr = (void *)(PAGE_MASK & (unsigned long )io_addr);#ifndef CONFIG_SMP struct vm_struct **p, *tmp; -#endif -unsigned int section_mapping =0 ; -#ifndef CONFIG_SMP /* * If this is a section based mapping we need to handle it * specially as the VM subsystem does not know how to handle @@ -352,11 +349,8 @@ void __iounmap(volatile void __iomem *io_addr)for (p = &vmlist ; (tmp = *p) ; p = &tmp->next) {if ((tmp->flags &VM_IOREMAP ) && (tmp->addr == addr)) {if (tmp->flags &VM_ARM_SECTION_MAPPING ) { - *p = tmp->next;unmap_area_sections ((unsigned long )tmp->addr, tmp->size); -kfree (tmp); - section_mapping =1 ; }break ; }@@ -364,7 +358,6 @@ void __iounmap(volatile void __iomem *io_addr)write_unlock (&vmlist_lock);#endif -if (!section_mapping) -vunmap (addr); +vunmap (addr);Quoted 8 lines
arch/arm/mm/ioremap.c void __iounmap(volatile void __iomem *io_addr) · line 363
Quoted 2 lines
arch/arm/mm/ioremap.c +0
-0}EXPORT_SYMBOL (__iounmap);Before this is even tested. I've went down this path. You will receive a ton
of Bad PMD errors.unmap_area_sections()andvunmap()don't play nice together.
This type of solution will require changes tounmap_area_sections().
Quoted 1 line
Okay, I've just lost interest in persuing this bug any further.
I mis-spoke, the changes required are in remap_area_sections(). They are not doing proper manipulations to the pmd information. If remap_area_sections() is fixed, there will be no need for unmap_area_sections() as vunmap() will do all the work as far as I can tell.
From that statement you just made, where does that leave us? This in my estimation is a MAJOR issue. If you need help testing this, I'll be happy to do so. It may be faster for you to create a simple kernel module that ioremap() on module init and iounmep() on module exit. Printing the returned virtual address from ioremap() on each load will yeild a higher and higher addresses until it returns a NULL pointer and kernel OOPS. I'm running on a at91sam9260 and it will fail on about 6 to 10 load/unload cycles of the kernel module. So it happens pretty fast.
Signature · 1 line
Matt
Quoted 5 lines
The other thing to point out is that
vunmap()does its work lazily (as Richard tried to point out). However, it'll become unlazy when it has more than 32MB *NR_CPUSto deal with. So, make sure that your ioremap/vmalloc region is larger than 32MB otherwise you'll still see crashes.
Where we felt the most problems in past years was in low physical memory situations with this kind of looping.
The system would get caught in a live lock in rebalance code. That code tries to shrink caches but needs some allocations for it to happen. Maybe 2.6.29 code is better in this regard.
The other common but easier to fix spot was test code where user space mmap callers would spin in alloc and free. The MMAP calls would always fail after a while.
I've not hear reports for a while so maybe current kernel is great here or the tests aren't being run anymore.
Glad RMK has found the root cause for this issue.
Regards,
Richard W.