Message-ID: <1738949fd49e9804722bf82d790e3022fc714677.camel@intel.com>
Date:   Thu, 16 Jul 2020 19:22:30 +0000
From:   "Verma, Vishal L" <vishal.l.verma@...el.com>
To:     "Williams, Dan J" <dan.j.williams@...el.com>,
        "Kaneda, Erik" <erik.kaneda@...el.com>,
        "rjw@...ysocki.net" <rjw@...ysocki.net>
CC:     "linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
        "james.morse@....com" <james.morse@....com>,
        "lenb@...nel.org" <lenb@...nel.org>,
        "andriy.shevchenko@...ux.intel.com" 
        <andriy.shevchenko@...ux.intel.com>, "bp@...en8.de" <bp@...en8.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "myron.stowe@...hat.com" <myron.stowe@...hat.com>,
        "Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
        "Weiny, Ira" <ira.weiny@...el.com>,
        "Moore, Robert" <robert.moore@...el.com>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>
Subject: Re: [PATCH v4 2/2] ACPICA: Preserve memory opregion mappings

On Mon, 2020-06-29 at 18:33 +0200, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>
> 
> The ACPICA's strategy with respect to the handling of memory mappings
> associated with memory operation regions is to avoid mapping the
> entire region at once which may be problematic at least in principle
> (for example, it may lead to conflicts with overlapping mappings
> having different attributes created by drivers).  It may also be
> wasteful, because memory opregions on some systems take up vast
> chunks of address space while the fields in those regions actually
> accessed by AML are sparsely distributed.
> 
> For this reason, a one-page "window" is mapped for a given opregion
> on the first memory access through it and if that "window" does not
> cover an address range accessed through that opregion subsequently,
> it is unmapped and a new "window" is mapped to replace it.  Next,
> if the new "window" is not sufficient to acess memory through the
> opregion in question in the future, it will be replaced with yet
> another "window" and so on.  That may lead to a suboptimal sequence
> of memory mapping and unmapping operations, for example if two fields
> in one opregion separated from each other by a sufficiently wide
> chunk of unused address space are accessed in an alternating pattern.
> 
> The situation may still be suboptimal if the deferred unmapping
> introduced previously is supported by the OS layer.  For instance,
> the alternating memory access pattern mentioned above may produce
> a relatively long list of mappings to release with substantial
> duplication among the entries in it, which could be avoided if
> acpi_ex_system_memory_space_handler() did not release the mapping
> used by it previously as soon as the current access was not covered
> by it.
> 
> In order to improve that, modify acpi_ex_system_memory_space_handler()
> to preserve all of the memory mappings created by it until the memory
> regions associated with them go away.
> 
> Accordingly, update acpi_ev_system_memory_region_setup() to unmap all
> memory associated with memory opregions that go away.
> 
> Reported-by: Dan Williams <dan.j.williams@...el.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
>  drivers/acpi/acpica/evrgnini.c | 14 ++++----
>  drivers/acpi/acpica/exregion.c | 65 ++++++++++++++++++++++++----------
>  include/acpi/actypes.h         | 12 +++++--
>  3 files changed, 64 insertions(+), 27 deletions(-)
> 

Hi Rafael,

Picking up from Dan while he's out - I had these patches tested by the
original reporter, and they work fine. I see you had them staged in the
acpica-osl branch. Is that slated to go in during the 5.9 merge window?

You can add:
Tested-by: Xiang Li <xiang.z.li@...el.com>

Powered by blists - more mailing lists