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

next in thread | previous in thread | raw e-mail | index | archive | help
Il 10 aprile 2012 14:25, Marius Strobl <marius@alchemy.franken.de> ha scritto:
> On Tue, Apr 10, 2012 at 01:55:31PM +0100, Attilio Rao wrote:
>> Il 10 aprile 2012 12:41, Marius Strobl <marius@alchemy.franken.de> ha scritto:
>> > On Tue, Apr 10, 2012 at 01:03:56AM +0100, Attilio Rao wrote:
>> >> Il 10 aprile 2012 00:09, Marius Strobl <marius@alchemy.franken.de> ha scritto:
>> >> > On Mon, Apr 09, 2012 at 10:41:19PM +0000, Attilio Rao wrote:
>> >> >> Author: attilio
>> >> >> Date: Mon Apr ??9 22:41:19 2012
>> >> >> New Revision: 234074
>> >> >> URL: http://svn.freebsd.org/changeset/base/234074
>> >> >>
>> >> >> Log:
>> >> >> ?? BSP is not added to the mask of valid target CPUs for interrupts
>> >> >> ?? in set_apic_interrupt_ids(). Besides, set_apic_interrupts_ids() is not
>> >> >> ?? called in the !SMP case too.
>> >> >> ?? Fix this by:
>> >> >> ?? - Adding the BSP as an interrupt target directly in cpu_startup().
>> >> >> ?? - Remove an obsolete optimization where the BSP are skipped in
>> >> >> ?? ?? set_apic_interrupt_ids().
>> >> >>
>> >> >> ?? Reported by: ?? ?? ?? ??jh
>> >> >> ?? Reviewed by: ?? ?? ?? ??jhb
>> >> >> ?? MFC after: ??3 days
>> >> >> ?? X-MFC: ?? ?? ?? ?? ?? ?? ??r233961
>> >> >> ?? Pointy hat to: ?? ?? ??me
>> >> >>
>> >> >> Modified:
>> >> >> ?? head/sys/amd64/amd64/machdep.c
>> >> >> ?? head/sys/amd64/amd64/mp_machdep.c
>> >> >> ?? head/sys/i386/i386/machdep.c
>> >> >> ?? head/sys/i386/i386/mp_machdep.c
>> >> >>
>> >> >> Modified: head/sys/amd64/amd64/machdep.c
>> >> >> ==============================================================================
>> >> >> --- head/sys/amd64/amd64/machdep.c ?? ??Mon Apr ??9 22:01:43 2012 ?? ?? ?? ??(r234073)
>> >> >> +++ head/sys/amd64/amd64/machdep.c ?? ??Mon Apr ??9 22:41:19 2012 ?? ?? ?? ??(r234074)
>> >> >> @@ -295,6 +295,11 @@ cpu_startup(dummy)
>> >> >> ?? ?? ?? vm_pager_bufferinit();
>> >> >>
>> >> >> ?? ?? ?? cpu_setregs();
>> >> >> +
>> >> >> + ?? ?? /*
>> >> >> + ?? ?? ??* Add BSP as an interrupt target.
>> >> >> + ?? ?? ??*/
>> >> >> + ?? ?? intr_add_cpu(0);
>> >> >> ??}
>> >> >
>> >> > 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 = 1.
>> >> I think the attached patch should make its dirty job, opinion?
>> >
>> > I currently fail to see why the latter approach would be necessary,
>> > i.e. IMO wrapping the intr_add_cpu() calls in cpu_startup() should
>> > be sufficient. In case the kernel is compiled without SMP support,
>> > interrupt balancing support isn't available in the first place and
>> > the BSP is always the only available target (see the UP version of
>> > intr_next_cpu() at the end of x86/x86/intr_machdep.c), so there's
>> > no need to add the BSP as a valid target. If an SMP kernel is run
>> > on a UP machine or with SMP disabled, interrupt balancing support
>> > is available but the intr_add_cpu() calls in cpu_startup() will add
>> > the BSP as (the only) target, so everything should be fine. Maybe
>> > you can elaborate on why you think an SMP kernel with SMP disabled
>> > needs special handling.
>>
>> I do not understand what you mean.
>
> Well, so we are at least in the same boat :)
>
>> Right now there is a compile time issue where for !SMP kernel it won't
>> compile and this is what I'm trying to fix now, so I don't understand
>> what do you mean here.
>>
>
> AFAICT the below patch should take care of UP in both the compile-time
> and run-time cases.

Not sure why, I was convinced that this patch was going to break the
SMP + smp_disabled=1 case but of course it is not.

Please go ahead and commit it, thanks.

Attilio


-- 
Peace can only be achieved by understanding - A. Einstein



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-FndBYCMAutA0KnWqXS4s7V-twn7ukqRxQtkA=H7S4oU_S3A>