[lkml]   [2008]   [Sep]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 6 of 7] x86: use early_ioremap in __acpi_map_table
Ingo Molnar wrote:
> 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).

You mean there's code which just assumes that it can keep using a
linear-mapped acpi even after __acpi_map_table() should have implicitly
unmapped it?
> 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.

__acpi_map_table() is called by acpi_map_table(), which does have a
acpi_unmap_table() counterpart. But it only calls iounmap() once we're
past the stage of calling early_*(). I could easily make it call
__acpi_unmap_table()->early_iounmap(). But if the concern is that the
early boot callers of acpi_map_table() "know" that they never need to
unmap, then yes, I see the problem.

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

And the current behaviour of __acpi_map_table() is to remove the
previous mapping (implicitly, by overwriting the same fixmap slots), so
its only an issue if the callers assume they can keep using
linear-mapped acpi tables after a subsequent call to __acpi_map_table().


 \ /
  Last update: 2008-09-08 18:31    [W:0.073 / U:8.832 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site