remap_pfm_range() Linux-2.6.10

remap_pfm_range() Linux-2.6.10

[linux-os]

History....

For many years we had:

 	struct vm_area_struct *vma;

 	remap_page_range(vma->vm_start, base_addr, len, prot)

Then somebody needed the pointer so it became:

 	remap_page_range(vma, vma->vm_start, base_addr, len, prot)

Then they changed its name:

 	remap_pfn_range(vma, vma->vm_start, base_addr >> PAGE_SHIFT, len, prot)

Now, here's the $US0.02 question. Why wasn't PAGE_SHIFT put inside
the new function? The base address cannot ever be used without
PAGE_SHIFT.  In previous versions, information hiding was properly
used to hide the implementation details. Now, part of the implementation
detail is exposed to interface code.

Is this going to be removed in the future, forcing another change
to all drivers that use memory-mapping or is this now considered
the correct way to implement a kernel function?

If you are going to put PAGE_SHIFT inside the function, please
do it now so we don't have to modify all the drivers again.

Cheers,
Dick Johnson
Penguin : Linux version 2.6.10 on an i686 machine (5537.79 BogoMips).
  Notice : All mail here is now cached for review by Dictator Bush.
                  98.36% of all statistics are fiction.

Re: remap_pfm_range() Linux-2.6.10

[Andi Kleen]

linux-os <linux-os@chaos.analogic.com> writes:
>  	remap_pfn_range(vma, vma->vm_start, base_addr >> PAGE_SHIFT, len, prot)
>
> Now, here's the $US0.02 question. Why wasn't PAGE_SHIFT put inside
> the new function? The base address cannot ever be used without
> PAGE_SHIFT.  In previous versions, information hiding was properly

Because such a conversion would be very error prone.  People would
likely add subtle bugs. Changing units is always dangerous.

-Andi

Re: remap_pfm_range() Linux-2.6.10

[Jonathan Corbet]

> Why wasn't PAGE_SHIFT put inside
> the new function? The base address cannot ever be used without
> PAGE_SHIFT.  In previous versions, information hiding was properly
> used to hide the implementation details. Now, part of the implementation
> detail is exposed to interface code.

Hmm... seems to me that the new interface *removes* the page shift one
used to have to apply to the offset found in the VMA; looks like an
improvement to me.

Incidentally, the change allows the remapping of areas with physical
addresses beyond the 32-bit range, which, I believe, was why it was
done.  Meanwhile, there's a nice compatibility function, so nobody's
driver broke.  To me, there doesn't seem to be much to complain about.

jon