Re: [PATCH 6 of 7] x86: use early_ioremap in __acpi_map_table

Ingo Molnar
2008-09-08T14:26:42Z

* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> >> However, unlike early_ioremap(), __acpi_map_table() just maintains
> >> a single mapping which gets replaced each call, and has no
> >> corresponding unmap function. Implement this by just removing the
> >> previous mapping each time its called. Unfortunately, this will
> >> leave a stray mapping at the end.
> >
> > It would be better to just fix the ACPI code to unmap.
>
> I was concerned that would cause lots of cross-arch churn, but of
> course the only other relevant architecture is ia64. I'll prep a
> followup patch.

uhm, there's a nasty trap in that route: it can potentially cause a lot
of breakage.

It's not robust to assume that the ACPI code is sane wrt.
mapping/unmapping, because it currently simply doesnt rely on robust
unmapping (in the linear range).

I tried it in the past and i found tons of crappy ACPI code all around
that just never unmapped tables. Leaking ACPI maps are hard to find as
well, and it can occur anytime during bootup.

As a general principle it might be worth fixing those places, and we've
hardened up the early-ioremap code for leaks during the PAT rewrite,
still please realize that it can become non-trivial and it might cause a
lot of unhappy users.

So i'd suggest a different, more carful approach: keep the new code you
wrote, but print a WARN()ing if prev_map is not unmapped yet when the
next mapping is acquired. That way the ACPI code can be fixed gradually
and without breaking existing functionality.

There's another complication: ACPI might rely on multiple mappings being
present at once, so unmapping the previous one might not be safe. But it
_should_ be fine most of the time as __acpi_map_table() is only used
inearly init code - and we fixed most of these things in the PAT
patchset in any case.

Ingo