Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 22 Jan 2020 18:39:22 -0800
From:      Ryan Libby <rlibby@freebsd.org>
To:        Konstantin Belousov <kib@freebsd.org>
Cc:        src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>,  svn-src-head <svn-src-head@freebsd.org>, Mark Johnston <markj@freebsd.org>
Subject:   Re: svn commit: r356919 - head/sys/x86/x86
Message-ID:  <CAHgpiFzfiLT4tuHkK0yzhnCah-zUWrrQSVd-k%2Buv3gzgA4_a9w@mail.gmail.com>
In-Reply-To: <202001201723.00KHN3tX093432@repo.freebsd.org>
References:  <202001201723.00KHN3tX093432@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Jan 20, 2020 at 9:23 AM Konstantin Belousov <kib@freebsd.org> wrote:
>
> Author: kib
> Date: Mon Jan 20 17:23:03 2020
> New Revision: 356919
> URL: https://svnweb.freebsd.org/changeset/base/356919
>
> Log:
>   x86: Wait for curthread to be set up as an indicator that the boot stack
>   is no longer used.
>
>   pc_curthread is set by cpu_switch after it stopped using the old
>   thread (or boot) stack.  This makes the smp_after_idle_runnable()
>   function not dependent on the internals of the scheduler operations.
>
>   Reviewed by:  markj
>   Sponsored by: The FreeBSD Foundation
>   MFC after:    1 week
>   Differential revision:        https://reviews.freebsd.org/D23276
>
> Modified:
>   head/sys/x86/x86/mp_x86.c
>
> Modified: head/sys/x86/x86/mp_x86.c
> ==============================================================================
> --- head/sys/x86/x86/mp_x86.c   Mon Jan 20 16:59:39 2020        (r356918)
> +++ head/sys/x86/x86/mp_x86.c   Mon Jan 20 17:23:03 2020        (r356919)
> @@ -1092,13 +1092,12 @@ init_secondary_tail(void)
>  static void
>  smp_after_idle_runnable(void *arg __unused)
>  {
> -       struct thread *idle_td;
> +       struct pcpu *pc;
>         int cpu;
>
>         for (cpu = 1; cpu < mp_ncpus; cpu++) {
> -               idle_td = pcpu_find(cpu)->pc_idlethread;
> -               while (atomic_load_int(&idle_td->td_lastcpu) == NOCPU &&
> -                   atomic_load_int(&idle_td->td_oncpu) == NOCPU)
> +               pc = pcpu_find(cpu);
> +               while (atomic_load_ptr(&pc->pc_curthread) == (uintptr_t)NULL)
>                         cpu_spinwait();
>                 kmem_free((vm_offset_t)bootstacks[cpu], kstack_pages *
>                     PAGE_SIZE);
> _______________________________________________
> svn-src-all@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/svn-src-all
> To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"

I'm hitting a boot panic on a KVM VM that I think is because of this.
I don't think this works as advertised, because init_secondary_tail sets
curthread to its idlethread *itself* before it calls sched_switch.  So I
think the current check is not enough to know that we're actually off
the bootstack.

My panic is an AP page faults in the middle of init_secondary_tail,
after curthread is set.  Weirdly, I only seem to hit it when I have
disabled some CPUs (to test D23318).  I think this must just be
affecting some aspect of the timing.

Ryan



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAHgpiFzfiLT4tuHkK0yzhnCah-zUWrrQSVd-k%2Buv3gzgA4_a9w>