Re: [PATCH 0/6] Introduce acpi_table_parse_cedt and extra nodes for CXL.mem

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 2, 2021 at 6:44 PM Jonathan Cameron
<Jonathan.Cameron@xxxxxxxxxx> wrote:
>
> On Mon, 1 Nov 2021 20:41:34 -0700
> Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
>
> > On Mon, Nov 1, 2021 at 5:01 AM Jonathan Cameron
> > <Jonathan.Cameron@xxxxxxxxxx> wrote:
> > >
> > > On Fri, 29 Oct 2021 12:51:27 -0700
> > > Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
> > >
> > > > Hi Rafael,
> > > >
> > > > While reviewing "[PATCH v3] ACPI: NUMA: Add a node and memblk for each
> > > > CFMWS not in SRAT" [1]. I noticed that it was open coding CEDT sub-table
> > > > parsing in a similar fashion as drivers/cxl/acpi.c. The driver open
> > > > coded the parsing because the ACPI sub-table helpers are marked __init.
> > > > In order to avoid the ongoing maintenance burden of a split between
> > > > "early" and "late" ACPI sub-table parsing this series proposes to make
> > > > those helpers available to drivers.
> > > >
> > > > The savings in drivers/cxl/ are:
> > > >
> > > >  drivers/cxl/Kconfig |    1
> > > >  drivers/cxl/acpi.c  |  234 +++++++++++++++++++--------------------------------
> > > >  2 files changed, 88 insertions(+), 147 deletions(-)
> > > >
> > > > ...and 15 lines new code not added are saved in this new version of
> > > > "ACPI: NUMA: Add a node and memblk for each CFMWS not in SRAT".
> > > >
> > > > Let me know if this looks ok to you and I can carry it in the CXL tree
> > > > (i.e. after the merge window, for v5.17 consideration).
> > > >
> > > > [1]: https://lore.kernel.org/r/20211019050908.449231-1-alison.schofield@xxxxxxxxx
> > >
> > > Is it worth the complexity of the __init_or_acpilib and export part?
> > > Seems like a fiddly dance for what looks to be minor savings...
> >
> > It follows the __initdata_or_meminfo precedent that identifies data
> > that is normally __init unless a specific driver needs it. The lesson
> > from the tinyconfig effort was that image size dies a death of many
> > cuts unless care is taken to preserve minor savings. Yes, it's likely
> > trivial in this case, but it's at least a gesture to avoid bloating
> > the kernel image size unnecessarily when the kernel has gotten by so
> > long with this infrastructure being purely __init.
>
> I'm in favor avoiding bloat, but this is ACPI code so rarely very small machines
> and very like that all distros will turn it on anyway on basis they will want
> to support CXL (hopefully!)
>
> I guess let's see what Rafael's opinion is.  I don't feel that strongly about
> it if general view is that it is worth the small amount of complexity.

The general ACPI changes in this series are fine with me, so Dan
please feel free to add

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

to it.

Thanks!



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux