Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 10 Apr 2012 01:03:56 +0100
From:      Attilio Rao <attilio@freebsd.org>
To:        Marius Strobl <marius@alchemy.franken.de>, John Baldwin <jhb@freebsd.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r234074 - in head/sys: amd64/amd64 i386/i386
Message-ID:  <CAJ-FndDzh50_Xb%2B08EnhAVBnu1vFLo0d5MX%2BQwAOTcq_ewwDJQ@mail.gmail.com>
In-Reply-To: <20120409230949.GB68111@alchemy.franken.de>
References:  <201204092241.q39MfJZn081610@svn.freebsd.org> <20120409230949.GB68111@alchemy.franken.de>

next in thread | previous in thread | raw e-mail | index | archive | help
Il 10 aprile 2012 00:09, Marius Strobl <marius@alchemy.franken.de> ha scrit=
to:
> On Mon, Apr 09, 2012 at 10:41:19PM +0000, Attilio Rao wrote:
>> Author: attilio
>> Date: Mon Apr =C2=A09 22:41:19 2012
>> New Revision: 234074
>> URL: http://svn.freebsd.org/changeset/base/234074
>>
>> Log:
>> =C2=A0 BSP is not added to the mask of valid target CPUs for interrupts
>> =C2=A0 in set_apic_interrupt_ids(). Besides, set_apic_interrupts_ids() i=
s not
>> =C2=A0 called in the !SMP case too.
>> =C2=A0 Fix this by:
>> =C2=A0 - Adding the BSP as an interrupt target directly in cpu_startup()=
.
>> =C2=A0 - Remove an obsolete optimization where the BSP are skipped in
>> =C2=A0 =C2=A0 set_apic_interrupt_ids().
>>
>> =C2=A0 Reported by: =C2=A0 =C2=A0 =C2=A0 =C2=A0jh
>> =C2=A0 Reviewed by: =C2=A0 =C2=A0 =C2=A0 =C2=A0jhb
>> =C2=A0 MFC after: =C2=A03 days
>> =C2=A0 X-MFC: =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0r233961
>> =C2=A0 Pointy hat to: =C2=A0 =C2=A0 =C2=A0me
>>
>> Modified:
>> =C2=A0 head/sys/amd64/amd64/machdep.c
>> =C2=A0 head/sys/amd64/amd64/mp_machdep.c
>> =C2=A0 head/sys/i386/i386/machdep.c
>> =C2=A0 head/sys/i386/i386/mp_machdep.c
>>
>> Modified: head/sys/amd64/amd64/machdep.c
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
>> --- head/sys/amd64/amd64/machdep.c =C2=A0 =C2=A0Mon Apr =C2=A09 22:01:43=
 2012 =C2=A0 =C2=A0 =C2=A0 =C2=A0(r234073)
>> +++ head/sys/amd64/amd64/machdep.c =C2=A0 =C2=A0Mon Apr =C2=A09 22:41:19=
 2012 =C2=A0 =C2=A0 =C2=A0 =C2=A0(r234074)
>> @@ -295,6 +295,11 @@ cpu_startup(dummy)
>> =C2=A0 =C2=A0 =C2=A0 vm_pager_bufferinit();
>>
>> =C2=A0 =C2=A0 =C2=A0 cpu_setregs();
>> +
>> + =C2=A0 =C2=A0 /*
>> + =C2=A0 =C2=A0 =C2=A0* Add BSP as an interrupt target.
>> + =C2=A0 =C2=A0 =C2=A0*/
>> + =C2=A0 =C2=A0 intr_add_cpu(0);
>> =C2=A0}
>
> If I'm not mistaken, intr_add_cpu() is under #ifdef SMP, so it should be
> here as well.

You are right, sorry, I did forgot to test without SMP.
I think we still need intr_add_cpu() on cpu_startup() because of the
case smp_disabled =3D 1.
I think the attached patch should make its dirty job, opinion?

Thanks,
Attilio

Index: sys/i386/include/intr_machdep.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- sys/i386/include/intr_machdep.h     (revisione 234073)
+++ sys/i386/include/intr_machdep.h     (copia locale)
@@ -131,9 +131,7 @@ int elcr_probe(void);
 enum intr_trigger elcr_read_trigger(u_int irq);
 void   elcr_resume(void);
 void   elcr_write_trigger(u_int irq, enum intr_trigger trigger);
-#ifdef SMP
 void   intr_add_cpu(u_int cpu);
-#endif
 int    intr_add_handler(const char *name, int vector, driver_filter_t filt=
er,
     driver_intr_t handler, void *arg, enum intr_type flags, void **cookiep=
);
 #ifdef SMP
Index: sys/amd64/include/intr_machdep.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- sys/amd64/include/intr_machdep.h    (revisione 234073)
+++ sys/amd64/include/intr_machdep.h    (copia locale)
@@ -140,9 +140,7 @@ int elcr_probe(void);
 enum intr_trigger elcr_read_trigger(u_int irq);
 void   elcr_resume(void);
 void   elcr_write_trigger(u_int irq, enum intr_trigger trigger);
-#ifdef SMP
 void   intr_add_cpu(u_int cpu);
-#endif
 int    intr_add_handler(const char *name, int vector, driver_filter_t filt=
er,
                         driver_intr_t handler, void *arg, enum
intr_type flags,
                         void **cookiep);
Index: sys/x86/x86/intr_machdep.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- sys/x86/x86/intr_machdep.c  (revisione 234073)
+++ sys/x86/x86/intr_machdep.c  (copia locale)
@@ -446,16 +446,34 @@ DB_SHOW_COMMAND(irqs, db_show_irqs)
 }
 #endif

-#ifdef SMP
 /*
  * Support for balancing interrupt sources across CPUs.  For now we just
  * allocate CPUs round-robin.
  */

 static cpuset_t intr_cpus;
+#ifdef SMP
 static int current_cpu;
+#endif

 /*
+ * Add a CPU to our mask of valid CPUs that can be destinations of
+ * interrupts.
+ */
+void
+intr_add_cpu(u_int cpu)
+{
+
+       if (cpu >=3D MAXCPU)
+               panic("%s: Invalid CPU ID", __func__);
+       if (bootverbose)
+               printf("INTR: Adding CPU %u as a target\n", cpu);
+
+       CPU_SET(cpu, &intr_cpus);
+}
+
+#ifdef SMP
+/*
  * Return the CPU that the next interrupt source should use.  For now
  * this just returns the next local APIC according to round-robin.
  */
@@ -492,23 +510,6 @@ intr_bind(u_int vector, u_char cpu)
 }

 /*
- * Add a CPU to our mask of valid CPUs that can be destinations of
- * interrupts.
- */
-void
-intr_add_cpu(u_int cpu)
-{
-
-       if (cpu >=3D MAXCPU)
-               panic("%s: Invalid CPU ID", __func__);
-       if (bootverbose)
-               printf("INTR: Adding local APIC %d as a target\n",
-                   cpu_apic_ids[cpu]);
-
-       CPU_SET(cpu, &intr_cpus);
-}
-
-/*
  * Distribute all the interrupt sources among the available CPUs once the
  * AP's have been launched.
  */



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-FndDzh50_Xb%2B08EnhAVBnu1vFLo0d5MX%2BQwAOTcq_ewwDJQ>