From nobody Wed Nov 17 23:25:55 2021 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 68486189F92E; Wed, 17 Nov 2021 23:25:59 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-io1-xd34.google.com (mail-io1-xd34.google.com [IPv6:2607:f8b0:4864:20::d34]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4HvfF31HVLz3PQ1; Wed, 17 Nov 2021 23:25:59 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: by mail-io1-xd34.google.com with SMTP id k22so5459945iol.13; Wed, 17 Nov 2021 15:25:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=rL3B2qtiGMYxOdKmuEJjVpVBWQ3xEU3MTYRsDUwAKLU=; b=lMqV103ldmjilVbshFlAZaokyoWMXF5jqF83p15P0Df23END9Nl9ZQ/eB5F+HrZq/f 6NnHPJDJ7iKxvYpaZydnSwKfy3M20/H5JMC/4EVvMTKsXDsMgD2P22VFygwWYDFjeCGc 2AP6wrqc4B0JcnFaFIQiq05aO9WhbW+bCYRc4PLYXNLc6z+PrK27veIjozWvZMOTyB5y TGdOOaNYuLED/vIqdYSz1zU26kq/3JNGVkcfDge81KIvxVd5n3QOI95P8+GlEXqKfU38 PvKRxpfmvSBiosplyO8/GMpkHJUmyEyx43K2rfZ/BhHYO3tFUknPgSxBieM9AOcGaGJA 8nnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to; bh=rL3B2qtiGMYxOdKmuEJjVpVBWQ3xEU3MTYRsDUwAKLU=; b=oibVAijvuj9NbKxa3f0yjVTToIKjeFguRPtRzPIXsVf4wYIbRf2G70XjIE3Eoe61RF oD36QPcSz2FOB0CQAVTjV7jsPTH3dLa0L3Dk1cBMLdwMyDiZePtajR7k5XzvidWRiZM1 XduNUci5ng2MUthzl2+HYXyjGVNavhV+/x/4u8atRnudFdktL2ata8XFGMEMVfu28AJ9 CQFUEB9XVtZC55e1dqaUaGqAW3TjUcMw+YQ/SSpmHE4ZqtxMTKiiPBhy2BuajZI9siV/ /w5sKRiQD8WxEMgFUQKIRal38j+8n97iOKavT/z81n/k/oJfR66FLJWtKswULHn6Pn8R 63FQ== X-Gm-Message-State: AOAM531WF2k7SDKWBhh1PQk28GCJDblpoMyQj05fVb0QIg4Ddu8lwZfK ooef9cu42zLyd1AK4W35YgE= X-Google-Smtp-Source: ABdhPJzXMrbfn3fwqnIpMIJW5cnZunxep8hJN5k9+bEDsIdnko88ycJenaFe8dDS0B4bUwslMu6Ddg== X-Received: by 2002:a6b:b2c1:: with SMTP id b184mr13958136iof.24.1637191558445; Wed, 17 Nov 2021 15:25:58 -0800 (PST) Received: from nuc ([142.126.186.191]) by smtp.gmail.com with ESMTPSA id x10sm978676ilv.72.2021.11.17.15.25.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Nov 2021 15:25:57 -0800 (PST) Sender: Mark Johnston Date: Wed, 17 Nov 2021 18:25:55 -0500 From: Mark Johnston To: Konstantin Belousov Cc: Kyle Evans , src-committers , "" , dev-commits-src-main@freebsd.org Subject: Re: git: 589aed00e36c - main - sched: separate out schedinit_ap() Message-ID: References: <202111032055.1A3KtLQX071805@gitrepo.freebsd.org> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 4HvfF31HVLz3PQ1 X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-ThisMailContainsUnwantedMimeParts: N On Thu, Nov 18, 2021 at 01:10:17AM +0200, Konstantin Belousov wrote: > On Wed, Nov 17, 2021 at 04:44:29PM -0600, Kyle Evans wrote: > > On Wed, Nov 3, 2021 at 3:55 PM Kyle Evans wrote: > > > > > > The branch main has been updated by kevans: > > > > > > URL: https://cgit.FreeBSD.org/src/commit/?id=589aed00e36c22733d3fd9c9016deccf074830b1 > > > > > > commit 589aed00e36c22733d3fd9c9016deccf074830b1 > > > Author: Kyle Evans > > > AuthorDate: 2021-11-02 18:06:47 +0000 > > > Commit: Kyle Evans > > > CommitDate: 2021-11-03 20:54:59 +0000 > > > > > > sched: separate out schedinit_ap() > > > > > > schedinit_ap() sets up an AP for a later call to sched_throw(NULL). > > > > > > Currently, ULE sets up some pcpu bits and fixes the idlethread lock with > > > a call to sched_throw(NULL); this results in a window where curthread is > > > setup in platforms' init_secondary(), but it has the wrong td_lock. > > > Typical platform AP startup procedure looks something like: > > > > > > - Setup curthread > > > - ... other stuff, including cpu_initclocks_ap() > > > - Signal smp_started > > > - sched_throw(NULL) to enter the scheduler > > > > > > cpu_initclocks_ap() may have callouts to process (e.g., nvme) and > > > attempt to sched_add() for this AP, but this attempt fails because > > > of the noted violated assumption leading to locking heartburn in > > > sched_setpreempt(). > > > > > > Interrupts are still disabled until cpu_throw() so we're not really at > > > risk of being preempted -- just let the scheduler in on it a little > > > earlier as part of setting up curthread. > > > > > > > What's the general consensus on potential out-of-tree archs maintained > > on stable branches? I'd like to MFC this at least to stable/13 to > > avoid it being in the way of the nvme change that spurred it, and I'm > > trying to decide if it should have something like this added to make > > it safe: > I do not believe that we even think of guaranteeing this level of source > stability. At first I assumed this was referencing sparc64, but that is not present in stable/13 either. I believe stable/13 and main support the same set of platforms, in which case I agree that we shouldn't bother with trying to provide extra compatibility, and I think it's probably not necessary to merge this to 12. > > > > diff --git a/sys/kern/sched_ule.c b/sys/kern/sched_ule.c > > index 217d685b8587..f07f5e91d8f3 100644 > > --- a/sys/kern/sched_ule.c > > +++ b/sys/kern/sched_ule.c > > @@ -2995,6 +2995,11 @@ sched_throw(struct thread *td) > > > > tdq = TDQ_SELF(); > > if (__predict_false(td == NULL)) { > > + if (tdq == NULL || PCPU_GET(idlethread)->td_lock != > > + TDQ_LOCKPTR(tdq)) { > > + schedinit_ap(); > > + tdq = TDQ_SELF(); > > + } > > TDQ_LOCK(tdq); > > /* Correct spinlock nesting. */ > > spinlock_exit(); > > >