From: Myron Stowe <[email protected]>
Make sure the removal of mappings uses the same logic that put the
mappings in place.
Signed-off-by: Myron Stowe <[email protected]>
---
drivers/acpi/osl.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 412a1e0..5aef087 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -347,7 +347,7 @@ static void acpi_unmap(acpi_physical_address pg_off, void __iomem *vaddr)
unsigned long pfn;
pfn = pg_off >> PAGE_SHIFT;
- if (page_is_ram(pfn))
+ if (should_use_kmap(pfn))
kunmap(pfn_to_page(pfn));
else
iounmap(vaddr);
On Thu, Feb 9, 2012 at 8:36 AM, Myron Stowe <[email protected]> wrote:
> From: Myron Stowe <[email protected]>
>
> Make sure the removal of mappings uses the same logic that put the
> mappings in place.
>
> Signed-off-by: Myron Stowe <[email protected]>
> ---
>
> ?drivers/acpi/osl.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 412a1e0..5aef087 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -347,7 +347,7 @@ static void acpi_unmap(acpi_physical_address pg_off, void __iomem *vaddr)
> ? ? ? ?unsigned long pfn;
>
> ? ? ? ?pfn = pg_off >> PAGE_SHIFT;
> - ? ? ? if (page_is_ram(pfn))
> + ? ? ? if (should_use_kmap(pfn))
> ? ? ? ? ? ? ? ?kunmap(pfn_to_page(pfn));
> ? ? ? ?else
> ? ? ? ? ? ? ? ?iounmap(vaddr);
>
Whatever happened to the question of why we have arch-specific
ioremap() behavior? It's good to make map/unmap symmetric, but it'd
be better to get rid of the ioremap/kmap hack.
Bjorn
On Thu, 2012-02-09 at 08:42 -0800, Bjorn Helgaas wrote:
> On Thu, Feb 9, 2012 at 8:36 AM, Myron Stowe <[email protected]> wrote:
> > From: Myron Stowe <[email protected]>
> >
> > Make sure the removal of mappings uses the same logic that put the
> > mappings in place.
> >
> > Signed-off-by: Myron Stowe <[email protected]>
> > ---
> >
> > drivers/acpi/osl.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> > index 412a1e0..5aef087 100644
> > --- a/drivers/acpi/osl.c
> > +++ b/drivers/acpi/osl.c
> > @@ -347,7 +347,7 @@ static void acpi_unmap(acpi_physical_address pg_off, void __iomem *vaddr)
> > unsigned long pfn;
> >
> > pfn = pg_off >> PAGE_SHIFT;
> > - if (page_is_ram(pfn))
> > + if (should_use_kmap(pfn))
> > kunmap(pfn_to_page(pfn));
> > else
> > iounmap(vaddr);
> >
>
> Whatever happened to the question of why we have arch-specific
> ioremap() behavior? It's good to make map/unmap symmetric, but it'd
> be better to get rid of the ioremap/kmap hack.
+cc ingo
We never received any explanation for why ioremap() failed for Ying with
RAM on x86. Last I saw Ying asked Ingo for some input here but there
was never any reply -
http://marc.info/?l=linux-acpi&m=132788392604738&w=2
I like your idea of possibly changing ioremap's implementation so that
it would handle requests related to RAM - by using kmap() internally
when necessary - so that a *user* wouldn't need to care what
architecture we're on. I, however, feel like I don't have enough
experience with the memory management subsystem to know if such a tactic
would fly or not so was uncomfortable proceeding along those lines. As
a result, I just wanted to get this in for the meantime.
Myron
>
> Bjorn
On Thu, Feb 9, 2012 at 4:08 PM, Myron Stowe <[email protected]> wrote:
> On Thu, 2012-02-09 at 08:42 -0800, Bjorn Helgaas wrote:
>> On Thu, Feb 9, 2012 at 8:36 AM, Myron Stowe <[email protected]> wrote:
>> > From: Myron Stowe <[email protected]>
>> >
>> > Make sure the removal of mappings uses the same logic that put the
>> > mappings in place.
>> >
>> > Signed-off-by: Myron Stowe <[email protected]>
>> > ---
>> >
>> > ?drivers/acpi/osl.c | ? ?2 +-
>> > ?1 files changed, 1 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
>> > index 412a1e0..5aef087 100644
>> > --- a/drivers/acpi/osl.c
>> > +++ b/drivers/acpi/osl.c
>> > @@ -347,7 +347,7 @@ static void acpi_unmap(acpi_physical_address pg_off, void __iomem *vaddr)
>> > ? ? ? ?unsigned long pfn;
>> >
>> > ? ? ? ?pfn = pg_off >> PAGE_SHIFT;
>> > - ? ? ? if (page_is_ram(pfn))
>> > + ? ? ? if (should_use_kmap(pfn))
>> > ? ? ? ? ? ? ? ?kunmap(pfn_to_page(pfn));
>> > ? ? ? ?else
>> > ? ? ? ? ? ? ? ?iounmap(vaddr);
>> >
>>
>> Whatever happened to the question of why we have arch-specific
>> ioremap() behavior? ?It's good to make map/unmap symmetric, but it'd
>> be better to get rid of the ioremap/kmap hack.
>
> +cc ingo
>
> We never received any explanation for why ioremap() failed for Ying with
> RAM on x86. ?Last I saw Ying asked Ingo for some input here but there
> was never any reply -
> http://marc.info/?l=linux-acpi&m=132788392604738&w=2
>
> I like your idea of possibly changing ioremap's implementation so that
> it would handle requests related to RAM - by using kmap() internally
> when necessary - so that a *user* wouldn't need to care what
> architecture we're on. ?I, however, feel like I don't have enough
> experience with the memory management subsystem to know if such a tactic
> would fly or not so was uncomfortable proceeding along those lines. ?As
> a result, I just wanted to get this in for the meantime.
I dunno if you're comfortable with it, but you can always propose a
patch doing it how you think it should be done. People are more apt
to respond to a concrete patch than to an abstract question about why
things are the way they are.
Bjorn