Re: ioremap()/iounmap() problem

["Matt Gerassimoff" <mgeras@gmail.com>]

On Mon, 19 Jan 2009 08:22:37 -0700, Russell King - ARM Linux  
<linux@arm.linux.org.uk> wrote:

> On Mon, Jan 19, 2009 at 08:06:27AM -0700, Matt Gerassimoff wrote:
>> 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.

> The solution is to fix iounmap() to use proper interfaces into  
> mm/vmalloc.c
> when 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.
>
>> 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.

Ok, that makes sense.

>> 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?

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.

-- 
Matt