From owner-freebsd-current@freebsd.org Tue Aug 18 13:55:19 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 E03A53BD18A for ; Tue, 18 Aug 2020 13:55:19 +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 4BWC9353tlz464Y for ; Tue, 18 Aug 2020 13:55:19 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mailman.nyi.freebsd.org (Postfix) id AC0623BCF4B; Tue, 18 Aug 2020 13:55:19 +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 ABCE33BCE64 for ; Tue, 18 Aug 2020 13:55:19 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wr1-x441.google.com (mail-wr1-x441.google.com [IPv6:2a00:1450:4864:20::441]) (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 4BWC930LHTz45vH; Tue, 18 Aug 2020 13:55:18 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-wr1-x441.google.com with SMTP id f1so18385919wro.2; Tue, 18 Aug 2020 06:55:18 -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=5JbnFtOPXPUjiDtfS9oDMCq/mxcYSSaf2BpIMzPtY9g=; b=WOS8OoF6J5wB8rv3wBN5rwigI63Yfg6Pf3eAd99ZQbLvGWFXELctV+kyQOVjc1GoCB uFGrPh7U6TrKffaAQy/KC5v0dLqeA3EIzmb/jaodMWzPQMoOZHTwQXey2zU+bAIX4mcy rjaGM0VkkX2GIfZ2PbMK41CX468zh6xJZDccCIaKS0MRcsM2l+++IIhGEl3cb9inyv9U 2CeG3dsG0s9OX/A9TyySHndN/4K2sx8BgadZGYDCxo+PeW38RXsa5RWWv0k1GX1o0QJz ieRP0JMGe83AVRBUP9EvkvuUcbbRuYTUG42wQBaPrvbG7mcNkY8SLMWbwH2Igkj6pmlO PKHg== X-Gm-Message-State: AOAM533DUWxVaxM3nBS6e6px+gAv/dVEEcydyK98utV92ZUN9pNTu9nV 57pnncTcgYHgiPYsoo0xtUgL6NAHmV60wP9I9nKYqzoT X-Google-Smtp-Source: ABdhPJzx5gHWffdw8MAFmP5nBetXDoGaXMzER5SGKsIBMTEeIKAMMvEzH5qBPSHYVLBPQa19IBDAV3tFRlkN0YDpLJA= X-Received: by 2002:adf:e94c:: with SMTP id m12mr20424602wrn.109.1597758916384; Tue, 18 Aug 2020 06:55:16 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a5d:614c:0:0:0:0:0 with HTTP; Tue, 18 Aug 2020 06:55:15 -0700 (PDT) In-Reply-To: <20200818134900.GA28906@raichu> References: <20200818124419.GO1394@albert.catwhisker.org> <20200818125825.GP1394@albert.catwhisker.org> <20200818134900.GA28906@raichu> From: Mateusz Guzik Date: Tue, 18 Aug 2020 15:55:15 +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: 4BWC930LHTz45vH X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US] 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 13:55:19 -0000 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. 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.: diff --git a/sys/cam/scsi/scsi_pass.c b/sys/cam/scsi/scsi_pass.c index b299fbddd84..c6d6a5da403 100644 --- a/sys/cam/scsi/scsi_pass.c +++ b/sys/cam/scsi/scsi_pass.c @@ -652,6 +652,7 @@ passregister(struct cam_periph *periph, void *arg) args.mda_gid = GID_OPERATOR; args.mda_mode = 0600; args.mda_si_drv1 = periph; + args.mda_flags = MAKEDEV_NOWAIT; error = make_dev_s(&args, &softc->dev, "%s%d", periph->periph_name, periph->unit_number); if (error != 0) { > diff --git a/sys/cam/scsi/scsi_da.c b/sys/cam/scsi/scsi_da.c > index d648d511b1a2..d940e7db24e0 100644 > --- a/sys/cam/scsi/scsi_da.c > +++ b/sys/cam/scsi/scsi_da.c > @@ -2767,18 +2767,23 @@ daregister(struct cam_periph *periph, void *arg) > return(CAM_REQ_CMP_ERR); > } > > - softc = (struct da_softc *)malloc(sizeof(*softc), M_DEVBUF, > - M_NOWAIT|M_ZERO); > - > + softc = malloc(sizeof(*softc), M_DEVBUF, M_NOWAIT | M_ZERO); > if (softc == NULL) { > printf("daregister: Unable to probe new device. " > "Unable to allocate softc\n"); > return(CAM_REQ_CMP_ERR); > } > + softc->disk = disk_alloc_flags(M_NOWAIT); > + if (softc->disk == NULL) { > + printf("daregister: Unable to probe new device. " > + "Unable to allocate disk structure\n"); > + return (CAM_REQ_CMP_ERR); > + } > > if (cam_iosched_init(&softc->cam_iosched, periph) != 0) { > printf("daregister: Unable to probe new device. " > "Unable to allocate iosched memory\n"); > + disk_destroy(softc->disk); > free(softc, M_DEVBUF); > return(CAM_REQ_CMP_ERR); > } > @@ -2898,7 +2903,6 @@ daregister(struct cam_periph *periph, void *arg) > /* > * Register this media as a disk. > */ > - softc->disk = disk_alloc(); > softc->disk->d_devstat = devstat_new_entry(periph->periph_name, > periph->unit_number, 0, > DEVSTAT_BS_UNAVAILABLE, > diff --git a/sys/geom/geom_disk.c b/sys/geom/geom_disk.c > index eaba770828d0..c07494ab4ec0 100644 > --- a/sys/geom/geom_disk.c > +++ b/sys/geom/geom_disk.c > @@ -850,15 +850,22 @@ g_disk_ident_adjust(char *ident, size_t size) > } > > struct disk * > -disk_alloc(void) > +disk_alloc_flags(int mflag) > { > struct disk *dp; > > - dp = g_malloc(sizeof(struct disk), M_WAITOK | M_ZERO); > - LIST_INIT(&dp->d_aliases); > + dp = g_malloc(sizeof(struct disk), mflag | M_ZERO); > + if (dp != NULL) > + LIST_INIT(&dp->d_aliases); > return (dp); > } > > +struct disk * > +disk_alloc(void) > +{ > + return (disk_alloc_flags(M_WAITOK)); > +} > + > void > disk_create(struct disk *dp, int version) > { > diff --git a/sys/geom/geom_disk.h b/sys/geom/geom_disk.h > index 8e2282a09a3a..794ce2cc38bd 100644 > --- a/sys/geom/geom_disk.h > +++ b/sys/geom/geom_disk.h > @@ -137,6 +137,7 @@ struct disk { > #define DISKFLAG_WRITE_PROTECT 0x0100 > > struct disk *disk_alloc(void); > +struct disk *disk_alloc_flags(int mflag); > void disk_create(struct disk *disk, int version); > void disk_destroy(struct disk *disk); > void disk_gone(struct disk *disk); > -- Mateusz Guzik