From owner-dev-commits-src-main@freebsd.org Thu Mar 11 17:24:03 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 8E31C57C07E; Thu, 11 Mar 2021 17:24:03 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4DxG5H3Xj6z3MTK; Thu, 11 Mar 2021 17:24:03 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from John-Baldwins-MacBook-Pro.local (ralph.baldwin.cx [66.234.199.215]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id DA889D86E; Thu, 11 Mar 2021 17:24:02 +0000 (UTC) (envelope-from jhb@FreeBSD.org) To: Mateusz Guzik , Kyle Evans Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org References: <202103091117.129BHOZa042851@gitrepo.freebsd.org> From: John Baldwin Subject: Re: git: 1ae20f7c70ea - main - kern: malloc: fix panic on M_WAITOK during THREAD_NO_SLEEPING() Message-ID: <8e37c710-bd9d-6fe0-0263-4efeabfd9beb@FreeBSD.org> Date: Thu, 11 Mar 2021 09:23:58 -0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Mar 2021 17:24:03 -0000 On 3/10/21 3:57 AM, Mateusz Guzik wrote: > There is something very wrong going on here. > > Key thing to note is that malloc is ultimately a wrapper around > uma_zalloc. Whatever asserts you may want to add to malloc to catch > problems sooner, should also present in uma. > > uma has the following: > > if (flags & M_WAITOK) { > WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, > "uma_zalloc_debug: zone \"%s\"", zone->uz_name); > } This warns about holding locks, not about TD_NO_SLEEPING. Witness' role is to check lock orders and warn about invalid operations when holding certain locks. It does not currently verify anything about other inhibitions like TD_NO_SLEEPING. See, e.g. the list of assertions in userret() which includes a WITNESS_WARN in addition to several other checks (including TD_NO_SLEEPING). Arguably we should just move the TD_NO_SLEEPING check from malloc() to here along with the td_intr_nesting_level. Any case where you can't use malloc() with M_WAITOK you can't use uma with M_WAITOK either. > This code used to execute prior to this commit and fail to catch the > problems which got reported already. > > In other words: > - WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK is incomplete in terms of > internals its internals > - the above should be in malloc, perhaps after being abstracted into a > an assert-helper > - fixing witness deficiency will probably find a slew of new problems > - this should remain as a warning (maybe rate-limited) for the foreseable future I don't know that we've had so many issues that we can't just fix them on HEAD right now vs having to make this a warning instead of keeping it as a panic. -- John Baldwin