From owner-freebsd-current@freebsd.org Sat May 18 19:48:46 2019 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id D6654159674A for ; Sat, 18 May 2019 19:48:45 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 28B358E6FC; Sat, 18 May 2019 19:48:45 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id x4IJmViB088716 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Sat, 18 May 2019 22:48:35 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua x4IJmViB088716 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id x4IJmVjB088715; Sat, 18 May 2019 22:48:31 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Sat, 18 May 2019 22:48:31 +0300 From: Konstantin Belousov To: Warner Losh Cc: Mark Johnston , Rebecca Cran , freebsd-current@freebsd.org Subject: Re: panic booting with if_tap_load="YES" in loader.conf Message-ID: <20190518194831.GG2748@kib.kiev.ua> References: <105ad083-ea95-21a7-35be-de01e32578c4@bluestop.org> <20190518053328.GA7370@raichu> <20190518085546.GY2748@kib.kiev.ua> <20190518142436.GB7370@raichu> <20190518143815.GD2748@kib.kiev.ua> <20190518144547.GE2748@kib.kiev.ua> <20190518154653.GD7370@raichu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.11.4 (2019-03-13) X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on tom.home X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 18 May 2019 19:48:46 -0000 On Sat, May 18, 2019 at 12:57:29PM -0600, Warner Losh wrote: > On Sat, May 18, 2019, 9:48 AM Mark Johnston wrote: > > > On Sat, May 18, 2019 at 05:45:47PM +0300, Konstantin Belousov wrote: > > > On Sat, May 18, 2019 at 05:38:15PM +0300, Konstantin Belousov wrote: > > > > On Sat, May 18, 2019 at 10:24:36AM -0400, Mark Johnston wrote: > > > > > On Sat, May 18, 2019 at 11:55:46AM +0300, Konstantin Belousov wrote: > > > > > > On Sat, May 18, 2019 at 01:33:28AM -0400, Mark Johnston wrote: > > > > > > > On Fri, May 17, 2019 at 10:18:57PM -0600, Rebecca Cran wrote: > > > > > > > > I just updated from r346856 to r347950 and ran into a new > > panic, caused > > > > > > > > by having if_tap_load="YES" in /boot/loader.conf - because > > it's already > > > > > > > > built-in to the kernel: > > > > > > > > > > > > > > I think this patch should fix the panic, but I only > > compile-tested it. > > > > > > > I considered having the logic live in preload_delete_name() > > instead, but > > > > > > > the boot-time ucode code must still defer the deletion somewhat. > > > > > > > > > > > > Try this instead. I will revert r347931 after this landed, or > > could keep > > > > > > it alone. > > > > > > > > > > I have no strong feeling either way. > > > > > > > > > > > diff --git a/sys/amd64/amd64/machdep.c b/sys/amd64/amd64/machdep.c > > > > > > index 1cf09dc5cb7..03fe8a5d096 100644 > > > > > > --- a/sys/amd64/amd64/machdep.c > > > > > > +++ b/sys/amd64/amd64/machdep.c > > > > > > @@ -1616,6 +1616,13 @@ hammer_time(u_int64_t modulep, u_int64_t > > physfree) > > > > > > bzero((void *)thread0.td_kstack, kstack0_sz); > > > > > > physfree += kstack0_sz; > > > > > > > > > > > > + /* > > > > > > + * Initialize enough of thread0 for delayed invalidation to > > > > > > + * work very early. Rely on thread0.td_base_pri > > > > > > + * zero-initialization, it is reset to PVM at proc0_init(). > > > > > > + */ > > > > > > + pmap_thread_init_invl_gen(&thread0); > > > > > > + > > > > > > > > > > I think pmap_thread_init_invl_gen() also needs to initialize > > > > > invl_gen->saved_pri to 0. > > > > It is zero-initialized, same as td_base_pri. The call to > > > > pmap_thread_init_invl_gen() is needed because _INVALID bit must be set, > > > > which is cleared by default. > > > I now think that you mean something else, that invl_gen.saved_pri must > > > be set on each entry to invl_start_u(). Otherwise invl_finish_u() might > > > act on the priority from the previous DI block. > > > > That is not what I was thinking about, but I agree. thread0's saved_pri > > should be zero-initialized, but I don't see how any other thread's > > saved_pri is initialized - td_md is not in the zero or copy region of > > the thread structure. Your patch should address this as well, but maybe > > I am still missing something about how saved_pri gets initialized. The > > patch looks right to me. > > > > Could this crash on other architectures? Do we need fixes there as well... The lock-less DI only exists on amd64.