* Mike Travis <travis@xxxxxxx> wrote:
> Ingo Molnar wrote:
> >* Mike Travis <travis@xxxxxxx> wrote:
> >
> >>Condense the SRAT: messages to show all the APIC id's on one line for
> >>each Node. This not only saves space in the log buf, it also makes
> >>it easier to spot inconsistencies in core to node placement.
> >>
> >>On a system with 2368 cores on 248 nodes the change will be...
> >>
> >>Was 2368 lines (for 2368 cores):
> >>
> >> 779 [0] SRAT: PXM 0 -> APIC 0x0000 -> Node 0
> >> 780 [0] SRAT: PXM 0 -> APIC 0x0002 -> Node 0
> >> 781 [0] SRAT: PXM 0 -> APIC 0x0004 -> Node 0
> >> ...
> >> 3145 [0] SRAT: PXM 247 -> APIC 0x3df0 -> Node 247
> >> 3146 [0] SRAT: PXM 247 -> APIC 0x3df2 -> Node 247
> >>
> >>Now it's 248 lines (for 248 Nodes):
> >>
> >> 821 [0] SRAT: Node 0: PXM:APIC 0:0x0 :0x2 :0x4 :0x10 :0x12 ...
> >> 822 [0] SRAT: Node 1: PXM:APIC 1:0x40 :0x42 :0x44 :0x50 :0x52 ...
> >> 823 [0] SRAT: Node 2: PXM:APIC 2:0x80 :0x82 :0x84 :0x90 :0x92 ...
> >> ...
> >> 1067 [0] SRAT: Node 246: PXM:APIC 246:0x3d80 :0x3d82 :0x3d84 :0x3d90 ...
> >> 1068 [0] SRAT: Node 247: PXM:APIC 247:0x3dc0 :0x3dc2 :0x3dc4 :0x3dd2 ...
> >>
> >>Signed-off-by: Mike Travis <travis@xxxxxxx>
> >>Reviewed-by: Jack Steiner <steiner@xxxxxxx>
> >>Reviewed-by: Robin Holt <holt@xxxxxxx>
> >>---
> >> arch/x86/mm/srat_64.c | 19 +++++++++++++++++--
> >> drivers/acpi/numa.c | 7 +++++++
> >> 2 files changed, 24 insertions(+), 2 deletions(-)
> >>
> >>--- linux.orig/arch/x86/mm/srat_64.c
> >>+++ linux/arch/x86/mm/srat_64.c
> >>@@ -110,6 +110,12 @@ void __init acpi_numa_slit_init(struct a
> >> memblock_x86_reserve_range(phys, phys + length, "ACPI SLIT");
> >> }
> >>+/*
> >>+ * Keep track of previous node and PXM values so we can combine
> >>+ * same ones onto a single line.
> >>+ */
> >>+static int __initdata last_node = NUMA_NO_NODE, last_pxm = PXM_INVAL;
> >>+
> >> /* Callback for Proximity Domain -> x2APIC mapping */
> >> void __init
> >> acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
> >>@@ -141,8 +147,17 @@ acpi_numa_x2apic_affinity_init(struct ac
> >> set_apicid_to_node(apic_id, node);
> >> node_set(node, cpu_nodes_parsed);
> >> acpi_numa = 1;
> >>- printk(KERN_INFO "SRAT: PXM %u -> APIC 0x%04x -> Node %u\n",
> >>- pxm, apic_id, node);
> >>+ if (node != last_node) {
> >>+ pr_info("SRAT: Node %u: PXM:APIC %u:0x%x",
> >>+ node, pxm, apic_id);
> >>+ last_node = node;
> >>+ last_pxm = pxm;
> >>+ } else if (pxm != last_pxm) {
> >>+ pr_cont(" %u:0x%x", pxm, apic_id);
> >>+ last_pxm = pxm;
> >>+ } else {
> >>+ pr_cont(" :0x%x", apic_id);
> >>+ }
> >> }
> >> /* Callback for Proximity Domain -> LAPIC mapping */
> >>--- linux.orig/drivers/acpi/numa.c
> >>+++ linux/drivers/acpi/numa.c
> >>@@ -286,6 +286,13 @@ int __init acpi_numa_init(void)
> >> if (!acpi_table_parse(ACPI_SIG_SRAT, acpi_parse_srat)) {
> >> acpi_table_parse_srat(ACPI_SRAT_TYPE_X2APIC_CPU_AFFINITY,
> >> acpi_parse_x2apic_affinity, 0);
> >>+ /*
> >>+ * Parsing ACPI_SRAT_TYPE_X2APIC_CPU_AFFINITY entries place
> >>+ * multiple CPU's on the same Node line. This can leave the
> >>+ * last entry "dangling" without a newline. Insert it here.
> >>+ */
> >>+ pr_cont("\n");
> >
> >This is quite ugly as it breaks the genericity of the ACPI parsing
> >here. Is there no cleaner method that keeps this deinit \n
> >printing somehow within the realm of x86?
> >
> >Also, can there be cases where there's no 'dangling' line pending?
> >In that case the \n will be superfluous here.
> >
> >Thanks,
> >
> > Ingo
>
> Yes, David brought up the same point a couple of weeks ago. I've tried and
> failed to find a solution, except that the printk function seems to add the
> newline if there is not one. I asked if this was sufficient to rely on,
> and no one spoke up. (Everyone is quick to object, but seemingly very slow
> to agree.)
>
> And yes, there will always be a dangling line. If the ACPI guys could tell
> me how to predict when this is the last entry, I would gladly change it.
Your problem is that the current way of ACPI parsing does not lend itself well to
your stateful approach to printing compressed info.
Last i checked the C language was still Turing-complete so there *ought* to be some
solution.
My problem is that you are asking me to commit a change to a piece of code i do not
maintain. I do that reluctantly and i absolutely cannot do it when a patch has
easily visible negative side-effects on code quality. So either get Len's Acked-by
to add a small amount of crap to drivers/acpi/ (or better yet, get him to commit
it), or code up a clean solution ...
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
