From owner-freebsd-current@freebsd.org Tue Aug 18 14:13:33 2020 Return-Path: Delivered-To: freebsd-current@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 9D9D43BD94C for ; Tue, 18 Aug 2020 14:13:33 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mailman.nyi.freebsd.org (mailman.nyi.freebsd.org [IPv6:2610:1c1:1:606c::50:13]) by mx1.freebsd.org (Postfix) with ESMTP id 4BWCZ536r8z47Xg for ; Tue, 18 Aug 2020 14:13:33 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mailman.nyi.freebsd.org (Postfix) id 6B0223BDC13; Tue, 18 Aug 2020 14:13:33 +0000 (UTC) Delivered-To: current@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 6ACCF3BD8CB for ; Tue, 18 Aug 2020 14:13:33 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wm1-x341.google.com (mail-wm1-x341.google.com [IPv6:2a00:1450:4864:20::341]) (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 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4BWCZ46h7Wz479q; Tue, 18 Aug 2020 14:13:32 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-wm1-x341.google.com with SMTP id 3so17160763wmi.1; Tue, 18 Aug 2020 07:13:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=IwwtP5GQJ1+n0mFq8KScVIL2H3LzucYzG6bE8xIfLUA=; b=Dfo5PKBovXcyxA0bOWg7IAbhxnmuIqRp585puJefBfeKJG+SViG4FcQ7SMCYvP4gP4 4IxTnj6oQObQYXl7O4+MXGlRO72GxWO5tpHcQl1YExo6jiIMpkNA5pXToZLYlYUTJyMx 1cTcJcOnbqPdy3d2kglBaSpJ+jk7dymptTxBl0Iecnw8UGBmwaZPjhhcKOY8Dzum89zA BYpIqJBabnh6Epk2bnDhJrri4tD8hsHgxRl06CYCxLLQsiQ85dExGlBemomWfwt7qtSb ZpbnjCTZLnHyhMp8mABzFIxGzfR2+1B4wATAjmZXaXAdydpLXGLkNHZjgCF4Sqz+rKgG 8GuA== X-Gm-Message-State: AOAM532BPN6slAS1r02/wkkeryd7Dv5ubLe/W9+mq01EkAzJ5q3WklGu oCSN04HxC1Y/xENH1BdyM8bZz7IgkrxM8jx48F7HQjXl X-Google-Smtp-Source: ABdhPJxTkbX4yO5LHpEWmITSxvAOZaSZCce+ZaYGwRMHjjcoJAm8WKV0mqt7e0tMB+VFaoV84Nd0+HFTcgUpl9IrcXY= X-Received: by 2002:a7b:cb98:: with SMTP id m24mr179375wmi.10.1597760011082; Tue, 18 Aug 2020 07:13:31 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a5d:614c:0:0:0:0:0 with HTTP; Tue, 18 Aug 2020 07:13:30 -0700 (PDT) In-Reply-To: <20200818140419.GB28906@raichu> References: <20200818124419.GO1394@albert.catwhisker.org> <20200818125825.GP1394@albert.catwhisker.org> <20200818134900.GA28906@raichu> <20200818140419.GB28906@raichu> From: Mateusz Guzik Date: Tue, 18 Aug 2020 16:13:30 +0200 Message-ID: Subject: Re: "panic: malloc(M_WAITOK) in non-sleepable context" after r364296 -> r364341 To: Mark Johnston Cc: current@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 4BWCZ46h7Wz479q X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US]; REPLY(-4.00)[] X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.33 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: Tue, 18 Aug 2020 14:13:33 -0000 On 8/18/20, Mark Johnston wrote: > On Tue, Aug 18, 2020 at 03:55:15PM +0200, Mateusz Guzik wrote: >> On 8/18/20, Mark Johnston wrote: >> > On Tue, Aug 18, 2020 at 03:24:52PM +0200, Mateusz Guzik wrote: >> >> Try this: >> >> >> >> diff --git a/sys/kern/kern_malloc.c b/sys/kern/kern_malloc.c >> >> index 46eb1c4347c..7b94ca7b880 100644 >> >> --- a/sys/kern/kern_malloc.c >> >> +++ b/sys/kern/kern_malloc.c >> >> @@ -618,8 +618,8 @@ void * >> >> unsigned long osize = size; >> >> #endif >> >> >> >> - KASSERT((flags & M_WAITOK) == 0 || THREAD_CAN_SLEEP(), >> >> - ("malloc(M_WAITOK) in non-sleepable context")); >> >> + if ((flags & M_WAITOK) != 0) >> >> + WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, >> >> __func__); >> >> >> >> #ifdef MALLOC_DEBUG >> >> va = NULL; >> >> diff --git a/sys/vm/uma_core.c b/sys/vm/uma_core.c >> >> index 37d78354200..2e1267ec02f 100644 >> >> --- a/sys/vm/uma_core.c >> >> +++ b/sys/vm/uma_core.c >> >> @@ -3355,8 +3355,8 @@ uma_zalloc_arg(uma_zone_t zone, void *udata, int >> >> flags) >> >> uma_cache_bucket_t bucket; >> >> uma_cache_t cache; >> >> >> >> - KASSERT((flags & M_WAITOK) == 0 || THREAD_CAN_SLEEP(), >> >> - ("uma_zalloc(M_WAITOK) in non-sleepable context")); >> >> + if ((flags & M_WAITOK) != 0) >> >> + WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, >> >> __func__); >> >> >> >> /* Enable entropy collection for RANDOM_ENABLE_UMA kernel >> >> option >> >> */ >> >> random_harvest_fast_uma(&zone, sizeof(zone), RANDOM_UMA); >> > >> > Doesn't it only print a warning if non-sleepable locks are held? >> > THREAD_CAN_SLEEP() catches other cases, like epoch sections and worker >> > threads that are not allowed to sleep (CAM doneq threads in this case). >> > Otherwise uma_zalloc_debug() already checks with WITNESS. >> > >> > I'm inclined to simply revert the commit for now. Alternately, >> > disk_alloc() could be modified to handle M_NOWAIT/M_WAITOK flags, and >> > that could be used in daregister() and other CAM periph drivers. >> > daregister() already uses M_NOWAIT to allocate the driver softc itself. >> > >> >> If WITNESS_WARN(.., WARN_SLEEPOK) does not check for all possible >> blockers for going off CPU that's a bug. I do support reverting the >> patch for now until the dust settles. I don't propose the above hack >> as the final fix. > > Well, IMO the bug is that we have no subroutine to perform all of these > checks (which includes those done by WITNESS_WARN(..., WARN_SLEEPOK)). > WITNESS' responsibilities are more narrow. I just meant that your patch > effectively reverts Gleb's commit. > not exactly, as it still adds something for malloc. I argued for a dedicated routine to perform all the relevant checks. Note a dedicated routine instead of a handrolled call to witness/panic/whatever can also report the exact blockers (if it knows them), in this case what lock was taken and where. I would not mind any extras (e.g., critnest). >> As for the culrpit at hand, given the backtrace: >> devfs_alloc() at devfs_alloc+0x28/frame 0xffffffff8218d570 >> make_dev_sv() at make_dev_sv+0x97/frame 0xffffffff8218d600 >> make_dev_s() at make_dev_s+0x3b/frame 0xffffffff8218d660 >> passregister() at passregister+0x3e7/frame 0xffffffff8218d8b0 >> >> I think this is missing wait flags, resulting in M_WAITOK later, i.e.: > > Ah, I was looking at a different report. All the more reason to revert > for now. > -- Mateusz Guzik