2005-01-24 17:17:07

by Mark_H_Johnson

[permalink] [raw]
Subject: Query on remap_pfn_range compatibility

I read the messages on lkml from September 2004 about the introduction of
remap_pfn_range and have a question related to coding for it. What do you
recommend for driver coding to be compatible with these functions
(remap_page_range, remap_pfn_range)?

For example, I see at least two (or three) combination I need to address:
- 2.4 (with remap_page_range) OR 2.6.x (with remap_page_range)
- 2.6.x-mm (with remap_pfn_range)
Is there some symbol or #ifdef value I can depend on to determine which
function I should be calling (and the value to pass in)?

Thanks.
--Mark H Johnson
<mailto:[email protected]>


2005-01-24 17:50:08

by William Lee Irwin III

[permalink] [raw]
Subject: Re: Query on remap_pfn_range compatibility

On Mon, Jan 24, 2005 at 10:54:22AM -0600, [email protected] wrote:
> I read the messages on lkml from September 2004 about the introduction of
> remap_pfn_range and have a question related to coding for it. What do you
> recommend for driver coding to be compatible with these functions
> (remap_page_range, remap_pfn_range)?
> For example, I see at least two (or three) combination I need to address:
> - 2.4 (with remap_page_range) OR 2.6.x (with remap_page_range)
> - 2.6.x-mm (with remap_pfn_range)
> Is there some symbol or #ifdef value I can depend on to determine which
> function I should be calling (and the value to pass in)?

Not sure. One on kernel version being <= 2.6.10 would probably serve
your purposes, though it's not particularly well thought of. I suspect
people would suggest splitting up the codebase instead of sharing it
between 2.4.x and 2.6.x, where I've no idea how well that sits with you.

I vaguely suspected something like this would happen, but there were
serious and legitimate concerns about new usage of the 32-bit unsafe
methods being reintroduced, so at some point the old hook had to go.


-- wli

2005-01-24 19:35:59

by Mark_H_Johnson

[permalink] [raw]
Subject: Re: Query on remap_pfn_range compatibility

wli wrote...
> On Mon, Jan 24, 2005 at 10:54:22AM -0600, [email protected]
wrote:
> > I read the messages on lkml from September 2004 about the introduction
of
> > remap_pfn_range and have a question related to coding for it. What do
you
> > recommend for driver coding to be compatible with these functions
> > (remap_page_range, remap_pfn_range)?
> > For example, I see at least two (or three) combination I need to
address:
> > - 2.4 (with remap_page_range) OR 2.6.x (with remap_page_range)
> > - 2.6.x-mm (with remap_pfn_range)
> > Is there some symbol or #ifdef value I can depend on to determine which
> > function I should be calling (and the value to pass in)?
>
> Not sure. One on kernel version being <= 2.6.10 would probably serve
> your purposes, though it's not particularly well thought of. I suspect
> people would suggest splitting up the codebase instead of sharing it
> between 2.4.x and 2.6.x, where I've no idea how well that sits with you.

I guess I could do that, but if a distribution picks up remap_pfn_range
in an earlier kernel, that doesn't work either. If it gets back ported
to 2.4 the conditional gets a little more complicated.

Splitting the code base is a pretty harsh solution.

I am also trying to avoid an ugly hack like the following:

VMA_PARAM_IN_REMAP=`grep remap_page_range
$PATH_LINUX_INCLUDE/linux/mm.h|grep vma`
if [ -z "$VMA_PARAM_IN_REMAP" ]; then
export REMAP_PAGE_RANGE_PARAM="4"
else
export REMAP_PAGE_RANGE_PARAM="5"
endif

in a build script which detects if remap_page_range() has 4 or 5 parameters
and then pass an appropriate value into the code using gcc -D. [ugh]

Would it be acceptable to add a symbol like
#define MM_VM_REMAP_PFN_RANGE
in include/linux/mm.h or is that too much of a hack as well?

> I vaguely suspected something like this would happen, but there were
> serious and legitimate concerns about new usage of the 32-bit unsafe
> methods being reintroduced, so at some point the old hook had to go.
I don't doubt the need to remove the old interface. But I see possible
problem areas on > 4 Gbyte machines, such as virt_to_phys defined in
linux/asm-i386/io.h, that are not getting fixed or do I misread the
way that code works.

--Mark H Johnson
<mailto:[email protected]>

2005-01-24 22:32:01

by William Lee Irwin III

[permalink] [raw]
Subject: Re: Query on remap_pfn_range compatibility

wli wrote...
>> Not sure. One on kernel version being <= 2.6.10 would probably serve
>> your purposes, though it's not particularly well thought of. I suspect
>> people would suggest splitting up the codebase instead of sharing it
>> between 2.4.x and 2.6.x, where I've no idea how well that sits with you.

On Mon, Jan 24, 2005 at 01:05:44PM -0600, [email protected] wrote:
> I guess I could do that, but if a distribution picks up remap_pfn_range
> in an earlier kernel, that doesn't work either. If it gets back ported
> to 2.4 the conditional gets a little more complicated.
> Splitting the code base is a pretty harsh solution.

I suspect it's the one most often recommended. In general, I'm not the
arbiter of taste in drivers (and as you've worked with them enough, I'm
sure you have unanswered questions of your own on that front), but I'm
expecting the general consensus to be something adverse to your concerns.


On Mon, Jan 24, 2005 at 01:05:44PM -0600, [email protected] wrote:
> I am also trying to avoid an ugly hack like the following:
> VMA_PARAM_IN_REMAP=`grep remap_page_range
> $PATH_LINUX_INCLUDE/linux/mm.h|grep vma`
> if [ -z "$VMA_PARAM_IN_REMAP" ]; then
> export REMAP_PAGE_RANGE_PARAM="4"
> else
> export REMAP_PAGE_RANGE_PARAM="5"
> endif
> in a build script which detects if remap_page_range() has 4 or 5 parameters
> and then pass an appropriate value into the code using gcc -D. [ugh]

Some codebases have literally gone so far as to use autoconf to cope
with constellations of issues like these that arise in portable driver
codebases. I don't have an adequate answer to the simultaneous needs of
mainline acceptance and portability across kernel versions. The second
of those is one I'm very rarely faced with myself and my inexperience
in such is accompanied by a lack of ideas.


On Mon, Jan 24, 2005 at 01:05:44PM -0600, [email protected] wrote:
> Would it be acceptable to add a symbol like
> #define MM_VM_REMAP_PFN_RANGE
> in include/linux/mm.h or is that too much of a hack as well?

I highly suspect that this notion would not be seriously entertained.


wli wrote...
>> I vaguely suspected something like this would happen, but there were
>> serious and legitimate concerns about new usage of the 32-bit unsafe
>> methods being reintroduced, so at some point the old hook had to go.

On Mon, Jan 24, 2005 at 01:05:44PM -0600, [email protected] wrote:
> I don't doubt the need to remove the old interface. But I see possible
> problem areas on > 4 Gbyte machines, such as virt_to_phys defined in
> linux/asm-i386/io.h, that are not getting fixed or do I misread the
> way that code works.

virt_to_phys() represents something of a trap for unwary programmers,
but not a true semantic gap as remap_pfn_range() addressed. It's also
not of any particular help, as the areas for which it fails are
universally not in ZONE_NORMAL. Primitives for resolving vmallocspace
and userspace addresses to pfn's may be in order if these are
sufficiently used, but the convenience of them can be done without at
the cost of some memory to account physical locations mapped without
the benefit of a struct page to track them, which when present is well
backed by primitives like follow_page(), vmalloc_to_page(), etc.


-- wli

2005-01-25 18:15:23

by Timur Tabi

[permalink] [raw]
Subject: Re: Query on remap_pfn_range compatibility

[email protected] wrote:


> I am also trying to avoid an ugly hack like the following:
>
> VMA_PARAM_IN_REMAP=`grep remap_page_range
> $PATH_LINUX_INCLUDE/linux/mm.h|grep vma`
> if [ -z "$VMA_PARAM_IN_REMAP" ]; then
> export REMAP_PAGE_RANGE_PARAM="4"
> else
> export REMAP_PAGE_RANGE_PARAM="5"
> endif

My makefile has a ton of stuff like this. Our driver needs to work with
all 2.4 and 2.6 kernels, and it makes heavy use of the VM. It also
needs to deal with distros that have "broken" header files.

This wouldn't be such a problem if the kernel developers would add
macros to indicate the version of the function parameters. Basically,
the header file should define REMAP_PAGE_RANGE_PARAM (or some
equivalent), so that you don't need to calculate it in your makefile.
But the kernel developers don't care about backwards compatibility, so
we're stuck with these ugly hacks.

> Would it be acceptable to add a symbol like
> #define MM_VM_REMAP_PFN_RANGE
> in include/linux/mm.h or is that too much of a hack as well?

The easiest solution would be to update gcc to provide some kind of
internal macro that would tell me if a function is defined or not. For
instance, I could do this:

#if defined(remap_pfn_range)
remap_pfn_range(...)
#else
remap_page_range(...)
#endif

This doesn't work because remap_pfn_range is a function, not a macro.
Then we could do other things like:

#if parameters(remap_page_range) = 4
remap_page_range(a, b, c, d)
#else
remap_page_range(a, b, c, d, e)
#endif

This would allow me to handle kernel versions that have 4 instead of 5
parameters for remap_page_range().

Ironically, even if gcc were updated like this, it wouldn't help me a
whole lot, because I still need to use the older versions of gcc on the
older distros. But at least it the problem wouldn't be getting worse,
like it is today.

--
Timur Tabi
Staff Software Engineer
[email protected]