[PATCH] ACPI: move tables.c fully into .init.*

[PATCH] ACPI: move tables.c fully into .init.*

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 1416 bytes --]

The only non-init item was the space reserved for the initial tables,
but we can as well dynamically allocate that array.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/acpi/Makefile
+++ b/xen/drivers/acpi/Makefile
@@ -2,7 +2,7 @@ subdir-y += tables
 subdir-y += utilities
 subdir-$(x86) += apei
 
-obj-y += tables.o
+obj-bin-y += tables.init.o
 obj-y += numa.o
 obj-y += osl.o
 obj-y += pmstat.o
--- a/xen/drivers/acpi/tables.c
+++ b/xen/drivers/acpi/tables.c
@@ -25,6 +25,8 @@
 
 #include <xen/init.h>
 #include <xen/kernel.h>
+#include <xen/mm.h>
+#include <xen/pfn.h>
 #include <xen/smp.h>
 #include <xen/string.h>
 #include <xen/types.h>
@@ -41,8 +43,6 @@ mps_inti_flags_polarity[] = { "dfl", "hi
 static const char *__initdata
 mps_inti_flags_trigger[] = { "dfl", "edge", "res", "level" };
 
-static struct acpi_table_desc initial_tables[ACPI_MAX_TABLES];
-
 static int acpi_apic_instance __initdata;
 
 void __init acpi_table_print_madt_entry(struct acpi_subtable_header *header)
@@ -324,6 +324,11 @@ static void __init check_multiple_madt(v
 
 int __init acpi_table_init(void)
 {
+	struct acpi_table_desc *initial_tables =
+		mfn_to_virt(alloc_boot_pages(PFN_UP(ACPI_MAX_TABLES *
+						    sizeof(*initial_tables)),
+					     1));
+
 	acpi_initialize_tables(initial_tables, ACPI_MAX_TABLES, 0);
 	check_multiple_madt();
 	return 0;




[-- Attachment #2: ACPI-tables-init.patch --]
[-- Type: text/plain, Size: 1452 bytes --]

ACPI: move tables.c fully into .init.*

The only non-init item was the space reserved for the initial tables,
but we can as well dynamically allocate that array.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/acpi/Makefile
+++ b/xen/drivers/acpi/Makefile
@@ -2,7 +2,7 @@ subdir-y += tables
 subdir-y += utilities
 subdir-$(x86) += apei
 
-obj-y += tables.o
+obj-bin-y += tables.init.o
 obj-y += numa.o
 obj-y += osl.o
 obj-y += pmstat.o
--- a/xen/drivers/acpi/tables.c
+++ b/xen/drivers/acpi/tables.c
@@ -25,6 +25,8 @@
 
 #include <xen/init.h>
 #include <xen/kernel.h>
+#include <xen/mm.h>
+#include <xen/pfn.h>
 #include <xen/smp.h>
 #include <xen/string.h>
 #include <xen/types.h>
@@ -41,8 +43,6 @@ mps_inti_flags_polarity[] = { "dfl", "hi
 static const char *__initdata
 mps_inti_flags_trigger[] = { "dfl", "edge", "res", "level" };
 
-static struct acpi_table_desc initial_tables[ACPI_MAX_TABLES];
-
 static int acpi_apic_instance __initdata;
 
 void __init acpi_table_print_madt_entry(struct acpi_subtable_header *header)
@@ -324,6 +324,11 @@ static void __init check_multiple_madt(v
 
 int __init acpi_table_init(void)
 {
+	struct acpi_table_desc *initial_tables =
+		mfn_to_virt(alloc_boot_pages(PFN_UP(ACPI_MAX_TABLES *
+						    sizeof(*initial_tables)),
+					     1));
+
 	acpi_initialize_tables(initial_tables, ACPI_MAX_TABLES, 0);
 	check_multiple_madt();
 	return 0;

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] ACPI: move tables.c fully into .init.*

[Keir Fraser]

On 18/09/2012 16:26, "Jan Beulich" <JBeulich@suse.com> wrote:

> The only non-init item was the space reserved for the initial tables,
> but we can as well dynamically allocate that array.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Really worthwhile, versus having to round up the initial_tables[] allocation
to a whole number of pages?

> --- a/xen/drivers/acpi/Makefile
> +++ b/xen/drivers/acpi/Makefile
> @@ -2,7 +2,7 @@ subdir-y += tables
>  subdir-y += utilities
>  subdir-$(x86) += apei
>  
> -obj-y += tables.o
> +obj-bin-y += tables.init.o
>  obj-y += numa.o
>  obj-y += osl.o
>  obj-y += pmstat.o
> --- a/xen/drivers/acpi/tables.c
> +++ b/xen/drivers/acpi/tables.c
> @@ -25,6 +25,8 @@
>  
>  #include <xen/init.h>
>  #include <xen/kernel.h>
> +#include <xen/mm.h>
> +#include <xen/pfn.h>
>  #include <xen/smp.h>
>  #include <xen/string.h>
>  #include <xen/types.h>
> @@ -41,8 +43,6 @@ mps_inti_flags_polarity[] = { "dfl", "hi
>  static const char *__initdata
>  mps_inti_flags_trigger[] = { "dfl", "edge", "res", "level" };
>  
> -static struct acpi_table_desc initial_tables[ACPI_MAX_TABLES];
> -
>  static int acpi_apic_instance __initdata;
>  
>  void __init acpi_table_print_madt_entry(struct acpi_subtable_header *header)
> @@ -324,6 +324,11 @@ static void __init check_multiple_madt(v
>  
>  int __init acpi_table_init(void)
>  {
> + struct acpi_table_desc *initial_tables =
> +  mfn_to_virt(alloc_boot_pages(PFN_UP(ACPI_MAX_TABLES *
> +          sizeof(*initial_tables)),
> +          1));
> +
> acpi_initialize_tables(initial_tables, ACPI_MAX_TABLES, 0);
> check_multiple_madt();
> return 0;
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] ACPI: move tables.c fully into .init.*

[Jan Beulich]

>>> On 18.09.12 at 17:42, Keir Fraser <keir.xen@gmail.com> wrote:
> On 18/09/2012 16:26, "Jan Beulich" <JBeulich@suse.com> wrote:
> 
>> The only non-init item was the space reserved for the initial tables,
>> but we can as well dynamically allocate that array.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Really worthwhile, versus having to round up the initial_tables[] allocation
> to a whole number of pages?

It was exactly one page in size already.

Jan

Re: [PATCH] ACPI: move tables.c fully into .init.*

[Keir Fraser]

On 19/09/2012 08:29, "Jan Beulich" <JBeulich@suse.com> wrote:

>>>> On 18.09.12 at 17:42, Keir Fraser <keir.xen@gmail.com> wrote:
>> On 18/09/2012 16:26, "Jan Beulich" <JBeulich@suse.com> wrote:
>> 
>>> The only non-init item was the space reserved for the initial tables,
>>> but we can as well dynamically allocate that array.
>>> 
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> 
>> Really worthwhile, versus having to round up the initial_tables[] allocation
>> to a whole number of pages?
> 
> It was exactly one page in size already.

Well, still I'm not sure about the style of converting static data to heap
data, just to be able to mark a whole unit init-only. If everything else is
already  marked init inside that file already, is there a win?

 -- Keir

> Jan
> 

Re: [PATCH] ACPI: move tables.c fully into .init.*

[Jan Beulich]

>>> On 19.09.12 at 09:56, Keir Fraser <keir.xen@gmail.com> wrote:
> On 19/09/2012 08:29, "Jan Beulich" <JBeulich@suse.com> wrote:
> 
>>>>> On 18.09.12 at 17:42, Keir Fraser <keir.xen@gmail.com> wrote:
>>> On 18/09/2012 16:26, "Jan Beulich" <JBeulich@suse.com> wrote:
>>> 
>>>> The only non-init item was the space reserved for the initial tables,
>>>> but we can as well dynamically allocate that array.
>>>> 
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> 
>>> Really worthwhile, versus having to round up the initial_tables[] allocation
>>> to a whole number of pages?
>> 
>> It was exactly one page in size already.
> 
> Well, still I'm not sure about the style of converting static data to heap
> data, just to be able to mark a whole unit init-only. If everything else is
> already  marked init inside that file already, is there a win?

Yes - there a little of 1k of string literals, and there's no way
(other than making them variables to be able to apply section
attributes) to move them into .init.* - that's the purpose of the
other instances where the same post processing is being applied.

Of course, if you're only against the dynamic allocation, moving
the array elsewhere would be another option (but would require
making the symbol global, whereas here the symbol goes away
altogether from the symbol tables).

Yet another option would be to do the dynamic allocation where
it was actually intended to be done, passing NULL here. The
resizing there isn't being made use of anyway (and wouldn't
work afaict), so we could as well do away with it and replace it
by the simple allocation needed here (or simply fix it,
considering that we might need it at some point).

Jan

Re: [PATCH] ACPI: move tables.c fully into .init.*

[Keir Fraser]

On 19/09/2012 09:58, "Jan Beulich" <JBeulich@suse.com> wrote:

> Of course, if you're only against the dynamic allocation, moving
> the array elsewhere would be another option (but would require
> making the symbol global, whereas here the symbol goes away
> altogether from the symbol tables).
> 
> Yet another option would be to do the dynamic allocation where
> it was actually intended to be done, passing NULL here. The
> resizing there isn't being made use of anyway (and wouldn't
> work afaict), so we could as well do away with it and replace it
> by the simple allocation needed here (or simply fix it,
> considering that we might need it at some point).

I'm not wild about any of the options really. Perhaps passing NULL would be
best. Still, overall, I'm not *that* bothered. You can have my Ack for the
original patch:
Acked-by: Keir Fraser <keir@xen.org>

 -- Keir

Re: [PATCH] ACPI: move tables.c fully into .init.*

[Jan Beulich]

>>> On 19.09.12 at 13:40, Keir Fraser <keir.xen@gmail.com> wrote:
> On 19/09/2012 09:58, "Jan Beulich" <JBeulich@suse.com> wrote:
> 
>> Of course, if you're only against the dynamic allocation, moving
>> the array elsewhere would be another option (but would require
>> making the symbol global, whereas here the symbol goes away
>> altogether from the symbol tables).
>> 
>> Yet another option would be to do the dynamic allocation where
>> it was actually intended to be done, passing NULL here. The
>> resizing there isn't being made use of anyway (and wouldn't
>> work afaict), so we could as well do away with it and replace it
>> by the simple allocation needed here (or simply fix it,
>> considering that we might need it at some point).
> 
> I'm not wild about any of the options really. Perhaps passing NULL would be
> best. Still, overall, I'm not *that* bothered. You can have my Ack for the
> original patch:
> Acked-by: Keir Fraser <keir@xen.org>

As you were not really happy with it, and as I (also already in
the past) wondered about when the broken re-allocation there
would hit us, I decided to produce a v2, fixing and using the
re-allocation mechanism instead.

Jan

Re: [PATCH] ACPI: move tables.c fully into .init.*

[Keir Fraser]

On 19/09/2012 13:57, "Jan Beulich" <JBeulich@suse.com> wrote:

>>>> On 19.09.12 at 13:40, Keir Fraser <keir.xen@gmail.com> wrote:
>> On 19/09/2012 09:58, "Jan Beulich" <JBeulich@suse.com> wrote:
>> 
>>> Of course, if you're only against the dynamic allocation, moving
>>> the array elsewhere would be another option (but would require
>>> making the symbol global, whereas here the symbol goes away
>>> altogether from the symbol tables).
>>> 
>>> Yet another option would be to do the dynamic allocation where
>>> it was actually intended to be done, passing NULL here. The
>>> resizing there isn't being made use of anyway (and wouldn't
>>> work afaict), so we could as well do away with it and replace it
>>> by the simple allocation needed here (or simply fix it,
>>> considering that we might need it at some point).
>> 
>> I'm not wild about any of the options really. Perhaps passing NULL would be
>> best. Still, overall, I'm not *that* bothered. You can have my Ack for the
>> original patch:
>> Acked-by: Keir Fraser <keir@xen.org>
> 
> As you were not really happy with it, and as I (also already in
> the past) wondered about when the broken re-allocation there
> would hit us, I decided to produce a v2, fixing and using the
> re-allocation mechanism instead.

I like that more, yes. Thanks!

> Jan
>