2009-09-27 07:02:24

by Yinghai Lu

[permalink] [raw]
Subject: [PATCH] x86: add show_lapic=


so could use that together with apci=debug to print less cpu apic.

otherwise for system have more cpus will take a while

also move apic_verbosity check to print_all_ICs

Signed-off-by: Yinghai Lu <[email protected]>

---
arch/x86/kernel/apic/io_apic.c | 42 ++++++++++++++++++++++++++---------------
1 file changed, 27 insertions(+), 15 deletions(-)

Index: linux-2.6/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6/arch/x86/kernel/apic/io_apic.c
@@ -1599,9 +1599,6 @@ __apicdebuginit(void) print_IO_APIC(void
struct irq_desc *desc;
unsigned int irq;

- if (apic_verbosity == APIC_QUIET)
- return;
-
printk(KERN_DEBUG "number of MP IRQ sources: %d.\n", mp_irq_entries);
for (i = 0; i < nr_ioapics; i++)
printk(KERN_DEBUG "number of IO-APIC #%d registers: %d.\n",
@@ -1708,9 +1705,6 @@ __apicdebuginit(void) print_APIC_field(i
{
int i;

- if (apic_verbosity == APIC_QUIET)
- return;
-
printk(KERN_DEBUG);

for (i = 0; i < 8; i++)
@@ -1724,9 +1718,6 @@ __apicdebuginit(void) print_local_APIC(v
unsigned int i, v, ver, maxlvt;
u64 icr;

- if (apic_verbosity == APIC_QUIET)
- return;
-
printk(KERN_DEBUG "printing local APIC contents on CPU#%d/%d:\n",
smp_processor_id(), hard_smp_processor_id());
v = apic_read(APIC_ID);
@@ -1824,13 +1815,17 @@ __apicdebuginit(void) print_local_APIC(v
printk("\n");
}

-__apicdebuginit(void) print_all_local_APICs(void)
+__apicdebuginit(void) print_local_APICs(int num)
{
int cpu;

preempt_disable();
- for_each_online_cpu(cpu)
+ for_each_online_cpu(cpu) {
+ if (cpu >= num)
+ break;
+
smp_call_function_single(cpu, print_local_APIC, NULL, 1);
+ }
preempt_enable();
}

@@ -1839,7 +1834,7 @@ __apicdebuginit(void) print_PIC(void)
unsigned int v;
unsigned long flags;

- if (apic_verbosity == APIC_QUIET || !nr_legacy_irqs)
+ if (!nr_legacy_irqs)
return;

printk(KERN_DEBUG "\nprinting PIC contents\n");
@@ -1866,21 +1861,38 @@ __apicdebuginit(void) print_PIC(void)
printk(KERN_DEBUG "... PIC ELCR: %04x\n", v);
}

-__apicdebuginit(int) print_all_ICs(void)
+static int __initdata show_lapic = 1;
+static __init int setup_show_lapic(char *arg)
+{
+ int num;
+
+ get_option(&arg, &num);
+
+ if (num >= 0)
+ show_lapic = num;
+
+ return 1;
+}
+__setup("show_lapic=", setup_show_lapic);
+
+__apicdebuginit(int) print_ICs(void)
{
+ if (apic_verbosity == APIC_QUIET)
+ return 0;
+
print_PIC();

/* don't print out if apic is not there */
if (!cpu_has_apic && !apic_from_smp_config())
return 0;

- print_all_local_APICs();
+ print_local_APICs(show_lapic);
print_IO_APIC();

return 0;
}

-fs_initcall(print_all_ICs);
+fs_initcall(print_ICs);


/* Where if anywhere is the i8259 connect in external int mode */


2009-09-27 08:06:10

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH] x86: add show_lapic=

[Yinghai Lu - Sun, Sep 27, 2009 at 12:01:52AM -0700]
|
| so could use that together with apci=debug to print less cpu apic.
|
| otherwise for system have more cpus will take a while
|
| also move apic_verbosity check to print_all_ICs
|
| Signed-off-by: Yinghai Lu <[email protected]>
|
| ---
| arch/x86/kernel/apic/io_apic.c | 42 ++++++++++++++++++++++++++---------------
| 1 file changed, 27 insertions(+), 15 deletions(-)
...

Hi Yinghai, good idea!

I've tuned your patch a bit together with changelog.

1) show_lapic=1 by default does change current kernel behaviour
so it should be set to max cpu available.
2) show_lapic contains the max number of lapics being printed
so we need to count them instead of plain comparation with
cpu number, as a result I've changed print_local_APICs. Or
you meant _exactly_ cpuid number? In meaning like
show_lapic=max_cpuid. Hmm?

Someting like this perhaps?

(I keep you SOB since I've touched a few lines only)

How do you think?
---
[PATCH -tip] x86,apic: introduce show_lapic setup option

In case if a system has a large number of cpus printing apics
contents may consume a long time period. So to control it
we introduce "show_lapic" setup option which allow us to limit
the number of APICs being dumped.

Also move apic_verbosity check up so helpers routine do not
need to check it.

Signed-off-by: Yinghai Lu <[email protected]>
Signed-off-by: Cyrill Gorcunov <[email protected]>
---
arch/x86/kernel/apic/io_apic.c | 43 ++++++++++++++++++++++++++---------------
1 file changed, 28 insertions(+), 15 deletions(-)

Index: linux-2.6.git/arch/x86/kernel/apic/io_apic.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6.git/arch/x86/kernel/apic/io_apic.c
@@ -1599,9 +1599,6 @@ __apicdebuginit(void) print_IO_APIC(void
struct irq_desc *desc;
unsigned int irq;

- if (apic_verbosity == APIC_QUIET)
- return;
-
printk(KERN_DEBUG "number of MP IRQ sources: %d.\n", mp_irq_entries);
for (i = 0; i < nr_ioapics; i++)
printk(KERN_DEBUG "number of IO-APIC #%d registers: %d.\n",
@@ -1708,9 +1705,6 @@ __apicdebuginit(void) print_APIC_field(i
{
int i;

- if (apic_verbosity == APIC_QUIET)
- return;
-
printk(KERN_DEBUG);

for (i = 0; i < 8; i++)
@@ -1724,9 +1718,6 @@ __apicdebuginit(void) print_local_APIC(v
unsigned int i, v, ver, maxlvt;
u64 icr;

- if (apic_verbosity == APIC_QUIET)
- return;
-
printk(KERN_DEBUG "printing local APIC contents on CPU#%d/%d:\n",
smp_processor_id(), hard_smp_processor_id());
v = apic_read(APIC_ID);
@@ -1824,13 +1815,19 @@ __apicdebuginit(void) print_local_APIC(v
printk("\n");
}

-__apicdebuginit(void) print_all_local_APICs(void)
+__apicdebuginit(void) print_local_APICs(int napics)
{
int cpu;

+ if (!napics)
+ return;
+
preempt_disable();
- for_each_online_cpu(cpu)
+ for_each_online_cpu(cpu) {
+ if (!napics--)
+ break;
smp_call_function_single(cpu, print_local_APIC, NULL, 1);
+ }
preempt_enable();
}

@@ -1839,7 +1836,7 @@ __apicdebuginit(void) print_PIC(void)
unsigned int v;
unsigned long flags;

- if (apic_verbosity == APIC_QUIET || !nr_legacy_irqs)
+ if (!nr_legacy_irqs)
return;

printk(KERN_DEBUG "\nprinting PIC contents\n");
@@ -1866,21 +1863,37 @@ __apicdebuginit(void) print_PIC(void)
printk(KERN_DEBUG "... PIC ELCR: %04x\n", v);
}

-__apicdebuginit(int) print_all_ICs(void)
+static int __initdata show_lapic = NR_CPUS;
+static __init int setup_show_lapic(char *arg)
{
+ int num;
+
+ get_option(&arg, &num);
+ if (num >= 0)
+ show_lapic = num;
+
+ return 1;
+}
+__setup("show_lapic=", setup_show_lapic);
+
+__apicdebuginit(int) print_ICs(void)
+{
+ if (apic_verbosity == APIC_QUIET)
+ return 0;
+
print_PIC();

/* don't print out if apic is not there */
if (!cpu_has_apic && !apic_from_smp_config())
return 0;

- print_all_local_APICs();
+ print_local_APICs(show_lapic);
print_IO_APIC();

return 0;
}

-fs_initcall(print_all_ICs);
+fs_initcall(print_ICs);


/* Where if anywhere is the i8259 connect in external int mode */

2009-09-27 08:29:59

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH] x86: add show_lapic=

Cyrill Gorcunov wrote:
> [Yinghai Lu - Sun, Sep 27, 2009 at 12:01:52AM -0700]
> |
> | so could use that together with apci=debug to print less cpu apic.
> |
> | otherwise for system have more cpus will take a while
> |
> | also move apic_verbosity check to print_all_ICs
> |
> | Signed-off-by: Yinghai Lu <[email protected]>
> |
> | ---
> | arch/x86/kernel/apic/io_apic.c | 42 ++++++++++++++++++++++++++---------------
> | 1 file changed, 27 insertions(+), 15 deletions(-)
> ...
>
> Hi Yinghai, good idea!
>
> I've tuned your patch a bit together with changelog.
>
> 1) show_lapic=1 by default does change current kernel behaviour
> so it should be set to max cpu available.

think about system with lots of cpu. but i still want to use apic=debug

> 2) show_lapic contains the max number of lapics being printed
> so we need to count them instead of plain comparation with
> cpu number, as a result I've changed print_local_APICs. Or
> you meant _exactly_ cpuid number? In meaning like
> show_lapic=max_cpuid. Hmm?
>
your change anything about that. there is not gap between cpu id.

YH

2009-09-27 08:54:07

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH] x86: add show_lapic=

[Yinghai Lu - Sun, Sep 27, 2009 at 01:29:22AM -0700]
| Cyrill Gorcunov wrote:
| > [Yinghai Lu - Sun, Sep 27, 2009 at 12:01:52AM -0700]
| > |
| > | so could use that together with apci=debug to print less cpu apic.
| > |
| > | otherwise for system have more cpus will take a while
| > |
| > | also move apic_verbosity check to print_all_ICs
| > |
| > | Signed-off-by: Yinghai Lu <[email protected]>
| > |
| > | ---
| > | arch/x86/kernel/apic/io_apic.c | 42 ++++++++++++++++++++++++++---------------
| > | 1 file changed, 27 insertions(+), 15 deletions(-)
| > ...
| >
| > Hi Yinghai, good idea!
| >
| > I've tuned your patch a bit together with changelog.
| >
| > 1) show_lapic=1 by default does change current kernel behaviour
| > so it should be set to max cpu available.
|
| think about system with lots of cpu. but i still want to use apic=debug

but you should not change current behaviour. starting with this change
I'll get only first apic dump, so if I don;t know how many apics I have
what the number I should pass? Does it mean I would need to
pass say show_lapic=2, or 4, or 6 or whatever _all-the-time_ to be sure
my Core2Duo prints all lapics?

|
| > 2) show_lapic contains the max number of lapics being printed
| > so we need to count them instead of plain comparation with
| > cpu number, as a result I've changed print_local_APICs. Or
| > you meant _exactly_ cpuid number? In meaning like
| > show_lapic=max_cpuid. Hmm?
| >
| your change anything about that. there is not gap between cpu id.

you walk with for_each_online_cpu with is not the same
as for_each_present_cpu. yes, at this stage we hardly get
the situation when cpu initialized and then suddenly get
offline status but anyway

|
| YH
|
-- Cyrill

2009-09-27 10:28:06

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH] x86: add show_lapic=

[Cyrill Gorcunov - Sun, Sep 27, 2009 at 12:54:06PM +0400]
| [Yinghai Lu - Sun, Sep 27, 2009 at 01:29:22AM -0700]
| | Cyrill Gorcunov wrote:
| | > [Yinghai Lu - Sun, Sep 27, 2009 at 12:01:52AM -0700]
| | > |
| | > | so could use that together with apci=debug to print less cpu apic.
| | > |
| | > | otherwise for system have more cpus will take a while
| | > |
| | > | also move apic_verbosity check to print_all_ICs
| | > |
| | > | Signed-off-by: Yinghai Lu <[email protected]>
| | > |
| | > | ---
| | > | arch/x86/kernel/apic/io_apic.c | 42 ++++++++++++++++++++++++++---------------
| | > | 1 file changed, 27 insertions(+), 15 deletions(-)
| | > ...
| | >
| | > Hi Yinghai, good idea!
| | >
| | > I've tuned your patch a bit together with changelog.
| | >
| | > 1) show_lapic=1 by default does change current kernel behaviour
| | > so it should be set to max cpu available.
| |
| | think about system with lots of cpu. but i still want to use apic=debug
|
| but you should not change current behaviour. starting with this change
| I'll get only first apic dump, so if I don;t know how many apics I have
| what the number I should pass? Does it mean I would need to
| pass say show_lapic=2, or 4, or 6 or whatever _all-the-time_ to be sure
| my Core2Duo prints all lapics?
|
| |
| | > 2) show_lapic contains the max number of lapics being printed
| | > so we need to count them instead of plain comparation with
| | > cpu number, as a result I've changed print_local_APICs. Or
| | > you meant _exactly_ cpuid number? In meaning like
| | > show_lapic=max_cpuid. Hmm?
| | >
| | your change anything about that. there is not gap between cpu id.
|
| you walk with for_each_online_cpu with is not the same
| as for_each_present_cpu. yes, at this stage we hardly get
| the situation when cpu initialized and then suddenly get
| offline status but anyway
|
| |
| | YH
| |
| -- Cyrill

Also and your and my "update" (that named :-) missed the fact that get_option
may return without set argument at all, so num should be explicitly
initialized as well (in setup_show_lapic).

-- Cyrill

2009-09-27 14:40:09

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH] x86: add show_lapic=

[Cyrill Gorcunov - Sun, Sep 27, 2009 at 02:28:05PM +0400]
| [Cyrill Gorcunov - Sun, Sep 27, 2009 at 12:54:06PM +0400]
| | [Yinghai Lu - Sun, Sep 27, 2009 at 01:29:22AM -0700]
| | | Cyrill Gorcunov wrote:
| | | > [Yinghai Lu - Sun, Sep 27, 2009 at 12:01:52AM -0700]
| | | > |
| | | > | so could use that together with apci=debug to print less cpu apic.
| | | > |
| | | > | otherwise for system have more cpus will take a while
| | | > |
| | | > | also move apic_verbosity check to print_all_ICs
| | | > |
| | | > | Signed-off-by: Yinghai Lu <[email protected]>
| | | > |
| | | > | ---
| | | > | arch/x86/kernel/apic/io_apic.c | 42 ++++++++++++++++++++++++++---------------
| | | > | 1 file changed, 27 insertions(+), 15 deletions(-)
| | | > ...
| | | >
| | | > Hi Yinghai, good idea!
| | | >
| | | > I've tuned your patch a bit together with changelog.
| | | >
| | | > 1) show_lapic=1 by default does change current kernel behaviour
| | | > so it should be set to max cpu available.
| | |
| | | think about system with lots of cpu. but i still want to use apic=debug
| |
| | but you should not change current behaviour. starting with this change
| | I'll get only first apic dump, so if I don;t know how many apics I have
| | what the number I should pass? Does it mean I would need to
| | pass say show_lapic=2, or 4, or 6 or whatever _all-the-time_ to be sure
| | my Core2Duo prints all lapics?
| |
| | |
| | | > 2) show_lapic contains the max number of lapics being printed
| | | > so we need to count them instead of plain comparation with
| | | > cpu number, as a result I've changed print_local_APICs. Or
| | | > you meant _exactly_ cpuid number? In meaning like
| | | > show_lapic=max_cpuid. Hmm?
| | | >
| | | your change anything about that. there is not gap between cpu id.
| |
| | you walk with for_each_online_cpu with is not the same
| | as for_each_present_cpu. yes, at this stage we hardly get
| | the situation when cpu initialized and then suddenly get
| | offline status but anyway
| |
| | |
| | | YH
| | |
| | -- Cyrill
|
| Also and your and my "update" (that named :-) missed the fact that get_option
| may return without set argument at all, so num should be explicitly
| initialized as well (in setup_show_lapic).
|
| -- Cyrill

Yinghai, I've updated it to fix this nit. Convinced on for_each_online_cpu,
since it's early stage indeed. But for show_lapic=1 by default -- this is
plainly *wrong*. Now I put apic=debug and see only one apic dump regardless
how many apics I have, so we should place limit explicitly.

So here is an updated version. I don't know what is the best way would
be to say that initial patch was yours and the idea as well.

If you *agreed* on this patch -- you may either SOB it, Ack it, or take
it without my SOB at all (ie do anything you like) -- I don't care much :)

-- Cyrill
---
[PATCH -tip] x86,apic: introduce show_lapic setup option

In case if a system has a large number of cpus printing apics
contents may consume a long time period. So to control it
we introduce "show_lapic" setup option which allow us to limit
the number of APICs being dumped.

Example: apic=debug show_lapic=5

Also lift apic_verbosity checking upper that way so helper routines
do not need to inspect it at all.

Suggested-by: Yinghai Lu <[email protected]>
Signed-off-by: Cyrill Gorcunov <[email protected]>
---
arch/x86/kernel/apic/io_apic.c | 43 ++++++++++++++++++++++++++---------------
1 file changed, 28 insertions(+), 15 deletions(-)

Index: linux-2.6.git/arch/x86/kernel/apic/io_apic.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6.git/arch/x86/kernel/apic/io_apic.c
@@ -1599,9 +1599,6 @@ __apicdebuginit(void) print_IO_APIC(void
struct irq_desc *desc;
unsigned int irq;

- if (apic_verbosity == APIC_QUIET)
- return;
-
printk(KERN_DEBUG "number of MP IRQ sources: %d.\n", mp_irq_entries);
for (i = 0; i < nr_ioapics; i++)
printk(KERN_DEBUG "number of IO-APIC #%d registers: %d.\n",
@@ -1708,9 +1705,6 @@ __apicdebuginit(void) print_APIC_field(i
{
int i;

- if (apic_verbosity == APIC_QUIET)
- return;
-
printk(KERN_DEBUG);

for (i = 0; i < 8; i++)
@@ -1724,9 +1718,6 @@ __apicdebuginit(void) print_local_APIC(v
unsigned int i, v, ver, maxlvt;
u64 icr;

- if (apic_verbosity == APIC_QUIET)
- return;
-
printk(KERN_DEBUG "printing local APIC contents on CPU#%d/%d:\n",
smp_processor_id(), hard_smp_processor_id());
v = apic_read(APIC_ID);
@@ -1824,13 +1815,19 @@ __apicdebuginit(void) print_local_APIC(v
printk("\n");
}

-__apicdebuginit(void) print_all_local_APICs(void)
+__apicdebuginit(void) print_local_APICs(int maxcpu)
{
int cpu;

+ if (!maxcpu)
+ return;
+
preempt_disable();
- for_each_online_cpu(cpu)
+ for_each_online_cpu(cpu) {
+ if (cpu >= maxcpu)
+ break;
smp_call_function_single(cpu, print_local_APIC, NULL, 1);
+ }
preempt_enable();
}

@@ -1839,7 +1836,7 @@ __apicdebuginit(void) print_PIC(void)
unsigned int v;
unsigned long flags;

- if (apic_verbosity == APIC_QUIET || !nr_legacy_irqs)
+ if (!nr_legacy_irqs)
return;

printk(KERN_DEBUG "\nprinting PIC contents\n");
@@ -1866,21 +1863,37 @@ __apicdebuginit(void) print_PIC(void)
printk(KERN_DEBUG "... PIC ELCR: %04x\n", v);
}

-__apicdebuginit(int) print_all_ICs(void)
+static int __initdata show_lapic = NR_CPUS;
+static __init int setup_show_lapic(char *arg)
{
+ int num = 0;
+
+ get_option(&arg, &num);
+ if (num >= 0)
+ show_lapic = num;
+
+ return 1;
+}
+__setup("show_lapic=", setup_show_lapic);
+
+__apicdebuginit(int) print_ICs(void)
+{
+ if (apic_verbosity == APIC_QUIET)
+ return 0;
+
print_PIC();

/* don't print out if apic is not there */
if (!cpu_has_apic && !apic_from_smp_config())
return 0;

- print_all_local_APICs();
+ print_local_APICs(show_lapic);
print_IO_APIC();

return 0;
}

-fs_initcall(print_all_ICs);
+fs_initcall(print_ICs);


/* Where if anywhere is the i8259 connect in external int mode */

2009-09-27 17:42:19

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH] x86: add show_lapic=

[Cyrill Gorcunov - Sun, Sep 27, 2009 at 06:40:08PM +0400]
...
| @@ -1866,21 +1863,37 @@ __apicdebuginit(void) print_PIC(void)
| printk(KERN_DEBUG "... PIC ELCR: %04x\n", v);
| }
|
| -__apicdebuginit(int) print_all_ICs(void)
| +static int __initdata show_lapic = NR_CPUS;
| +static __init int setup_show_lapic(char *arg)
| {
| + int num = 0;
| +
| + get_option(&arg, &num);
| + if (num >= 0)
| + show_lapic = num;
| +
| + return 1;
| +}
| +__setup("show_lapic=", setup_show_lapic);
| +
...

Sigh... this is wrong as well. The proper initial value
should be -1 :(

-- Cyrill

2009-09-28 16:08:05

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH] x86: add show_lapic=

Yinghai, if you still need show_lapic=1 by default the
acceptable solution (I guess) could be that we accept
show_lapic=all as well. So a user may safely put it without
knowing how many apics he has.

Does it look acceptable for you? Something like (tested) below.
Also I think maybe we should better extend apic= option itself instead
of intruducing new one?

Anyway, what you think? (again, if you like this patch -- SOB it, Ack it,
or whatever you want).
---
[PATCH -tip] x86,apic: limit apic dumping, introduce show_lapic setup option

In case if a system has a large number of cpus printing apics
contents may consume a long time period.

We limit such an output by 1 apic by default. But to have an
ability to see all apics or some part of them we introduce
"show_lapic" setup option which allow us to limit/unlimit
the number of APICs being dumped.

Example: apic=debug show_lapic=5, or apic=debug show_lapic=all

Also move apic_verbosity checking upper that way so helper routines
do not need to inspect it at all.

Suggested-by: Yinghai Lu <[email protected]>
Signed-off-by: Cyrill Gorcunov <[email protected]>
---
arch/x86/kernel/apic/io_apic.c | 47 +++++++++++++++++++++++++++--------------
1 file changed, 32 insertions(+), 15 deletions(-)

Index: linux-2.6.git/arch/x86/kernel/apic/io_apic.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6.git/arch/x86/kernel/apic/io_apic.c
@@ -1599,9 +1599,6 @@ __apicdebuginit(void) print_IO_APIC(void
struct irq_desc *desc;
unsigned int irq;

- if (apic_verbosity == APIC_QUIET)
- return;
-
printk(KERN_DEBUG "number of MP IRQ sources: %d.\n", mp_irq_entries);
for (i = 0; i < nr_ioapics; i++)
printk(KERN_DEBUG "number of IO-APIC #%d registers: %d.\n",
@@ -1708,9 +1705,6 @@ __apicdebuginit(void) print_APIC_field(i
{
int i;

- if (apic_verbosity == APIC_QUIET)
- return;
-
printk(KERN_DEBUG);

for (i = 0; i < 8; i++)
@@ -1724,9 +1718,6 @@ __apicdebuginit(void) print_local_APIC(v
unsigned int i, v, ver, maxlvt;
u64 icr;

- if (apic_verbosity == APIC_QUIET)
- return;
-
printk(KERN_DEBUG "printing local APIC contents on CPU#%d/%d:\n",
smp_processor_id(), hard_smp_processor_id());
v = apic_read(APIC_ID);
@@ -1824,13 +1815,19 @@ __apicdebuginit(void) print_local_APIC(v
printk("\n");
}

-__apicdebuginit(void) print_all_local_APICs(void)
+__apicdebuginit(void) print_local_APICs(int maxcpu)
{
int cpu;

+ if (!maxcpu)
+ return;
+
preempt_disable();
- for_each_online_cpu(cpu)
+ for_each_online_cpu(cpu) {
+ if (cpu >= maxcpu)
+ break;
smp_call_function_single(cpu, print_local_APIC, NULL, 1);
+ }
preempt_enable();
}

@@ -1839,7 +1836,7 @@ __apicdebuginit(void) print_PIC(void)
unsigned int v;
unsigned long flags;

- if (apic_verbosity == APIC_QUIET || !nr_legacy_irqs)
+ if (!nr_legacy_irqs)
return;

printk(KERN_DEBUG "\nprinting PIC contents\n");
@@ -1866,21 +1863,41 @@ __apicdebuginit(void) print_PIC(void)
printk(KERN_DEBUG "... PIC ELCR: %04x\n", v);
}

-__apicdebuginit(int) print_all_ICs(void)
+static int __initdata show_lapic = 1;
+static __init int setup_show_lapic(char *arg)
+{
+ int num = -1;
+
+ if (strcmp(arg, "all") == 0) {
+ show_lapic = CONFIG_NR_CPUS;
+ } else {
+ get_option(&arg, &num);
+ if (num >= 0)
+ show_lapic = num;
+ }
+
+ return 1;
+}
+__setup("show_lapic=", setup_show_lapic);
+
+__apicdebuginit(int) print_ICs(void)
{
+ if (apic_verbosity == APIC_QUIET)
+ return 0;
+
print_PIC();

/* don't print out if apic is not there */
if (!cpu_has_apic && !apic_from_smp_config())
return 0;

- print_all_local_APICs();
+ print_local_APICs(show_lapic);
print_IO_APIC();

return 0;
}

-fs_initcall(print_all_ICs);
+fs_initcall(print_ICs);


/* Where if anywhere is the i8259 connect in external int mode */

2009-09-28 17:46:51

by Yinghai Lu

[permalink] [raw]
Subject: Re: [PATCH] x86: add show_lapic=

On Mon, Sep 28, 2009 at 9:07 AM, Cyrill Gorcunov <[email protected]> wrote:
> Yinghai, if you still need show_lapic=1 by default the
> acceptable solution (I guess) could be that we accept
> show_lapic=all as well. So a user may safely put it without
> knowing how many apics he has.
>
> Does it look acceptable for you? Something like (tested) below.
> Also I think maybe we should better extend apic= option itself instead
> of intruducing new one?
>
> Anyway, what you think? (again, if you like this patch -- SOB it, Ack it,
> or whatever you want).
> ---
> [PATCH -tip] x86,apic: limit apic dumping, introduce show_lapic setup option
>
> In case if a system has a large number of cpus printing apics
> contents may consume a long time period.
>
> We limit such an output by 1 apic by default. But to have an
> ability to see all apics or some part of them we introduce
> "show_lapic" setup option which allow us to limit/unlimit
> the number of APICs being dumped.
>
> Example: apic=debug show_lapic=5, or apic=debug show_lapic=all
>
> Also move apic_verbosity checking upper that way so helper routines
> do not need to inspect it at all.
>
> Suggested-by: Yinghai Lu <[email protected]>
> Signed-off-by: Cyrill Gorcunov <[email protected]>
> ---
> ?arch/x86/kernel/apic/io_apic.c | ? 47 +++++++++++++++++++++++++++--------------
> ?1 file changed, 32 insertions(+), 15 deletions(-)
>
> Index: linux-2.6.git/arch/x86/kernel/apic/io_apic.c
> =====================================================================
> --- linux-2.6.git.orig/arch/x86/kernel/apic/io_apic.c
> +++ linux-2.6.git/arch/x86/kernel/apic/io_apic.c
> @@ -1599,9 +1599,6 @@ __apicdebuginit(void) print_IO_APIC(void
> ? ? ? ?struct irq_desc *desc;
> ? ? ? ?unsigned int irq;
>
> - ? ? ? if (apic_verbosity == APIC_QUIET)
> - ? ? ? ? ? ? ? return;
> -
> ? ? ? ?printk(KERN_DEBUG "number of MP IRQ sources: %d.\n", mp_irq_entries);
> ? ? ? ?for (i = 0; i < nr_ioapics; i++)
> ? ? ? ? ? ? ? ?printk(KERN_DEBUG "number of IO-APIC #%d registers: %d.\n",
> @@ -1708,9 +1705,6 @@ __apicdebuginit(void) print_APIC_field(i
> ?{
> ? ? ? ?int i;
>
> - ? ? ? if (apic_verbosity == APIC_QUIET)
> - ? ? ? ? ? ? ? return;
> -
> ? ? ? ?printk(KERN_DEBUG);
>
> ? ? ? ?for (i = 0; i < 8; i++)
> @@ -1724,9 +1718,6 @@ __apicdebuginit(void) print_local_APIC(v
> ? ? ? ?unsigned int i, v, ver, maxlvt;
> ? ? ? ?u64 icr;
>
> - ? ? ? if (apic_verbosity == APIC_QUIET)
> - ? ? ? ? ? ? ? return;
> -
> ? ? ? ?printk(KERN_DEBUG "printing local APIC contents on CPU#%d/%d:\n",
> ? ? ? ? ? ? ? ?smp_processor_id(), hard_smp_processor_id());
> ? ? ? ?v = apic_read(APIC_ID);
> @@ -1824,13 +1815,19 @@ __apicdebuginit(void) print_local_APIC(v
> ? ? ? ?printk("\n");
> ?}
>
> -__apicdebuginit(void) print_all_local_APICs(void)
> +__apicdebuginit(void) print_local_APICs(int maxcpu)
> ?{
> ? ? ? ?int cpu;
>
> + ? ? ? if (!maxcpu)
> + ? ? ? ? ? ? ? return;
> +
> ? ? ? ?preempt_disable();
> - ? ? ? for_each_online_cpu(cpu)
> + ? ? ? for_each_online_cpu(cpu) {
> + ? ? ? ? ? ? ? if (cpu >= maxcpu)
> + ? ? ? ? ? ? ? ? ? ? ? break;
> ? ? ? ? ? ? ? ?smp_call_function_single(cpu, print_local_APIC, NULL, 1);
> + ? ? ? }
> ? ? ? ?preempt_enable();
> ?}
>
> @@ -1839,7 +1836,7 @@ __apicdebuginit(void) print_PIC(void)
> ? ? ? ?unsigned int v;
> ? ? ? ?unsigned long flags;
>
> - ? ? ? if (apic_verbosity == APIC_QUIET || !nr_legacy_irqs)
> + ? ? ? if (!nr_legacy_irqs)
> ? ? ? ? ? ? ? ?return;
>
> ? ? ? ?printk(KERN_DEBUG "\nprinting PIC contents\n");
> @@ -1866,21 +1863,41 @@ __apicdebuginit(void) print_PIC(void)
> ? ? ? ?printk(KERN_DEBUG "... PIC ELCR: %04x\n", v);
> ?}
>
> -__apicdebuginit(int) print_all_ICs(void)
> +static int __initdata show_lapic = 1;
> +static __init int setup_show_lapic(char *arg)
> +{
> + ? ? ? int num = -1;
> +
> + ? ? ? if (strcmp(arg, "all") == 0) {
> + ? ? ? ? ? ? ? show_lapic = CONFIG_NR_CPUS;

NR_CPUS or nr_cpu_ids?

YH

2009-09-28 18:02:01

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH] x86: add show_lapic=

[Yinghai Lu - Mon, Sep 28, 2009 at 10:46:54AM -0700]
| On Mon, Sep 28, 2009 at 9:07 AM, Cyrill Gorcunov <[email protected]> wrote:
...
| >
| > -__apicdebuginit(int) print_all_ICs(void)
| > +static int __initdata show_lapic = 1;
| > +static __init int setup_show_lapic(char *arg)
| > +{
| > + ? ? ? int num = -1;
| > +
| > + ? ? ? if (strcmp(arg, "all") == 0) {
| > + ? ? ? ? ? ? ? show_lapic = CONFIG_NR_CPUS;
|
| NR_CPUS or nr_cpu_ids?

NR_CPUS as far as I can tell -- maximal to not exit for() loop until end.
nr_cpu_ids safe here as well, but constant is better :)

|
| YH
|
-- Cyrill

2009-09-30 15:26:03

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: [PATCH] x86: add show_lapic=

[Cyrill Gorcunov - Mon, Sep 28, 2009 at 10:02:01PM +0400]
| [Yinghai Lu - Mon, Sep 28, 2009 at 10:46:54AM -0700]
| | On Mon, Sep 28, 2009 at 9:07 AM, Cyrill Gorcunov <[email protected]> wrote:
| ...
| | >
| | > -__apicdebuginit(int) print_all_ICs(void)
| | > +static int __initdata show_lapic = 1;
| | > +static __init int setup_show_lapic(char *arg)
| | > +{
| | > + ? ? ? int num = -1;
| | > +
| | > + ? ? ? if (strcmp(arg, "all") == 0) {
| | > + ? ? ? ? ? ? ? show_lapic = CONFIG_NR_CPUS;
| |
| | NR_CPUS or nr_cpu_ids?
|
| NR_CPUS as far as I can tell -- maximal to not exit for() loop until end.
| nr_cpu_ids safe here as well, but constant is better :)
|
| |
| | YH
| |
| -- Cyrill

Yinghai, any news here? I would like to know your conclusion
on this patch.

-- Cyrill